[llvm] 27f8a62 - [LoopPredication] Fix where we generate widened condition. PR61963
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 10 07:37:20 PDT 2023
Author: Anna Thomas
Date: 2023-04-10T10:37:05-04:00
New Revision: 27f8a62a541e650d8d6ee4b7001b78e976519306
URL: https://github.com/llvm/llvm-project/commit/27f8a62a541e650d8d6ee4b7001b78e976519306
DIFF: https://github.com/llvm/llvm-project/commit/27f8a62a541e650d8d6ee4b7001b78e976519306.diff
LOG: [LoopPredication] Fix where we generate widened condition. PR61963
Loop predication's predicateLoopExit pass does two incorrect things:
It sinks the widenable call into the loop, thereby converting an invariant condition to a variant one
It widens the widenable call at a branch thereby converting the branch into a loop-varying one.
The latter is problematic when the branch may have been loop-invariant
and prior optimizations (such as indvars) may have relied on this
fact, and updated the deopt state accordingly.
Now, when we widen this with a loop-varying condition, the deopt state
is no longer correct.
https://github.com/llvm/llvm-project/issues/61963 fixed.
Differential Revision: https://reviews.llvm.org/D147662
Added:
Modified:
llvm/lib/Transforms/Scalar/LoopPredication.cpp
llvm/test/Transforms/LoopPredication/pr61963.ll
llvm/test/Transforms/LoopPredication/predicate-exits.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/LoopPredication.cpp b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
index a52a0051b2d9c..12852ae5c4608 100644
--- a/llvm/lib/Transforms/Scalar/LoopPredication.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopPredication.cpp
@@ -1175,6 +1175,11 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
if (ChangedLoop)
SE->forgetLoop(L);
+ // The insertion point for the widening should be at the widenably call, not
+ // at the WidenableBR. If we do this at the widenableBR, we can incorrectly
+ // change a loop-invariant condition to a loop-varying one.
+ auto *IP = cast<Instruction>(WidenableBR->getCondition());
+
// The use of umin(all analyzeable exits) instead of latch is subtle, but
// important for profitability. We may have a loop which hasn't been fully
// canonicalized just yet. If the exit we chose to widen is provably never
@@ -1184,21 +1189,9 @@ bool LoopPredication::predicateLoopExits(Loop *L, SCEVExpander &Rewriter) {
const SCEV *MinEC = getMinAnalyzeableBackedgeTakenCount(*SE, *DT, L);
if (isa<SCEVCouldNotCompute>(MinEC) || MinEC->getType()->isPointerTy() ||
!SE->isLoopInvariant(MinEC, L) ||
- !Rewriter.isSafeToExpandAt(MinEC, WidenableBR))
+ !Rewriter.isSafeToExpandAt(MinEC, IP))
return ChangedLoop;
- // Subtlety: We need to avoid inserting additional uses of the WC. We know
- // that it can only have one transitive use at the moment, and thus moving
- // that use to just before the branch and inserting code before it and then
- // modifying the operand is legal.
- auto *IP = cast<Instruction>(WidenableBR->getCondition());
- // Here we unconditionally modify the IR, so after this point we should return
- // only `true`!
- IP->moveBefore(WidenableBR);
- if (MSSAU)
- if (auto *MUD = MSSAU->getMemorySSA()->getMemoryAccess(IP))
- MSSAU->moveToPlace(MUD, WidenableBR->getParent(),
- MemorySSA::BeforeTerminator);
Rewriter.setInsertPoint(IP);
IRBuilder<> B(IP);
diff --git a/llvm/test/Transforms/LoopPredication/pr61963.ll b/llvm/test/Transforms/LoopPredication/pr61963.ll
index dc7fe347031c2..cec5e1b766fcd 100644
--- a/llvm/test/Transforms/LoopPredication/pr61963.ll
+++ b/llvm/test/Transforms/LoopPredication/pr61963.ll
@@ -12,19 +12,14 @@ define i32 @foo(ptr addrspace(1) %arg) {
; CHECK-SAME: (ptr addrspace(1) [[ARG:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[INIT_VAL:%.*]] = load i32, ptr addrspace(1) [[ARG]], align 4
+; CHECK-NEXT: [[WIDENABLE_COND11:%.*]] = call i1 @llvm.experimental.widenable.condition()
; CHECK-NEXT: br label [[LOOP_OUTER:%.*]]
; CHECK: loop_outer:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ [[PHI36:%.*]], [[OUTER_LOOP_LATCH:%.*]] ], [ 42, [[ENTRY:%.*]] ]
; CHECK-NEXT: [[PHI21:%.*]] = phi i32 [ [[ADD39:%.*]], [[OUTER_LOOP_LATCH]] ], [ [[INIT_VAL]], [[ENTRY]] ]
; CHECK-NEXT: [[ADD27:%.*]] = add i32 [[IV]], 1
; CHECK-NEXT: [[ICMP28:%.*]] = icmp eq i32 [[ADD27]], 60
-; CHECK-NEXT: [[TMP0:%.*]] = sub i32 -2, [[IV]]
-; CHECK-NEXT: [[UMIN:%.*]] = call i32 @llvm.umin.i32(i32 [[TMP0]], i32 8)
-; CHECK-NEXT: [[TMP1:%.*]] = icmp ugt i32 [[TMP0]], [[UMIN]]
-; CHECK-NEXT: [[TMP2:%.*]] = freeze i1 [[TMP1]]
-; CHECK-NEXT: [[WIDENABLE_COND11:%.*]] = call i1 @llvm.experimental.widenable.condition()
-; CHECK-NEXT: [[TMP3:%.*]] = and i1 [[TMP2]], [[WIDENABLE_COND11]]
-; CHECK-NEXT: br i1 [[TMP3]], label [[INNER_LOOP_PH:%.*]], label [[DEOPT9:%.*]]
+; CHECK-NEXT: br i1 [[WIDENABLE_COND11]], label [[INNER_LOOP_PH:%.*]], label [[DEOPT9:%.*]]
; CHECK: inner_loop_ph:
; CHECK-NEXT: store atomic i32 606, ptr addrspace(1) [[ARG]] unordered, align 4
; CHECK-NEXT: br label [[INNER_LOOP:%.*]]
@@ -33,7 +28,7 @@ define i32 @foo(ptr addrspace(1) %arg) {
; CHECK-NEXT: [[PHI44:%.*]] = phi i32 [ [[ADD27]], [[INNER_LOOP_PH]] ], [ [[ADD48:%.*]], [[INNER_LOOP_LATCH]] ]
; CHECK-NEXT: [[ADD48]] = add i32 [[PHI44]], 1
; CHECK-NEXT: [[ICMP49:%.*]] = icmp eq i32 [[ADD48]], 0
-; CHECK-NEXT: br i1 false, label [[DEOPT57:%.*]], label [[INNER_LOOP_LATCH]]
+; CHECK-NEXT: br i1 [[ICMP49]], label [[DEOPT57:%.*]], label [[INNER_LOOP_LATCH]]
; CHECK: inner_loop_latch:
; CHECK-NEXT: store atomic i32 606, ptr addrspace(1) [[ARG]] unordered, align 4
; CHECK-NEXT: [[ADD55]] = add nuw nsw i32 [[PHI43]], 1
diff --git a/llvm/test/Transforms/LoopPredication/predicate-exits.ll b/llvm/test/Transforms/LoopPredication/predicate-exits.ll
index e41f0efc75b7d..470ae3bdcac02 100644
--- a/llvm/test/Transforms/LoopPredication/predicate-exits.ll
+++ b/llvm/test/Transforms/LoopPredication/predicate-exits.ll
@@ -1085,8 +1085,8 @@ exit:
define void @test_memssa() {
; CHECK-LABEL: @test_memssa(
; CHECK-NEXT: bb:
-; CHECK-NEXT: [[TMP1:%.*]] = call i1 @llvm.experimental.widenable.condition()
; CHECK-NEXT: [[TMP:%.*]] = call i1 @llvm.experimental.widenable.condition()
+; CHECK-NEXT: [[TMP1:%.*]] = call i1 @llvm.experimental.widenable.condition()
; CHECK-NEXT: br i1 [[TMP]], label [[BB3:%.*]], label [[BB2:%.*]]
; CHECK: bb2:
; CHECK-NEXT: unreachable
More information about the llvm-commits
mailing list