[llvm] dbf6ab5 - [LSR] Fix bug for optimizing unused IVs to final values
Zaara Syeda via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 5 09:31:14 PDT 2022
Author: Zaara Syeda
Date: 2022-07-05T12:30:58-04:00
New Revision: dbf6ab5ef9ae0e1f4706917c2b3f98a67c35826e
URL: https://github.com/llvm/llvm-project/commit/dbf6ab5ef9ae0e1f4706917c2b3f98a67c35826e
DIFF: https://github.com/llvm/llvm-project/commit/dbf6ab5ef9ae0e1f4706917c2b3f98a67c35826e.diff
LOG: [LSR] Fix bug for optimizing unused IVs to final values
This is a fix for a crash reported for https://reviews.llvm.org/D118808
The fix is to only consider PHINodes which are induction phis.
Fixes #55529
Differential Revision: https://reviews.llvm.org/D125990
Added:
Modified:
llvm/include/llvm/Transforms/Utils/LoopUtils.h
llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
llvm/lib/Transforms/Utils/LoopUtils.cpp
llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index 676c0c1487db8..adb39a410b550 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -435,7 +435,13 @@ bool cannotBeMaxInLoop(const SCEV *S, const Loop *L, ScalarEvolution &SE,
bool cannotBeMinInLoop(const SCEV *S, const Loop *L, ScalarEvolution &SE,
bool Signed);
-enum ReplaceExitVal { NeverRepl, OnlyCheapRepl, NoHardUse, AlwaysRepl };
+enum ReplaceExitVal {
+ NeverRepl,
+ OnlyCheapRepl,
+ NoHardUse,
+ UnusedIndVarInLoop,
+ AlwaysRepl
+};
/// If the final value of any expressions that are recurrent in the loop can
/// be computed, substitute the exit values from the loop into any instructions
diff --git a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
index e977dd18be9f9..12cb20fb32e24 100644
--- a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -106,13 +106,18 @@ static cl::opt<bool> VerifyIndvars(
static cl::opt<ReplaceExitVal> ReplaceExitValue(
"replexitval", cl::Hidden, cl::init(OnlyCheapRepl),
cl::desc("Choose the strategy to replace exit value in IndVarSimplify"),
- cl::values(clEnumValN(NeverRepl, "never", "never replace exit value"),
- clEnumValN(OnlyCheapRepl, "cheap",
- "only replace exit value when the cost is cheap"),
- clEnumValN(NoHardUse, "noharduse",
- "only replace exit values when loop def likely dead"),
- clEnumValN(AlwaysRepl, "always",
- "always replace exit value whenever possible")));
+ cl::values(
+ clEnumValN(NeverRepl, "never", "never replace exit value"),
+ clEnumValN(OnlyCheapRepl, "cheap",
+ "only replace exit value when the cost is cheap"),
+ clEnumValN(
+ UnusedIndVarInLoop, "unusedindvarinloop",
+ "only replace exit value when it is an unused "
+ "induction variable in the loop and has cheap replacement cost"),
+ clEnumValN(NoHardUse, "noharduse",
+ "only replace exit values when loop def likely dead"),
+ clEnumValN(AlwaysRepl, "always",
+ "always replace exit value whenever possible")));
static cl::opt<bool> UsePostIncrementRanges(
"indvars-post-increment-ranges", cl::Hidden,
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index 9959e408e2e22..ddf7775a3595b 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5601,27 +5601,6 @@ void LSRInstance::Rewrite(const LSRUse &LU, const LSRFixup &LF,
DeadInsts.emplace_back(OperandIsInstr);
}
-// Check if there are any loop exit values which are only used once within the
-// loop which may potentially be optimized with a call to rewriteLoopExitValue.
-static bool LoopExitValHasSingleUse(Loop *L) {
- BasicBlock *ExitBB = L->getExitBlock();
- if (!ExitBB)
- return false;
-
- for (PHINode &ExitPhi : ExitBB->phis()) {
- if (ExitPhi.getNumIncomingValues() != 1)
- break;
-
- BasicBlock *Pred = ExitPhi.getIncomingBlock(0);
- Value *IVNext = ExitPhi.getIncomingValueForBlock(Pred);
- // One use would be the exit phi node, and there should be only one other
- // use for this to be considered.
- if (IVNext->getNumUses() == 2)
- return true;
- }
- return false;
-}
-
/// Rewrite all the fixup locations with new values, following the chosen
/// solution.
void LSRInstance::ImplementSolution(
@@ -6627,12 +6606,12 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// When this is the case, if the exit value of the IV can be calculated using
// SCEV, we can replace the exit block PHI with the final value of the IV and
// skip the updates in each loop iteration.
- if (L->isRecursivelyLCSSAForm(DT, LI) && LoopExitValHasSingleUse(L)) {
+ if (L->isRecursivelyLCSSAForm(DT, LI) && L->getExitBlock()) {
SmallVector<WeakTrackingVH, 16> DeadInsts;
const DataLayout &DL = L->getHeader()->getModule()->getDataLayout();
SCEVExpander Rewriter(SE, DL, "lsr", false);
int Rewrites = rewriteLoopExitValues(L, &LI, &TLI, &SE, &TTI, Rewriter, &DT,
- OnlyCheapRepl, DeadInsts);
+ UnusedIndVarInLoop, DeadInsts);
if (Rewrites) {
Changed = true;
RecursivelyDeleteTriviallyDeadInstructionsPermissive(DeadInsts, &TLI,
diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp
index ec898c4635749..e088cf829ae20 100644
--- a/llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -1297,6 +1297,53 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
if (!L->contains(Inst))
continue;
+ // Find exit values which are induction variables in the loop, and are
+ // unused in the loop, with the only use being the exit block PhiNode,
+ // and the induction variable update binary operator.
+ // The exit value can be replaced with the final value when it is cheap
+ // to do so.
+ if (ReplaceExitValue == UnusedIndVarInLoop) {
+ InductionDescriptor ID;
+ PHINode *IndPhi = dyn_cast<PHINode>(Inst);
+ if (IndPhi) {
+ if (IndPhi->getParent() != L->getHeader())
+ continue;
+ // Do not consider non induction phis.
+ if (!InductionDescriptor::isInductionPHI(IndPhi, L, SE, ID))
+ continue;
+ // This is an induction PHI. Check that the only users are PHI
+ // nodes, and induction variable update binary operators.
+ if (llvm::any_of(Inst->users(), [&](User *U) {
+ if (!isa<PHINode>(U) && !isa<BinaryOperator>(U))
+ return true;
+ BinaryOperator *B = dyn_cast<BinaryOperator>(U);
+ if (B && B != ID.getInductionBinOp())
+ return true;
+ return false;
+ }))
+ continue;
+ } else {
+ // If it is not an induction phi, it must be an induction update
+ // binary operator with an induction phi user.
+ BinaryOperator *B = dyn_cast<BinaryOperator>(Inst);
+ if (!B)
+ continue;
+ if (llvm::any_of(Inst->users(), [&](User *U) {
+ PHINode *Phi = dyn_cast<PHINode>(U);
+ if (!Phi)
+ return true;
+ if (Phi->getParent() == L->getHeader()) {
+ if (!InductionDescriptor::isInductionPHI(Phi, L, SE, ID))
+ return true;
+ }
+ return false;
+ }))
+ continue;
+ if (B != ID.getInductionBinOp())
+ continue;
+ }
+ }
+
// Okay, this instruction has a user outside of the current loop
// and varies predictably *inside* the loop. Evaluate the value it
// contains when the loop exits, if possible. We prefer to start with
@@ -1362,7 +1409,9 @@ int llvm::rewriteLoopExitValues(Loop *L, LoopInfo *LI, TargetLibraryInfo *TLI,
// Only do the rewrite when the ExitValue can be expanded cheaply.
// If LoopCanBeDel is true, rewrite exit value aggressively.
- if (ReplaceExitValue == OnlyCheapRepl && !LoopCanBeDel && Phi.HighCost)
+ if ((ReplaceExitValue == OnlyCheapRepl ||
+ ReplaceExitValue == UnusedIndVarInLoop) &&
+ !LoopCanBeDel && Phi.HighCost)
continue;
Value *ExitVal = Rewriter.expandCodeFor(
diff --git a/llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll b/llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
index 9a9ef194caea7..37c7d294ad05f 100644
--- a/llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
+++ b/llvm/test/Transforms/LoopStrengthReduce/remove_scev_indvars.ll
@@ -55,3 +55,39 @@ exit:
%indvars.iv.unr = phi i64 [%iv.prol.lcssa, %for.exit]
ret void
}
+
+define void @testNonIndVarPhi() {
+cont5820:
+ br label %for.cond5821
+
+for.cond5821: ; preds = %cont5825, %cont5820
+ %0 = phi i32 [ 0, %cont5825 ], [ 1, %cont5820 ]
+ br label %cont5825
+
+cont5825: ; preds = %for.cond5821
+ br i1 false, label %for.cond5821, label %for.cond6403
+
+for.cond6403: ; preds = %dead, %cont5825
+ %1 = phi i32 [ %.lcssa221, %dead ], [ 0, %cont5825 ]
+ br label %for.cond6418
+
+for.cond6418: ; preds = %cont6497, %for.cond6403
+ %2 = phi i32 [ %0, %cont6497 ], [ %1, %for.cond6403 ]
+ %3 = phi i64 [ 1, %cont6497 ], [ 0, %for.cond6403 ]
+ %cmp6419 = icmp ule i64 %3, 0
+ br i1 %cmp6419, label %cont6497, label %for.end6730
+
+cont6497: ; preds = %for.cond6418
+ %conv6498 = sext i32 %2 to i64
+ br label %for.cond6418
+
+for.end6730: ; preds = %for.cond6418
+; Check that we don't make changes for phis which are not considered
+; induction variables
+; CHECK: %.lcssa221 = phi i32 [ %2, %for.cond6418 ]
+ %.lcssa221 = phi i32 [ %2, %for.cond6418 ]
+ ret void
+
+dead: ; No predecessors!
+ br label %for.cond6403
+}
More information about the llvm-commits
mailing list