[llvm] [SCEVExpander] Relax hoisting condition for AddRec start (PR #75916)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 2 10:32:58 PST 2024
https://github.com/averine updated https://github.com/llvm/llvm-project/pull/75916
>From 3350c5ec136b406a717d00b2482ddf8cba478e6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20V=C3=A9rine?= <“verine.alexandre at proton.me”>
Date: Tue, 19 Dec 2023 10:23:08 +0100
Subject: [PATCH 1/4] [SCEVExpander] Relax hoisting condition for AddRec start
Don't require the existence of a preheader to hoist an
AddRec start value: a unique loop predecessor which
isLegalToHoistInto should be enough.
---
.../Utils/ScalarEvolutionExpander.cpp | 13 ++++---
.../scevexpander-in-non-simplified-loop.ll | 39 +++++++++++++++++++
2 files changed, 47 insertions(+), 5 deletions(-)
create mode 100644 llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index cd3ac317cd238e..f4be4c94925e62 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -980,11 +980,14 @@ SCEVExpander::getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
PostIncLoopSet SavedPostIncLoops = PostIncLoops;
PostIncLoops.clear();
- // Expand code for the start value into the loop preheader.
- assert(L->getLoopPreheader() &&
- "Can't expand add recurrences without a loop preheader!");
- Value *StartV =
- expand(Normalized->getStart(), L->getLoopPreheader()->getTerminator());
+ // Expand code for the start value into the loop predecessor. The loop is not
+ // necessarily in Loop Simplify Form, so assert it's legal to do so.
+ auto *LP = L->getLoopPredecessor();
+ assert(LP && LP->isLegalToHoistInto() &&
+ "Can't expand add recurrences without hoisting into the loop "
+ "predecessor.");
+
+ Value *StartV = expand(Normalized->getStart(), LP->getTerminator());
// StartV must have been be inserted into L's preheader to dominate the new
// phi.
diff --git a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
new file mode 100644
index 00000000000000..7b546ab7d5d329
--- /dev/null
+++ b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
@@ -0,0 +1,39 @@
+; RUN: opt -passes=indvars < %s
+
+target datalayout = "e-n32:64"
+
+declare void @g(i64)
+
+define void @f(ptr %block_address_arg) {
+
+; This loop isn't in Simplified Form. Ensure SCEV expander doesn't hit an assert
+; when visiting it.
+f_entry:
+ indirectbr ptr %block_address_arg, [label %f.exit, label %f_for.cond]
+
+f_for.cond: ; preds = %f_for.body, %f_entry
+ %i = phi i32 [ %j, %f_for.body ], [ 0, %f_entry ]
+ %cmp = icmp ult i32 %i, 64
+ br i1 %cmp, label %f_for.body, label %f_for.end
+
+f_for.body: ; preds = %f_for.cond
+ %j = add nuw nsw i32 %i, 1
+ br label %f_for.cond
+
+; Indvars pass visits this loop, since it's in Simplified Form.
+; Because of the integer extension in that loop's body, SCEV expander may also
+; visit the first loop.
+f_for.end: ; preds = %f_for.cond
+ %k = phi i32 [ %i, %f_for.cond ]
+ br label %f_for2.body
+
+f_for2.body: ; preds = %f_for2.body, %f_for.end
+ %l = phi i32 [ %k, %f_for.end ], [ %n, %f_for2.body ]
+ %m = zext i32 %l to i64
+ call void @g(i64 %m)
+ %n = add nuw nsw i32 %l, 1
+ br label %f_for2.body
+
+f.exit: ; preds = %f_entry
+ ret void
+}
>From abc6770fe20bba60beaa8dd0d9912142e568366f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20V=C3=A9rine?= <“verine.alexandre at proton.me”>
Date: Fri, 29 Dec 2023 14:11:22 +0100
Subject: [PATCH 2/4] Revert "[SCEVExpander] Relax hoisting condition for
AddRec start"
This reverts commit 3350c5ec136b406a717d00b2482ddf8cba478e6d.
---
.../Utils/ScalarEvolutionExpander.cpp | 13 +++----
.../scevexpander-in-non-simplified-loop.ll | 39 -------------------
2 files changed, 5 insertions(+), 47 deletions(-)
delete mode 100644 llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index f4be4c94925e62..cd3ac317cd238e 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -980,14 +980,11 @@ SCEVExpander::getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
PostIncLoopSet SavedPostIncLoops = PostIncLoops;
PostIncLoops.clear();
- // Expand code for the start value into the loop predecessor. The loop is not
- // necessarily in Loop Simplify Form, so assert it's legal to do so.
- auto *LP = L->getLoopPredecessor();
- assert(LP && LP->isLegalToHoistInto() &&
- "Can't expand add recurrences without hoisting into the loop "
- "predecessor.");
-
- Value *StartV = expand(Normalized->getStart(), LP->getTerminator());
+ // Expand code for the start value into the loop preheader.
+ assert(L->getLoopPreheader() &&
+ "Can't expand add recurrences without a loop preheader!");
+ Value *StartV =
+ expand(Normalized->getStart(), L->getLoopPreheader()->getTerminator());
// StartV must have been be inserted into L's preheader to dominate the new
// phi.
diff --git a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
deleted file mode 100644
index 7b546ab7d5d329..00000000000000
--- a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
+++ /dev/null
@@ -1,39 +0,0 @@
-; RUN: opt -passes=indvars < %s
-
-target datalayout = "e-n32:64"
-
-declare void @g(i64)
-
-define void @f(ptr %block_address_arg) {
-
-; This loop isn't in Simplified Form. Ensure SCEV expander doesn't hit an assert
-; when visiting it.
-f_entry:
- indirectbr ptr %block_address_arg, [label %f.exit, label %f_for.cond]
-
-f_for.cond: ; preds = %f_for.body, %f_entry
- %i = phi i32 [ %j, %f_for.body ], [ 0, %f_entry ]
- %cmp = icmp ult i32 %i, 64
- br i1 %cmp, label %f_for.body, label %f_for.end
-
-f_for.body: ; preds = %f_for.cond
- %j = add nuw nsw i32 %i, 1
- br label %f_for.cond
-
-; Indvars pass visits this loop, since it's in Simplified Form.
-; Because of the integer extension in that loop's body, SCEV expander may also
-; visit the first loop.
-f_for.end: ; preds = %f_for.cond
- %k = phi i32 [ %i, %f_for.cond ]
- br label %f_for2.body
-
-f_for2.body: ; preds = %f_for2.body, %f_for.end
- %l = phi i32 [ %k, %f_for.end ], [ %n, %f_for2.body ]
- %m = zext i32 %l to i64
- call void @g(i64 %m)
- %n = add nuw nsw i32 %l, 1
- br label %f_for2.body
-
-f.exit: ; preds = %f_entry
- ret void
-}
>From ae053156d2837d6c83db2c78c42c855ecce760d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20V=C3=A9rine?= <“verine.alexandre at proton.me”>
Date: Fri, 29 Dec 2023 14:47:31 +0100
Subject: [PATCH 3/4] Address reviewers' comments: add a check before expansion
This canExpand() check verifies addrecs have a preheader
in non-canonical mode.
---
.../Utils/ScalarEvolutionExpander.h | 3 ++
.../Utils/ScalarEvolutionExpander.cpp | 25 ++++++++----
llvm/lib/Transforms/Utils/SimplifyIndVar.cpp | 4 ++
.../scevexpander-in-non-simplified-loop.ll | 39 +++++++++++++++++++
4 files changed, 64 insertions(+), 7 deletions(-)
create mode 100644 llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
index 92dbe268c8fa49..6f991274c81147 100644
--- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
+++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h
@@ -269,6 +269,9 @@ class SCEVExpander : public SCEVVisitor<SCEVExpander, Value *> {
/// defined, and the expander is capable of expanding the expression.
bool isSafeToExpand(const SCEV *S) const;
+ /// Return true if the expander is capable of expanding the expression.
+ bool canExpand(const SCEV *S) const;
+
/// Return true if the given expression is safe to expand in the sense that
/// all materialized values are defined and safe to speculate at the specified
/// location and their operands are defined at this location.
diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
index cd3ac317cd238e..32fc55e2d2d5df 100644
--- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
+++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
@@ -2276,15 +2276,20 @@ struct SCEVFindUnsafe {
ScalarEvolution &SE;
bool CanonicalMode;
bool IsUnsafe = false;
+ bool OnlyCheckPreheader;
- SCEVFindUnsafe(ScalarEvolution &SE, bool CanonicalMode)
- : SE(SE), CanonicalMode(CanonicalMode) {}
+ SCEVFindUnsafe(ScalarEvolution &SE, bool CanonicalMode,
+ bool OnlyCheckPreheader)
+ : SE(SE), CanonicalMode(CanonicalMode),
+ OnlyCheckPreheader(OnlyCheckPreheader) {}
bool follow(const SCEV *S) {
- if (const SCEVUDivExpr *D = dyn_cast<SCEVUDivExpr>(S)) {
- if (!SE.isKnownNonZero(D->getRHS())) {
- IsUnsafe = true;
- return false;
+ if (!OnlyCheckPreheader) {
+ if (const SCEVUDivExpr *D = dyn_cast<SCEVUDivExpr>(S)) {
+ if (!SE.isKnownNonZero(D->getRHS())) {
+ IsUnsafe = true;
+ return false;
+ }
}
}
if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(S)) {
@@ -2303,7 +2308,13 @@ struct SCEVFindUnsafe {
} // namespace
bool SCEVExpander::isSafeToExpand(const SCEV *S) const {
- SCEVFindUnsafe Search(SE, CanonicalMode);
+ SCEVFindUnsafe Search(SE, CanonicalMode, false);
+ visitAll(S, Search);
+ return !Search.IsUnsafe;
+}
+
+bool SCEVExpander::canExpand(const SCEV *S) const {
+ SCEVFindUnsafe Search(SE, CanonicalMode, true);
visitAll(S, Search);
return !Search.IsUnsafe;
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index 722ed03db3de30..5809c25ab10bbb 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -1920,6 +1920,9 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {
if (!AddRec || AddRec->getLoop() != L)
return nullptr;
+ if (!Rewriter.canExpand(AddRec))
+ return nullptr;
+
// An AddRec must have loop-invariant operands. Since this AddRec is
// materialized by a loop header phi, the expression cannot have any post-loop
// operands, so they must dominate the loop header.
@@ -1946,6 +1949,7 @@ PHINode *WidenIV::createWideIV(SCEVExpander &Rewriter) {
// expect a well-formed cyclic phi-with-increments. i.e. any operand not part
// of the phi-SCC dominates the loop entry.
Instruction *InsertPt = &*L->getHeader()->getFirstInsertionPt();
+
Value *ExpandInst = Rewriter.expandCodeFor(AddRec, WideType, InsertPt);
// If the wide phi is not a phi node, for example a cast node, like bitcast,
// inttoptr, ptrtoint, just skip for now.
diff --git a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
new file mode 100644
index 00000000000000..6599208ee8bbdc
--- /dev/null
+++ b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
@@ -0,0 +1,39 @@
+; RUN: opt --passes=indvars -disable-output < %s
+
+target datalayout = "e-n32:64"
+
+declare void @g(i64)
+
+define void @f(ptr %block_address_arg) {
+
+; This loop isn't in Simplified Form. Ensure that there's no SCEV
+; expansion, which would cause a crash.
+f_entry:
+ indirectbr ptr %block_address_arg, [label %f.exit, label %f_for.cond]
+
+f_for.cond: ; preds = %f_for.body, %f_entry
+ %i = phi i32 [ %j, %f_for.body ], [ 0, %f_entry ]
+ %cmp = icmp ult i32 %i, 64
+ br i1 %cmp, label %f_for.body, label %f_for.end
+
+f_for.body: ; preds = %f_for.cond
+ %j = add nuw nsw i32 %i, 1
+ br label %f_for.cond
+
+; Indvars pass visits this loop, since it's in Simplified Form.
+; Because of the integer extension in that loop's body, SCEV expander may also
+; visit the first loop.
+f_for.end: ; preds = %f_for.cond
+ %k = phi i32 [ %i, %f_for.cond ]
+ br label %f_for2.body
+
+f_for2.body: ; preds = %f_for2.body, %f_for.end
+ %l = phi i32 [ %k, %f_for.end ], [ %n, %f_for2.body ]
+ %m = zext i32 %l to i64
+ call void @g(i64 %m)
+ %n = add nuw nsw i32 %l, 1
+ br label %f_for2.body
+
+f.exit: ; preds = %f_entry
+ ret void
+}
>From 9b5fc2021d5b52e0ae952cbde121c8862a571c5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alexandre=20V=C3=A9rine?= <“verine.alexandre at proton.me”>
Date: Tue, 2 Jan 2024 19:13:59 +0100
Subject: [PATCH 4/4] Address reviewer's comment: add autogenerated CHECKs
---
.../scevexpander-in-non-simplified-loop.ll | 27 +++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
index 6599208ee8bbdc..5b6894f62f2013 100644
--- a/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
+++ b/llvm/test/Transforms/IndVarSimplify/scevexpander-in-non-simplified-loop.ll
@@ -1,13 +1,36 @@
-; RUN: opt --passes=indvars -disable-output < %s
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt --passes=indvars -S < %s | FileCheck %s
target datalayout = "e-n32:64"
declare void @g(i64)
define void @f(ptr %block_address_arg) {
-
; This loop isn't in Simplified Form. Ensure that there's no SCEV
; expansion, which would cause a crash.
+; CHECK-LABEL: define void @f(
+; CHECK-SAME: ptr [[BLOCK_ADDRESS_ARG:%.*]]) {
+; CHECK-NEXT: f_entry:
+; CHECK-NEXT: indirectbr ptr [[BLOCK_ADDRESS_ARG]], [label [[F_EXIT:%.*]], label %f_for.cond]
+; CHECK: f_for.cond:
+; CHECK-NEXT: [[I:%.*]] = phi i32 [ [[J:%.*]], [[F_FOR_BODY:%.*]] ], [ 0, [[F_ENTRY:%.*]] ]
+; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[I]], 64
+; CHECK-NEXT: br i1 [[CMP]], label [[F_FOR_BODY]], label [[F_FOR_END:%.*]]
+; CHECK: f_for.body:
+; CHECK-NEXT: [[J]] = add nuw nsw i32 [[I]], 1
+; CHECK-NEXT: br label [[F_FOR_COND:%.*]]
+; CHECK: f_for.end:
+; CHECK-NEXT: [[K:%.*]] = phi i32 [ [[I]], [[F_FOR_COND]] ]
+; CHECK-NEXT: br label [[F_FOR2_BODY:%.*]]
+; CHECK: f_for2.body:
+; CHECK-NEXT: [[L:%.*]] = phi i32 [ [[K]], [[F_FOR_END]] ], [ [[N:%.*]], [[F_FOR2_BODY]] ]
+; CHECK-NEXT: [[M:%.*]] = zext i32 [[L]] to i64
+; CHECK-NEXT: call void @g(i64 [[M]])
+; CHECK-NEXT: [[N]] = add nuw nsw i32 [[L]], 1
+; CHECK-NEXT: br label [[F_FOR2_BODY]]
+; CHECK: f.exit:
+; CHECK-NEXT: ret void
+;
f_entry:
indirectbr ptr %block_address_arg, [label %f.exit, label %f_for.cond]
More information about the llvm-commits
mailing list