[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
This commit is contained in:
Zaara Syeda 2022-07-05 12:16:08 -04:00
parent b8dbc6ffea
commit dbf6ab5ef9
5 changed files with 107 additions and 32 deletions

View File

@ -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

View File

@ -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,

View File

@ -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,

View File

@ -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(

View File

@ -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
}