[PATCH] D153254: [SCEV] Don't store AddRec loop when simplifying multiplication of AddRecs
Dmitry Makogon via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 19 02:43:56 PDT 2023
dmakogon created this revision.
dmakogon added a reviewer: apilipenko.
Herald added subscribers: javed.absar, hiraditya.
Herald added a project: All.
dmakogon requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
When multiplying several AddRecs, we do the following simplification:
{A1,+,A2,+,...,+,An}<L> * {B1,+,B2,+,...,+,Bn}<L> = {x=1 in [ sum y=x..2x [ sum z=max(y-x, y-n)..min(x,n) [choose(x, 2x)*choose(2x-y, x-z)*A_{y-z}*B_z]]],+,...up to x=2n}
This is done iteratively, pair by pair. So if we try to multiply three AddRecs `A1`, `A2`, `A3`, then we'd try to simplify `A1 * A2` to `A1'` and then try to simplify `A1' * A3` if `A1'` is also an AddRec.
The transform is only legal if the loops of the two AddRecs are the same. It is checked in the code, but the loop of one of the AddRecs is stored in a local variable and doesn't get updated when we simplify a pair to a new AddRec. In the motivating test the new AddRec was created for a different loop and as the loop variable didn't get updated, the transform worked for two AddRecs from different loops. So it created a wrong SCEV. And it caused LSR to replace an instruction with another one that had the same SCEV as the incorrectly computed one.
https://reviews.llvm.org/D153254
Files:
llvm/lib/Analysis/ScalarEvolution.cpp
llvm/test/Analysis/ScalarEvolution/pr62430.ll
Index: llvm/test/Analysis/ScalarEvolution/pr62430.ll
===================================================================
--- llvm/test/Analysis/ScalarEvolution/pr62430.ll
+++ llvm/test/Analysis/ScalarEvolution/pr62430.ll
@@ -9,38 +9,22 @@
; CHECK-NEXT: bb:
; CHECK-NEXT: br label [[BB3:%.*]]
; CHECK: bb3:
-; CHECK-NEXT: [[LSR_IV13:%.*]] = phi i32 [ [[LSR_IV_NEXT14:%.*]], [[BB7:%.*]] ], [ 0, [[BB:%.*]] ]
-; CHECK-NEXT: [[LSR_IV11:%.*]] = phi i32 [ [[LSR_IV_NEXT12:%.*]], [[BB7]] ], [ 98304, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV9:%.*]] = phi i32 [ [[LSR_IV_NEXT10:%.*]], [[BB7]] ], [ 0, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV7:%.*]] = phi i32 [ [[LSR_IV_NEXT8:%.*]], [[BB7]] ], [ 147456, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV5:%.*]] = phi i32 [ [[LSR_IV_NEXT6:%.*]], [[BB7]] ], [ 0, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV3:%.*]] = phi i32 [ [[LSR_IV_NEXT4:%.*]], [[BB7]] ], [ 57344, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV1:%.*]] = phi i32 [ [[LSR_IV_NEXT2:%.*]], [[BB7]] ], [ 0, [[BB]] ]
-; CHECK-NEXT: [[LSR_IV:%.*]] = phi i32 [ [[LSR_IV_NEXT:%.*]], [[BB7]] ], [ 4096, [[BB]] ]
+; CHECK-NEXT: [[LSR_IV1:%.*]] = phi i32 [ [[LSR_IV_NEXT2:%.*]], [[BB7:%.*]] ], [ 0, [[BB:%.*]] ]
+; CHECK-NEXT: [[LSR_IV:%.*]] = phi i32 [ [[LSR_IV_NEXT:%.*]], [[BB7]] ], [ 16, [[BB]] ]
; CHECK-NEXT: br label [[BB4:%.*]]
; CHECK: bb4:
-; CHECK-NEXT: [[LSR_IV21:%.*]] = phi i32 [ [[LSR_IV_NEXT22:%.*]], [[BB6:%.*]] ], [ 0, [[BB3]] ]
-; CHECK-NEXT: [[LSR_IV19:%.*]] = phi i32 [ [[LSR_IV_NEXT20:%.*]], [[BB6]] ], [ [[LSR_IV1]], [[BB3]] ]
-; CHECK-NEXT: [[LSR_IV17:%.*]] = phi i32 [ [[LSR_IV_NEXT18:%.*]], [[BB6]] ], [ [[LSR_IV5]], [[BB3]] ]
-; CHECK-NEXT: [[LSR_IV15:%.*]] = phi i32 [ [[LSR_IV_NEXT16:%.*]], [[BB6]] ], [ [[LSR_IV9]], [[BB3]] ]
+; CHECK-NEXT: [[LSR_IV3:%.*]] = phi i32 [ [[LSR_IV_NEXT4:%.*]], [[BB6:%.*]] ], [ [[LSR_IV1]], [[BB3]] ]
; CHECK-NEXT: br i1 true, label [[BB7]], label [[BB6]]
; CHECK: bb6:
-; CHECK-NEXT: [[LSR_IV_NEXT16]] = add i32 [[LSR_IV15]], [[LSR_IV13]]
-; CHECK-NEXT: [[LSR_IV_NEXT18]] = add i32 [[LSR_IV17]], [[LSR_IV15]]
-; CHECK-NEXT: [[LSR_IV_NEXT20]] = add i32 [[LSR_IV19]], [[LSR_IV17]]
-; CHECK-NEXT: [[LSR_IV_NEXT22]] = add i32 [[LSR_IV21]], [[LSR_IV19]]
+; CHECK-NEXT: [[LSR_IV_NEXT4]] = add i32 [[LSR_IV3]], 268435456
; CHECK-NEXT: br label [[BB4]]
; CHECK: bb7:
-; CHECK-NEXT: call void @foo(i32 [[LSR_IV21]])
-; CHECK-NEXT: [[SEXT:%.*]] = sext i32 [[LSR_IV21]] to i64
-; CHECK-NEXT: [[LSR_IV_NEXT]] = add i32 [[LSR_IV]], 8192
+; CHECK-NEXT: [[MUL9:%.*]] = mul i32 [[LSR_IV3]], [[LSR_IV3]]
+; CHECK-NEXT: [[MUL10:%.*]] = mul i32 [[MUL9]], [[LSR_IV3]]
+; CHECK-NEXT: call void @foo(i32 [[MUL10]])
+; CHECK-NEXT: [[SEXT:%.*]] = sext i32 [[MUL10]] to i64
+; CHECK-NEXT: [[LSR_IV_NEXT]] = add i32 [[LSR_IV]], 32
; CHECK-NEXT: [[LSR_IV_NEXT2]] = add i32 [[LSR_IV1]], [[LSR_IV]]
-; CHECK-NEXT: [[LSR_IV_NEXT4]] = add i32 [[LSR_IV3]], 114688
-; CHECK-NEXT: [[LSR_IV_NEXT6]] = add i32 [[LSR_IV5]], [[LSR_IV3]]
-; CHECK-NEXT: [[LSR_IV_NEXT8]] = add i32 [[LSR_IV7]], 294912
-; CHECK-NEXT: [[LSR_IV_NEXT10]] = add i32 [[LSR_IV9]], [[LSR_IV7]]
-; CHECK-NEXT: [[LSR_IV_NEXT12]] = add i32 [[LSR_IV11]], 196608
-; CHECK-NEXT: [[LSR_IV_NEXT14]] = add i32 [[LSR_IV13]], [[LSR_IV11]]
; CHECK-NEXT: br label [[BB3]]
;
bb:
Index: llvm/lib/Analysis/ScalarEvolution.cpp
===================================================================
--- llvm/lib/Analysis/ScalarEvolution.cpp
+++ llvm/lib/Analysis/ScalarEvolution.cpp
@@ -3261,9 +3261,8 @@
// if they are loop invariant w.r.t. the recurrence.
SmallVector<const SCEV *, 8> LIOps;
const SCEVAddRecExpr *AddRec = cast<SCEVAddRecExpr>(Ops[Idx]);
- const Loop *AddRecLoop = AddRec->getLoop();
for (unsigned i = 0, e = Ops.size(); i != e; ++i)
- if (isAvailableAtLoopEntry(Ops[i], AddRecLoop)) {
+ if (isAvailableAtLoopEntry(Ops[i], AddRec->getLoop())) {
LIOps.push_back(Ops[i]);
Ops.erase(Ops.begin()+i);
--i; --e;
@@ -3286,7 +3285,7 @@
// will be inferred if either NUW or NSW is true.
SCEV::NoWrapFlags Flags = ComputeFlags({Scale, AddRec});
const SCEV *NewRec = getAddRecExpr(
- NewOps, AddRecLoop, AddRec->getNoWrapFlags(Flags));
+ NewOps, AddRec->getLoop(), AddRec->getNoWrapFlags(Flags));
// If all of the other operands were loop invariant, we are done.
if (Ops.size() == 1) return NewRec;
@@ -3320,7 +3319,7 @@
++OtherIdx) {
const SCEVAddRecExpr *OtherAddRec =
dyn_cast<SCEVAddRecExpr>(Ops[OtherIdx]);
- if (!OtherAddRec || OtherAddRec->getLoop() != AddRecLoop)
+ if (!OtherAddRec || OtherAddRec->getLoop() != AddRec->getLoop())
continue;
// Limit max number of arguments to avoid creation of unreasonably big
@@ -3359,7 +3358,7 @@
AddRecOps.push_back(getAddExpr(SumOps, SCEV::FlagAnyWrap, Depth + 1));
}
if (!Overflow) {
- const SCEV *NewAddRec = getAddRecExpr(AddRecOps, AddRecLoop,
+ const SCEV *NewAddRec = getAddRecExpr(AddRecOps, AddRec->getLoop(),
SCEV::FlagAnyWrap);
if (Ops.size() == 2) return NewAddRec;
Ops[Idx] = NewAddRec;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153254.532578.patch
Type: text/x-patch
Size: 5297 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230619/94076e62/attachment.bin>
More information about the llvm-commits
mailing list