Skip to content

Commit 4db93e5

Browse files
fhahnfhahn
authored
[IndVars] Recompute flags if needed in widenIVUse of IV increment. (#82352)
widenIVUse may hoist a wide induction increment and introduce new uses, but does not recompute the wrap flags. In some cases this can make the new uses of the wide IV inc more poisonous. Update the code to recompute flags if needed when hoisting an IV. If both the narrow and wide IV increment's flags match and we can re-use the flags from the increments, there's no need to recompute the flags, as the replacement won't make the new uses of the wide IV's increment more poisonous. Note that this also updates a stale comment which claimed that the widen increment is only used if it dominates the new use. The helper should also be used to guard the code added in da43733, which I am planning on doing separately once the helper lands. Fixes #82243.
1 parent fcd6549 commit 4db93e5

File tree

4 files changed

+38
-13
lines changed

4 files changed

+38
-13
lines changed

‎llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,14 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
258258
bool hoistIVInc(Instruction *IncV, Instruction *InsertPos,
259259
bool RecomputePoisonFlags = false);
260260

261+
/// Return true if both increments directly increment the corresponding IV PHI
262+
/// nodes and have the same opcode. It is not safe to re-use the flags from
263+
/// the original increment, if it is more complex and SCEV expansion may have
264+
/// yielded a more simplified wider increment.
265+
static bool canReuseFlagsFromOriginalIVInc(PHINode *OrigPhi, PHINode *WidePhi,
266+
Instruction *OrigInc,
267+
Instruction *WideInc);
268+
261269
/// replace congruent phis with their most canonical representative. Return
262270
/// the number of phis eliminated.
263271
unsigned replaceCongruentIVs(Loop *L, const DominatorTree *DT,

‎llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,15 @@ bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
771771
return true;
772772
}
773773

774+
bool SCEVExpander::canReuseFlagsFromOriginalIVInc(PHINode *OrigPhi,
775+
PHINode *WidePhi,
776+
Instruction *OrigInc,
777+
Instruction *WideInc) {
778+
return match(OrigInc, m_c_BinOp(m_Specific(OrigPhi), m_Value())) &&
779+
match(WideInc, m_c_BinOp(m_Specific(WidePhi), m_Value())) &&
780+
OrigInc->getOpcode() == WideInc->getOpcode();
781+
}
782+
774783
/// Determine if this cyclic phi is in a form that would have been generated by
775784
/// LSR. We don't care if the phi was actually expanded in this pass, as long
776785
/// as it is in a low-cost form, for example, no implied multiplication. This

‎llvm/lib/Transforms/Utils/SimplifyIndVar.cpp

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,7 +1131,8 @@ class WidenIV {
11311131
const SCEV *getSCEVByOpCode(const SCEV *LHS, const SCEV *RHS,
11321132
unsigned OpCode) const;
11331133

1134-
Instruction *widenIVUse(NarrowIVDefUse DU, SCEVExpander &Rewriter);
1134+
Instruction *widenIVUse(NarrowIVDefUse DU, SCEVExpander &Rewriter,
1135+
PHINode *OrigPhi, PHINode *WidePhi);
11351136

11361137
bool widenLoopCompare(NarrowIVDefUse DU);
11371138
bool widenWithVariantUse(NarrowIVDefUse DU);
@@ -1731,7 +1732,9 @@ bool WidenIV::widenWithVariantUse(WidenIV::NarrowIVDefUse DU) {
17311732

17321733
/// Determine whether an individual user of the narrow IV can be widened. If so,
17331734
/// return the wide clone of the user.
1734-
Instruction *WidenIV::widenIVUse(WidenIV::NarrowIVDefUse DU, SCEVExpander &Rewriter) {
1735+
Instruction *WidenIV::widenIVUse(WidenIV::NarrowIVDefUse DU,
1736+
SCEVExpander &Rewriter, PHINode *OrigPhi,
1737+
PHINode *WidePhi) {
17351738
assert(ExtendKindMap.count(DU.NarrowDef) &&
17361739
"Should already know the kind of extension used to widen NarrowDef");
17371740

@@ -1825,11 +1828,18 @@ Instruction *WidenIV::widenIVUse(WidenIV::NarrowIVDefUse DU, SCEVExpander &Rewri
18251828
if (!WideAddRec.first)
18261829
return nullptr;
18271830

1828-
// Reuse the IV increment that SCEVExpander created as long as it dominates
1829-
// NarrowUse.
1831+
// Reuse the IV increment that SCEVExpander created. Recompute flags, unless
1832+
// the flags for both increments agree and it is safe to use the ones from
1833+
// the original inc. In that case, the new use of the wide increment won't
1834+
// be more poisonous.
1835+
bool NeedToRecomputeFlags =
1836+
!SCEVExpander::canReuseFlagsFromOriginalIVInc(OrigPhi, WidePhi,
1837+
DU.NarrowUse, WideInc) ||
1838+
DU.NarrowUse->hasNoUnsignedWrap() != WideInc->hasNoUnsignedWrap() ||
1839+
DU.NarrowUse->hasNoSignedWrap() != WideInc->hasNoSignedWrap();
18301840
Instruction *WideUse = nullptr;
18311841
if (WideAddRec.first == WideIncExpr &&
1832-
Rewriter.hoistIVInc(WideInc, DU.NarrowUse))
1842+
Rewriter.hoistIVInc(WideInc, DU.NarrowUse, NeedToRecomputeFlags))
18331843
WideUse = WideInc;
18341844
else {
18351845
WideUse = cloneIVUser(DU, WideAddRec.first);
@@ -1996,11 +2006,9 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {
19962006
// the same opcode. It is not safe to re-use the flags from the original
19972007
// increment, if it is more complex and SCEV expansion may have yielded a
19982008
// more simplified wider increment.
1999-
bool MatchingOps =
2000-
match(OrigInc, m_c_BinOp(m_Specific(OrigPhi), m_Value())) &&
2001-
match(WideInc, m_c_BinOp(m_Specific(WidePhi), m_Value())) &&
2002-
OrigInc->getOpcode() == WideInc->getOpcode();
2003-
if (MatchingOps && isa<OverflowingBinaryOperator>(OrigInc) &&
2009+
if (SCEVExpander::canReuseFlagsFromOriginalIVInc(OrigPhi, WidePhi,
2010+
OrigInc, WideInc) &&
2011+
isa<OverflowingBinaryOperator>(OrigInc) &&
20042012
isa<OverflowingBinaryOperator>(WideInc)) {
20052013
WideInc->setHasNoUnsignedWrap(WideInc->hasNoUnsignedWrap() ||
20062014
OrigInc->hasNoUnsignedWrap());
@@ -2024,7 +2032,7 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {
20242032

20252033
// Process a def-use edge. This may replace the use, so don't hold a
20262034
// use_iterator across it.
2027-
Instruction *WideUse = widenIVUse(DU, Rewriter);
2035+
Instruction *WideUse = widenIVUse(DU, Rewriter, OrigPhi, WidePhi);
20282036

20292037
// Follow all def-use edges from the previous narrow use.
20302038
if (WideUse)

‎llvm/test/Transforms/IndVarSimplify/hoist-wide-inc-for-narrow-use-recompute-flags.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
55

66
; Test for https://github.com/llvm/llvm-project/issues/82243.
7-
; FIXME: Currently no-wrap flags on hoisted wide IV are not dropped properly.
7+
; Check that NUW flag on hoisted wide IV is dropped properly.
88
define void @test_pr82243(ptr %f) {
99
; CHECK-LABEL: define void @test_pr82243(
1010
; CHECK-SAME: ptr [[F:%.*]]) {
@@ -14,7 +14,7 @@ define void @test_pr82243(ptr %f) {
1414
; CHECK-NEXT: [[INDVARS_IV:%.*]] = phi i64 [ [[INDVARS_IV_NEXT:%.*]], [[OUTER_LATCH:%.*]] ], [ 1, [[ENTRY:%.*]] ]
1515
; CHECK-NEXT: [[GEP_IV_EXT:%.*]] = getelementptr i32, ptr [[F]], i64 [[INDVARS_IV]]
1616
; CHECK-NEXT: store i32 1, ptr [[GEP_IV_EXT]], align 4
17-
; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], -1
17+
; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add nsw i64 [[INDVARS_IV]], -1
1818
; CHECK-NEXT: [[TMP0:%.*]] = trunc i64 [[INDVARS_IV_NEXT]] to i32
1919
; CHECK-NEXT: [[SHL:%.*]] = shl i32 123, [[TMP0]]
2020
; CHECK-NEXT: [[GEP_SHL:%.*]] = getelementptr i32, ptr [[F]], i32 [[SHL]]

0 commit comments

Comments
 (0)