Skip to content

Commit ba8facf

Browse files
committed
[SimpleLoopUnswitch] Fix SCEV invalidation for unswitchTrivialSwitch
When doing a trivial unswitch of a switch statement the code need to "invalidate SCEVs for the outermost loop reached by any of the exits", as indicated by code comments. Depending on if we find such an outermost loop or not we can limit the invalidation to some sub-loops or the full loop-nest. As shown in the added test case there seem to have been some bugs in the code that was finding the "outermost loop", so we could end up invalidating too few loops. Seems like commit 1bf8ae1 introduced the bug by moving the code that invalidates the loops above some of the code that computed 'OuterL'. This patch fixes that by also moving that computation of 'OuterL' so that we compute 'OuterL' properly before we use it for the SCEV invalidation. Differential Revision: https://reviews.llvm.org/D146963
1 parent d892521 commit ba8facf

File tree

2 files changed

+82
-4
lines changed

2 files changed

+82
-4
lines changed

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,13 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
796796
if (!ExitL || ExitL->contains(OuterL))
797797
OuterL = ExitL;
798798
}
799+
for (unsigned Index : ExitCaseIndices) {
800+
auto CaseI = SI.case_begin() + Index;
801+
// Compute the outer loop from this exit.
802+
Loop *ExitL = LI.getLoopFor(CaseI->getCaseSuccessor());
803+
if (!ExitL || ExitL->contains(OuterL))
804+
OuterL = ExitL;
805+
}
799806

800807
if (SE) {
801808
if (OuterL)
@@ -821,10 +828,6 @@ static bool unswitchTrivialSwitch(Loop &L, SwitchInst &SI, DominatorTree &DT,
821828
// and don't disrupt the earlier indices.
822829
for (unsigned Index : reverse(ExitCaseIndices)) {
823830
auto CaseI = SI.case_begin() + Index;
824-
// Compute the outer loop from this exit.
825-
Loop *ExitL = LI.getLoopFor(CaseI->getCaseSuccessor());
826-
if (!ExitL || ExitL->contains(OuterL))
827-
OuterL = ExitL;
828831
// Save the value of this case.
829832
auto W = SIW.getSuccessorWeight(CaseI->getSuccessorIndex());
830833
ExitCases.emplace_back(CaseI->getCaseValue(), CaseI->getCaseSuccessor(), W);
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
2+
; RUN: opt < %s -passes='print<scalar-evolution>,simple-loop-unswitch<no-nontrivial>,print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefix CHECK-SCEV %s
3+
; RUN: opt < %s -passes='function(loop-deletion,simple-loop-unswitch<no-nontrivial>),verify<scalar-evolution>' -S | FileCheck -check-prefix CHECK-IR %s
4+
5+
; These tests used to hit an assertion failure in llvm::ScalarEvolution::BackedgeTakenInfo::getExact:
6+
; "We should only have known counts for exiting blocks that dominate latch!"' failed.
7+
;
8+
; Verify that we no longer hit that assert, while we expect to have forgotten SCEV for the outer loop.
9+
; Also added checks to show and verify the IR transformation.
10+
11+
12+
; Before the unswitch:
13+
;
14+
; CHECK-SCEV: Classifying expressions for: @f4
15+
; CHECK-SCEV-NEXT: %j.0 = phi i16 [ 0, %entry ], [ %0, %sw.bb2 ]
16+
; CHECK-SCEV-NEXT: --> {0,+,1}<nuw><nsw><%lbl1> U: [0,3) S: [0,3) Exits: 2 LoopDispositions: { %lbl1: Computable, %lbl2: Invariant }
17+
; CHECK-SCEV-NEXT: %0 = add i16 %j.0, 1
18+
; CHECK-SCEV-NEXT: --> {1,+,1}<nuw><nsw><%lbl1> U: [1,4) S: [1,4) Exits: 3 LoopDispositions: { %lbl1: Computable, %lbl2: Invariant }
19+
; CHECK-SCEV-DAG: Loop %lbl2: Unpredictable backedge-taken count.
20+
; CHECK-SCEV-DAG: Loop %lbl1: backedge-taken count is 2
21+
;
22+
; After the unswitch:
23+
;
24+
; CHECK-SCEV: Classifying expressions for: @f4
25+
; CHECK-SCEV-NEXT: %j.0 = phi i16 [ 0, %entry ], [ %0, %sw.bb2 ]
26+
; CHECK-SCEV-NEXT: --> {0,+,1}<nuw><nsw><%lbl1> U: [0,-32768) S: [0,-32768) Exits: <<Unknown>> LoopDispositions: { %lbl1: Computable }
27+
; CHECK-SCEV-NEXT: %0 = add i16 %j.0, 1
28+
; CHECK-SCEV-NEXT: --> {1,+,1}<nuw><nsw><%lbl1> U: [1,-32768) S: [1,-32768) Exits: <<Unknown>> LoopDispositions: { %lbl1: Computable }
29+
; CHECK-SCEV-DAG: Loop %lbl1: Unpredictable backedge-taken count.
30+
; CHECK-SCEV-DAG: Loop %lbl2: <multiple exits> Unpredictable backedge-taken count.
31+
32+
33+
define i16 @f4() {
34+
; CHECK-IR-LABEL: define i16 @f4() {
35+
; CHECK-IR-NEXT: entry:
36+
; CHECK-IR-NEXT: br label [[LBL1:%.*]]
37+
; CHECK-IR: lbl1:
38+
; CHECK-IR-NEXT: [[J_0:%.*]] = phi i16 [ 0, [[ENTRY:%.*]] ], [ [[TMP0:%.*]], [[SW_BB2:%.*]] ]
39+
; CHECK-IR-NEXT: switch i16 [[J_0]], label [[LBL1_SPLIT:%.*]] [
40+
; CHECK-IR-NEXT: i16 0, label [[SW_BB2]]
41+
; CHECK-IR-NEXT: i16 1, label [[SW_BB2]]
42+
; CHECK-IR-NEXT: i16 2, label [[LBL3:%.*]]
43+
; CHECK-IR-NEXT: ]
44+
; CHECK-IR: lbl1.split:
45+
; CHECK-IR-NEXT: br label [[LBL2:%.*]]
46+
; CHECK-IR: lbl2:
47+
; CHECK-IR-NEXT: br label [[LBL2]]
48+
; CHECK-IR: sw.bb2:
49+
; CHECK-IR-NEXT: [[TMP0]] = add i16 [[J_0]], 1
50+
; CHECK-IR-NEXT: br label [[LBL1]]
51+
; CHECK-IR: lbl3:
52+
; CHECK-IR-NEXT: ret i16 0
53+
;
54+
entry:
55+
br label %lbl1
56+
57+
lbl1: ; preds = %sw.bb2, %entry
58+
%j.0 = phi i16 [ 0, %entry ], [ %0, %sw.bb2 ]
59+
br label %lbl2
60+
61+
lbl2: ; preds = %lbl2, %lbl1
62+
switch i16 %j.0, label %lbl2 [
63+
i16 0, label %sw.bb2
64+
i16 1, label %sw.bb2
65+
i16 2, label %lbl3
66+
]
67+
68+
sw.bb2: ; preds = %lbl2, %lbl2
69+
%0 = add i16 %j.0, 1
70+
br label %lbl1
71+
72+
lbl3: ; preds = %lbl2
73+
ret i16 0
74+
}
75+

0 commit comments

Comments
 (0)