Skip to content

Commit 2dd5204

Browse files
committed
Recommit "[LICM] Support integer mul/add in hoistFPAssociation. (#67736)"
With a fix for build bot failure. I was accessing the type of a deleted Instruction. Original message: The reassociation this is trying to repair can happen for integer types too. This patch adds support for integer mul/add to hoistFPAssociation. The function has been renamed to hoistMulAddAssociation. I've used separate statistics and limits for integer to allow tuning flexibility.
1 parent 91dcf53 commit 2dd5204

File tree

2 files changed

+414
-19
lines changed

2 files changed

+414
-19
lines changed

llvm/lib/Transforms/Scalar/LICM.cpp

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ STATISTIC(NumAddSubHoisted, "Number of add/subtract expressions reassociated "
110110
"and hoisted out of the loop");
111111
STATISTIC(NumFPAssociationsHoisted, "Number of invariant FP expressions "
112112
"reassociated and hoisted out of the loop");
113+
STATISTIC(NumIntAssociationsHoisted,
114+
"Number of invariant int expressions "
115+
"reassociated and hoisted out of the loop");
113116

114117
/// Memory promotion is enabled by default.
115118
static cl::opt<bool>
@@ -135,6 +138,12 @@ static cl::opt<unsigned> FPAssociationUpperLimit(
135138
"Set upper limit for the number of transformations performed "
136139
"during a single round of hoisting the reassociated expressions."));
137140

141+
cl::opt<unsigned> IntAssociationUpperLimit(
142+
"licm-max-num-int-reassociations", cl::init(5U), cl::Hidden,
143+
cl::desc(
144+
"Set upper limit for the number of transformations performed "
145+
"during a single round of hoisting the reassociated expressions."));
146+
138147
// Experimental option to allow imprecision in LICM in pathological cases, in
139148
// exchange for faster compile. This is to be removed if MemorySSA starts to
140149
// address the same issue. LICM calls MemorySSAWalker's
@@ -2661,21 +2670,29 @@ static bool hoistAddSub(Instruction &I, Loop &L, ICFLoopSafetyInfo &SafetyInfo,
26612670
return false;
26622671
}
26632672

