Skip to content

Commit

Permalink
Recommit "[LICM] Support integer mul/add in hoistFPAssociation. (#67736
Browse files Browse the repository at this point in the history
…)"

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.
  • Loading branch information
topperc committed Feb 13, 2024
1 parent 91dcf53 commit 2dd5204
Show file tree
Hide file tree
Showing 2 changed files with 414 additions and 19 deletions.
69 changes: 50 additions & 19 deletions llvm/lib/Transforms/Scalar/LICM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ STATISTIC(NumAddSubHoisted, "Number of add/subtract expressions reassociated "
"and hoisted out of the loop");
STATISTIC(NumFPAssociationsHoisted, "Number of invariant FP expressions "
"reassociated and hoisted out of the loop");
STATISTIC(NumIntAssociationsHoisted,
"Number of invariant int expressions "
"reassociated and hoisted out of the loop");

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

cl::opt<unsigned> IntAssociationUpperLimit(
"licm-max-num-int-reassociations", cl::init(5U), cl::Hidden,
cl::desc(
"Set upper limit for the number of transformations performed "
"during a single round of hoisting the reassociated expressions."));

// Experimental option to allow imprecision in LICM in pathological cases, in
// exchange for faster compile. This is to be removed if MemorySSA starts to
// address the same issue. LICM calls MemorySSAWalker's
Expand Down Expand Up @@ -2661,21 +2670,29 @@ static bool hoistAddSub(Instruction &I, Loop &L, ICFLoopSafetyInfo &SafetyInfo,
return false;
}

static bool isReassociableOp(Instruction *I, unsigned IntOpcode,
unsigned FPOpcode) {
if (I->getOpcode() == IntOpcode)
return true;
if (I->getOpcode() == FPOpcode && I->hasAllowReassoc() &&
I->hasNoSignedZeros())
return true;
return false;
}

/// Try to reassociate expressions like ((A1 * B1) + (A2 * B2) + ...) * C where
/// A1, A2, ... and C are loop invariants into expressions like
/// ((A1 * C * B1) + (A2 * C * B2) + ...) and hoist the (A1 * C), (A2 * C), ...
/// invariant expressions. This functions returns true only if any hoisting has
/// actually occured.
static bool hoistFPAssociation(Instruction &I, Loop &L,
ICFLoopSafetyInfo &SafetyInfo,
MemorySSAUpdater &MSSAU, AssumptionCache *AC,
DominatorTree *DT) {
using namespace PatternMatch;
Value *VariantOp = nullptr, *InvariantOp = nullptr;

if (!match(&I, m_FMul(m_Value(VariantOp), m_Value(InvariantOp))) ||
!I.hasAllowReassoc() || !I.hasNoSignedZeros())
static bool hoistMulAddAssociation(Instruction &I, Loop &L,
ICFLoopSafetyInfo &SafetyInfo,
MemorySSAUpdater &MSSAU, AssumptionCache *AC,
DominatorTree *DT) {
if (!isReassociableOp(&I, Instruction::Mul, Instruction::FMul))
return false;
Value *VariantOp = I.getOperand(0);
Value *InvariantOp = I.getOperand(1);
if (L.isLoopInvariant(VariantOp))
std::swap(VariantOp, InvariantOp);
if (L.isLoopInvariant(VariantOp) || !L.isLoopInvariant(InvariantOp))
Expand All @@ -2689,15 +2706,17 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
Worklist.push_back(VariantBinOp);
while (!Worklist.empty()) {
BinaryOperator *BO = Worklist.pop_back_val();
if (!BO->hasOneUse() || !BO->hasAllowReassoc() || !BO->hasNoSignedZeros())
if (!BO->hasOneUse())
return false;
BinaryOperator *Op0, *Op1;
if (match(BO, m_FAdd(m_BinOp(Op0), m_BinOp(Op1)))) {
Worklist.push_back(Op0);
Worklist.push_back(Op1);
if (isReassociableOp(BO, Instruction::Add, Instruction::FAdd) &&
isa<BinaryOperator>(BO->getOperand(0)) &&
isa<BinaryOperator>(BO->getOperand(1))) {
Worklist.push_back(cast<BinaryOperator>(BO->getOperand(0)));
Worklist.push_back(cast<BinaryOperator>(BO->getOperand(1)));
continue;
}
if (BO->getOpcode() != Instruction::FMul || L.isLoopInvariant(BO))
if (!isReassociableOp(BO, Instruction::Mul, Instruction::FMul) ||
L.isLoopInvariant(BO))
return false;
Use &U0 = BO->getOperandUse(0);
Use &U1 = BO->getOperandUse(1);
Expand All @@ -2707,7 +2726,10 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
Changes.push_back(&U1);
else
return false;
if (Changes.size() > FPAssociationUpperLimit)
unsigned Limit = I.getType()->isIntOrIntVectorTy()
? IntAssociationUpperLimit
: FPAssociationUpperLimit;
if (Changes.size() > Limit)
return false;
}
if (Changes.empty())
Expand All @@ -2720,7 +2742,12 @@ static bool hoistFPAssociation(Instruction &I, Loop &L,
for (auto *U : Changes) {
assert(L.isLoopInvariant(U->get()));
Instruction *Ins = cast<Instruction>(U->getUser());
U->set(Builder.CreateFMulFMF(U->get(), Factor, Ins, "factor.op.fmul"));
Value *Mul;
if (I.getType()->isIntOrIntVectorTy())
Mul = Builder.CreateMul(U->get(), Factor, "factor.op.mul");
else
Mul = Builder.CreateFMulFMF(U->get(), Factor, Ins, "factor.op.fmul");
U->set(Mul);
}
I.replaceAllUsesWith(VariantOp);
eraseInstruction(I, SafetyInfo, MSSAU);
Expand Down Expand Up @@ -2754,9 +2781,13 @@ static bool hoistArithmetics(Instruction &I, Loop &L,
return true;
}

if (hoistFPAssociation(I, L, SafetyInfo, MSSAU, AC, DT)) {
bool IsInt = I.getType()->isIntOrIntVectorTy();
if (hoistMulAddAssociation(I, L, SafetyInfo, MSSAU, AC, DT)) {
++NumHoisted;
++NumFPAssociationsHoisted;
if (IsInt)
++NumIntAssociationsHoisted;
else
++NumFPAssociationsHoisted;
return true;
}

Expand Down
Loading

0 comments on commit 2dd5204

Please sign in to comment.