Skip to content

[LICM] Support integer mul/add in hoistFPAssociation. #67736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 49 additions & 17 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."));

Comment on lines +141 to +146
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the type relevant to the limit? I'm not against the change just wondering if this is a compile time reduction flag whether it's worth just renaming the current one.

// 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,31 @@ 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) {
static bool hoistMulAddAssociation(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())
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 +2708,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 +2728,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 +2744,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 +2783,12 @@ static bool hoistArithmetics(Instruction &I, Loop &L,
return true;
}

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

Expand Down
Loading