2673+
static bool isReassociableOp(Instruction *I, unsigned IntOpcode,
2674+
unsigned FPOpcode) {
2675+
if (I->getOpcode() == IntOpcode)
2676+
return true;
2677+
if (I->getOpcode() == FPOpcode && I->hasAllowReassoc() &&
2678+
I->hasNoSignedZeros())
2679+
return true;
2680+
return false;
2681+
}
2682+
26642683
/// Try to reassociate expressions like ((A1 * B1) + (A2 * B2) + ...) * C where
26652684
/// A1, A2, ... and C are loop invariants into expressions like
26662685
/// ((A1 * C * B1) + (A2 * C * B2) + ...) and hoist the (A1 * C), (A2 * C), ...
26672686
/// invariant expressions. This functions returns true only if any hoisting has
26682687
/// actually occured.
2669-
static bool hoistFPAssociation(Instruction &I, Loop &L,
2670-
ICFLoopSafetyInfo &SafetyInfo,
2671-
MemorySSAUpdater &MSSAU, AssumptionCache *AC,
2672-
DominatorTree *DT) {
2673-
using namespace PatternMatch;
2674-
Value *VariantOp = nullptr, *InvariantOp = nullptr;
2675-
2676-
if (!match(&I, m_FMul(m_Value(VariantOp), m_Value(InvariantOp))) ||
2677-
!I.hasAllowReassoc() || !I.hasNoSignedZeros())
2688+
static bool hoistMulAddAssociation(Instruction &I, Loop &L,
2689+
ICFLoopSafetyInfo &SafetyInfo,
2690+
MemorySSAUpdater &MSSAU, AssumptionCache *AC,
2691+
DominatorTree *DT) {
2692+
if (!isReassociableOp(&I, Instruction::Mul, Instruction::FMul))
26782693
return false;
2694+
Value *VariantOp = I.getOperand(0);
2695+
Value *InvariantOp = I.getOperand(1);
26792696
if (L.isLoopInvariant(VariantOp))
26802697
std::swap(VariantOp, InvariantOp);
26812698
if (L.isLoopInvariant(VariantOp) || !L.isLoopInvariant(InvariantOp))
@@ -2689,15 +2706,17 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
26892706
Worklist.push_back(VariantBinOp);
26902707
while (!Worklist.empty()) {
26912708
BinaryOperator *BO = Worklist.pop_back_val();
2692-
if (!BO->hasOneUse() || !BO->hasAllowReassoc() || !BO->hasNoSignedZeros())
2709+
if (!BO->hasOneUse())
26932710
return false;
2694-
BinaryOperator *Op0, *Op1;
2695-
if (match(BO, m_FAdd(m_BinOp(Op0), m_BinOp(Op1)))) {
2696-
Worklist.push_back(Op0);
2697-
Worklist.push_back(Op1);
2711+
if (isReassociableOp(BO, Instruction::Add, Instruction::FAdd) &&
2712+
isa<BinaryOperator>(BO->getOperand(0)) &&
2713+
isa<BinaryOperator>(BO->getOperand(1))) {
2714+
Worklist.push_back(cast<BinaryOperator>(BO->getOperand(0)));
2715+
Worklist.push_back(cast<BinaryOperator>(BO->getOperand(1)));
26982716
continue;
26992717
}
2700-
if (BO->getOpcode() != Instruction::FMul || L.isLoopInvariant(BO))
2718+
if (!isReassociableOp(BO, Instruction::Mul, Instruction::FMul) ||
2719+
L.isLoopInvariant(BO))
27012720
return false;
27022721
Use &U0 = BO->getOperandUse(0);
27032722
Use &U1 = BO->getOperandUse(1);
@@ -2707,7 +2726,10 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
27072726
Changes.push_back(&U1);
27082727
else
27092728
return false;
2710-
if (Changes.size() > FPAssociationUpperLimit)
2729+
unsigned Limit = I.getType()->isIntOrIntVectorTy()
2730+
? IntAssociationUpperLimit
2731+
: FPAssociationUpperLimit;
2732+
if (Changes.size() > Limit)
27112733
return false;
27122734
}
27132735
if (Changes.empty())
@@ -2720,7 +2742,12 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
27202742
for (auto *U : Changes) {
27212743
assert(L.isLoopInvariant(U->get()));
27222744
Instruction *Ins = cast<Instruction>(U->getUser());
2723-
U->set(Builder.CreateFMulFMF(U->get(), Factor, Ins, "factor.op.fmul"));
2745+
Value *Mul;
2746+
if (I.getType()->isIntOrIntVectorTy())
2747+
Mul = Builder.CreateMul(U->get(), Factor, "factor.op.mul");
2748+
else
2749+
Mul = Builder.CreateFMulFMF(U->get(), Factor, Ins, "factor.op.fmul");
2750+
U->set(Mul);
27242751
}
27252752
I.replaceAllUsesWith(VariantOp);
27262753
eraseInstruction(I, SafetyInfo, MSSAU);
@@ -2754,9 +2781,13 @@ static bool hoistArithmetics(Instruction &I, Loop &L,
27542781
return true;
27552782
}
27562783

2757-
if (hoistFPAssociation(I, L, SafetyInfo, MSSAU, AC, DT)) {
2784+
bool IsInt = I.getType()->isIntOrIntVectorTy();
2785+
if (hoistMulAddAssociation(I, L, SafetyInfo, MSSAU, AC, DT)) {
27582786
++NumHoisted;
2759-
++NumFPAssociationsHoisted;
2787+
if (IsInt)
2788+
++NumIntAssociationsHoisted;
2789+
else
2790+
++NumFPAssociationsHoisted;
27602791
return true;
27612792
}
27622793

0 commit comments

Comments
 (0)