Skip to content

Commit 8051156

Browse files
committed
[LSR] Unify scheduling of existing and inserted addrecs
LSR goes to some lengths to schedule IV increments such that %iv and %iv.next never need to overlap. This is fairly fundamental to LSRs cost model. LSR assumes that an addrec can be represented with a single register. If %iv and %iv.next have to overlap, then that assumption does not hold. The bug - which this patch is fixing - is that LSR only does this scheduling for IVs which it inserts, but it's cost model assumes the same for existing IVs that it reuses. It will rewrite existing IV users such that the no-overlap property holds, but will not actually reschedule said IV increment. As you can see from the relatively lack of test updates, this doesn't actually impact codegen much. The main reason for doing it is to make a follow up patch series which improves post-increment use and scheduling easier to follow. Differential Revision: https://reviews.llvm.org/D97219
1 parent 7334b3d commit 8051156

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed

llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5561,6 +5561,46 @@ void LSRInstance::ImplementSolution(
55615561

55625562
Changed |= RecursivelyDeleteTriviallyDeadInstructionsPermissive(DeadInsts,
55635563
&TLI, MSSAU);
5564+
5565+
// In our cost analysis above, we assume that each addrec consumes exactly
5566+
// one register, and arrange to have increments inserted just before the
5567+
// latch to maximimize the chance this is true. However, if we reused
5568+
// existing IVs, we now need to move the increments to match our
5569+
// expectations. Otherwise, our cost modeling results in us having a
5570+
// chosen a non-optimal result for the actual schedule. (And yes, this
5571+
// scheduling decision does impact later codegen.)
5572+
for (PHINode &PN : L->getHeader()->phis()) {
5573+
// First, filter out anything not an obvious addrec
5574+
if (!SE.isSCEVable(PN.getType()) || !isa<SCEVAddRecExpr>(SE.getSCEV(&PN)))
5575+
continue;
5576+
Instruction *IncV =
5577+
dyn_cast<Instruction>(PN.getIncomingValueForBlock(L->getLoopLatch()));
5578+
if (!IncV)
5579+
continue;
5580+
5581+
if (IncV->getOpcode() != Instruction::Add &&
5582+
IncV->getOpcode() != Instruction::Sub)
5583+
continue;
5584+
5585+
if (IncV->getOperand(0) != &PN &&
5586+
!isa<Constant>(IncV->getOperand(1)))
5587+
// If not a constant step, might increase register pressure
5588+
// (We assume constants have been canonicalized to RHS)
5589+
continue;
5590+
5591+
if (IncV->getParent() == IVIncInsertPos->getParent())
5592+
// Only bother moving across blocks. Isel can handle block local case.
5593+
continue;
5594+
5595+
// Can we legally schedule inc at the desired point?
5596+
if (!llvm::all_of(IncV->uses(),
5597+
[&](Use &U) {return DT.dominates(IVIncInsertPos, U);}))
5598+
continue;
5599+
IncV->moveBefore(IVIncInsertPos);
5600+
Changed = true;
5601+
}
5602+
5603+
55645604
}
55655605

55665606
LSRInstance::LSRInstance(Loop *L, IVUsers &IU, ScalarEvolution &SE,

llvm/test/Transforms/LoopStrengthReduce/post-increment-insertion.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ define i32 @test_01(i32* %p, i64 %len, i32 %x) {
1313
; CHECK-NEXT: br label [[LOOP:%.*]]
1414
; CHECK: loop:
1515
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_NEXT:%.*]], [[BACKEDGE:%.*]] ], [ [[LEN:%.*]], [[ENTRY:%.*]] ]
16-
; CHECK-NEXT: [[IV_NEXT]] = add nsw i64 [[IV]], -1
1716
; CHECK-NEXT: [[COND_1:%.*]] = icmp eq i64 [[IV]], 0
1817
; CHECK-NEXT: br i1 [[COND_1]], label [[EXIT:%.*]], label [[BACKEDGE]]
1918
; CHECK: backedge:
2019
; CHECK-NEXT: [[SCEVGEP1:%.*]] = getelementptr i32, i32* [[SCEVGEP]], i64 [[IV]]
2120
; CHECK-NEXT: [[LOADED:%.*]] = load atomic i32, i32* [[SCEVGEP1]] unordered, align 4
2221
; CHECK-NEXT: [[COND_2:%.*]] = icmp eq i32 [[LOADED]], [[X:%.*]]
22+
; CHECK-NEXT: [[IV_NEXT]] = add nsw i64 [[IV]], -1
2323
; CHECK-NEXT: br i1 [[COND_2]], label [[FAILURE:%.*]], label [[LOOP]]
2424
; CHECK: exit:
2525
; CHECK-NEXT: ret i32 -1
@@ -145,13 +145,13 @@ define i32 @test_04(i32* %p, i64 %len, i32 %x) {
145145
; CHECK-NEXT: br label [[LOOP:%.*]]
146146
; CHECK: loop:
147147
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ [[IV_NEXT:%.*]], [[BACKEDGE:%.*]] ], [ [[LEN:%.*]], [[ENTRY:%.*]] ]
148-
; CHECK-NEXT: [[IV_NEXT]] = sub i64 [[IV]], 1
149148
; CHECK-NEXT: [[COND_1:%.*]] = icmp eq i64 [[IV]], 0
150149
; CHECK-NEXT: br i1 [[COND_1]], label [[EXIT:%.*]], label [[BACKEDGE]]
151150
; CHECK: backedge:
152151
; CHECK-NEXT: [[SCEVGEP1:%.*]] = getelementptr i32, i32* [[SCEVGEP]], i64 [[IV]]
153152
; CHECK-NEXT: [[LOADED:%.*]] = load atomic i32, i32* [[SCEVGEP1]] unordered, align 4
154153
; CHECK-NEXT: [[COND_2:%.*]] = icmp eq i32 [[LOADED]], [[X:%.*]]
154+
; CHECK-NEXT: [[IV_NEXT]] = sub i64 [[IV]], 1
155155
; CHECK-NEXT: br i1 [[COND_2]], label [[FAILURE:%.*]], label [[LOOP]]
156156
; CHECK: exit:
157157
; CHECK-NEXT: ret i32 -1

0 commit comments

Comments
 (0)