Skip to content

[IndVars] Recompute flags if needed in widenIVUse of IV increment. #82352

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.

Sign up for GitHub

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 1 commit into from
Feb 20, 2024
Merged

[IndVars] Recompute flags if needed in widenIVUse of IV increment. #82352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,14 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
bool hoistIVInc(Instruction *IncV, Instruction *InsertPos,
bool RecomputePoisonFlags = false);

/// Return true if both increments directly increment the corresponding IV PHI
/// nodes and have the same opcode. It is not safe to re-use the flags from
/// the original increment, if it is more complex and SCEV expansion may have
/// yielded a more simplified wider increment.
static bool canReuseFlagsFromOriginalIVInc(PHINode *OrigPhi, PHINode *WidePhi,
Instruction *OrigInc,
Instruction *WideInc);

/// replace congruent phis with their most canonical representative. Return
/// the number of phis eliminated.
unsigned replaceCongruentIVs(Loop *L, const DominatorTree *DT,
Expand Down
9 changes: 9 additions & 0 deletions llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,15 @@ bool SCEVExpander::hoistIVInc(Instruction *IncV, Instruction *InsertPos,
return true;
}

bool SCEVExpander::canReuseFlagsFromOriginalIVInc(PHINode *OrigPhi,
PHINode *WidePhi,
Instruction *OrigInc,
Instruction *WideInc) {
return match(OrigInc, m_c_BinOp(m_Specific(OrigPhi), m_Value())) &&
match(WideInc, m_c_BinOp(m_Specific(WidePhi), m_Value())) &&
OrigInc->getOpcode() == WideInc->getOpcode();
}

/// Determine if this cyclic phi is in a form that would have been generated by
/// LSR. We don't care if the phi was actually expanded in this pass, as long
/// as it is in a low-cost form, for example, no implied multiplication. This
Expand Down
30 changes: 19 additions & 11 deletions llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,8 @@ class WidenIV {
const SCEV *getSCEVByOpCode(const SCEV *LHS, const SCEV *RHS,
unsigned OpCode) const;

Instruction *widenIVUse(NarrowIVDefUse DU, SCEVExpander &Rewriter);
Instruction *widenIVUse(NarrowIVDefUse DU, SCEVExpander &Rewriter,
PHINode *OrigPhi, PHINode *WidePhi);

bool widenLoopCompare(NarrowIVDefUse DU);
bool widenWithVariantUse(NarrowIVDefUse DU);
Expand Down Expand Up @@ -1731,7 +1732,9 @@ bool WidenIV::widenWithVariantUse(WidenIV::NarrowIVDefUse DU) {

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

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

// Reuse the IV increment that SCEVExpander created as long as it dominates
// NarrowUse.
// Reuse the IV increment that SCEVExpander created. Recompute flags, unless
// the flags for both increments agree and it is safe to use the ones from
// the original inc. In that case, the new use of the wide increment won't
// be more poisonous.
bool NeedToRecomputeFlags =
!SCEVExpander::canReuseFlagsFromOriginalIVInc(OrigPhi, WidePhi,
DU.NarrowUse, WideInc) ||
DU.NarrowUse->hasNoUnsignedWrap() != WideInc->hasNoUnsignedWrap() ||
DU.NarrowUse->hasNoSignedWrap() != WideInc->hasNoSignedWrap();
Instruction *WideUse = nullptr;
if (WideAddRec.first == WideIncExpr &&
Rewriter.hoistIVInc(WideInc, DU.NarrowUse))
Rewriter.hoistIVInc(WideInc, DU.NarrowUse, NeedToRecomputeFlags))
WideUse = WideInc;
else {
WideUse = cloneIVUser(DU, WideAddRec.first);
Expand Down Expand Up @@ -1996,11 +2006,9 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {
// the same opcode. It is not safe to re-use the flags from the original
// increment, if it is more complex and SCEV expansion may have yielded a
// more simplified wider increment.
bool MatchingOps =
match(OrigInc, m_c_BinOp(m_Specific(OrigPhi), m_Value())) &&
match(WideInc, m_c_BinOp(m_Specific(WidePhi), m_Value())) &&
OrigInc->getOpcode() == WideInc->getOpcode();
if (MatchingOps && isa<OverflowingBinaryOperator>(OrigInc) &&
if (SCEVExpander::canReuseFlagsFromOriginalIVInc(OrigPhi, WidePhi,
OrigInc, WideInc) &&
isa<OverflowingBinaryOperator>(OrigInc) &&
isa<OverflowingBinaryOperator>(WideInc)) {
WideInc->setHasNoUnsignedWrap(WideInc->hasNoUnsignedWrap() ||
OrigInc->hasNoUnsignedWrap());
Expand All @@ -2024,7 +2032,7 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {

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

// Follow all def-use edges from the previous narrow use.
if (WideUse)
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/IndVarSimplify/hoist-wide-inc-for-narrow-use-recompute-flags.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"

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