Skip to content

Commit f7ef072

Browse files
dtcxzywdtcxzyw
authored
[SCEV] Do not allow refinement in the rewriting of BEValue (#117152)
See the following case: ``` ; bin/opt -passes="print<scalar-evolution>" test.ll --disable-output define i32 @widget() { b: br label %b1 b1: ; preds = %b5, %b %phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ] %phi2 = phi i32 [ 1, %b ], [ %add, %b5 ] %icmp = icmp eq i32 %phi, 0 br i1 %icmp, label %b3, label %b8 b3: ; preds = %b1 %udiv = udiv i32 10, %phi2 %urem = urem i32 %udiv, 10 %icmp4 = icmp eq i32 %urem, 0 br i1 %icmp4, label %b7, label %b5 b5: ; preds = %b3 %udiv6 = udiv i32 %phi2, 0 %add = add i32 %phi2, 1 br label %b1 b7: ; preds = %b3 ret i32 5 b8: ; preds = %b1 ret i32 7 } ``` ``` %phi2 = phi i32 [ 1, %b ], [ %add, %b5 ] --> {1,+,1}<nuw><nsw><%b1> %udiv6 = udiv i32 %phi2, 0 --> ({1,+,1}<nuw><nsw><%b1> /u 0) %phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ] --> ({0,+,1}<nuw><nsw><%b1> /u 0) ``` `ScalarEvolution::createAddRecFromPHI` gives a wrong SCEV result for `%phi`: https://github.com/llvm/llvm-project/blob/d7d6fb1804415b0f3e7f1cc9290bfb3d711cb707/llvm/lib/Analysis/ScalarEvolution.cpp#L5926-L5950 It converts `phi(0, ({1,+,1}<nuw><nsw><%b1> /u 0))` into `phi(0 / 0, ({1,+,1}<nuw><nsw><%b1> /u 0))`. Then it simplifies the expr into `{0,+,1}<nuw><nsw><%b1> /u 0`. As we did in acd700a, this patch disallows udiv simplification if we cannot prove that the denominator is a well-defined non-zero value. Fixes #117133.
1 parent 94df95d commit f7ef072

File tree

4 files changed

+171
-21
lines changed

4 files changed

+171
-21
lines changed

‎llvm/include/llvm/Analysis/ScalarEvolution.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2187,6 +2187,9 @@ class ScalarEvolution {
21872187
bool isGuaranteedToTransferExecutionTo(const Instruction *A,
21882188
const Instruction *B);
21892189

2190+
/// Returns true if \p Op is guaranteed not to cause immediate UB.
2191+
bool isGuaranteedNotToCauseUB(const SCEV *Op);
2192+
21902193
/// Returns true if \p Op is guaranteed to not be poison.
21912194
static bool isGuaranteedNotToBePoison(const SCEV *Op);
21922195

‎llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4307,15 +4307,7 @@ ScalarEvolution::getSequentialMinMaxExpr(SCEVTypes Kind,
43074307
}
43084308

43094309
for (unsigned i = 1, e = Ops.size(); i != e; ++i) {
4310-
bool MayBeUB = SCEVExprContains(Ops[i], [this](const SCEV *S) {
4311-
auto *UDiv = dyn_cast<SCEVUDivExpr>(S);
4312-
// The UDiv may be UB if the divisor is poison or zero. Unless the divisor
4313-
// is a non-zero constant, we have to assume the UDiv may be UB.
4314-
return UDiv && (!isKnownNonZero(UDiv->getOperand(1)) ||
4315-
!isGuaranteedNotToBePoison(UDiv->getOperand(1)));
4316-
});
4317-
4318-
if (MayBeUB)
4310+
if (!isGuaranteedNotToCauseUB(Ops[i]))
43194311
continue;
43204312
// We can replace %x umin_seq %y with %x umin %y if either:
43214313
// * %y being poison implies %x is also poison.
@@ -5933,18 +5925,22 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
59335925
// We can generalize this saying that i is the shifted value of BEValue
59345926
// by one iteration:
59355927
// PHI(f(0), f({1,+,1})) --> f({0,+,1})
5936-
const SCEV *Shifted = SCEVShiftRewriter::rewrite(BEValue, L, *this);
5937-
const SCEV *Start = SCEVInitRewriter::rewrite(Shifted, L, *this, false);
5938-
if (Shifted != getCouldNotCompute() &&
5939-
Start != getCouldNotCompute()) {
5940-
const SCEV *StartVal = getSCEV(StartValueV);
5941-
if (Start == StartVal) {
5942-
// Okay, for the entire analysis of this edge we assumed the PHI
5943-
// to be symbolic. We now need to go back and purge all of the
5944-
// entries for the scalars that use the symbolic expression.
5945-
forgetMemoizedResults(SymbolicName);
5946-
insertValueToMap(PN, Shifted);
5947-
return Shifted;
5928+
5929+
// Do not allow refinement in rewriting of BEValue.
5930+
if (isGuaranteedNotToCauseUB(BEValue)) {
5931+
const SCEV *Shifted = SCEVShiftRewriter::rewrite(BEValue, L, *this);
5932+
const SCEV *Start = SCEVInitRewriter::rewrite(Shifted, L, *this, false);
5933+
if (Shifted != getCouldNotCompute() && Start != getCouldNotCompute() &&
5934+
::impliesPoison(BEValue, Start)) {
5935+
const SCEV *StartVal = getSCEV(StartValueV);
5936+
if (Start == StartVal) {
5937+
// Okay, for the entire analysis of this edge we assumed the PHI
5938+
// to be symbolic. We now need to go back and purge all of the
5939+
// entries for the scalars that use the symbolic expression.
5940+
forgetMemoizedResults(SymbolicName);
5941+
insertValueToMap(PN, Shifted);
5942+
return Shifted;
5943+
}
59485944
}
59495945
}
59505946
}
@@ -7324,6 +7320,16 @@ bool ScalarEvolution::isGuaranteedNotToBePoison(const SCEV *Op) {
73247320
return PC.MaybePoison.empty();
73257321
}
73267322

7323+
bool ScalarEvolution::isGuaranteedNotToCauseUB(const SCEV *Op) {
7324+
return !SCEVExprContains(Op, [this](const SCEV *S) {
7325+
auto *UDiv = dyn_cast<SCEVUDivExpr>(S);
7326+
// The UDiv may be UB if the divisor is poison or zero. Unless the divisor
7327+
// is a non-zero constant, we have to assume the UDiv may be UB.
7328+
return UDiv && (!isKnownNonZero(UDiv->getOperand(1)) ||
7329+
!isGuaranteedNotToBePoison(UDiv->getOperand(1)));
7330+
});
7331+
}
7332+
73277333
bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
73287334
// Only proceed if we can prove that I does not yield poison.
73297335
if (!programUndefinedIfPoison(I))

‎llvm/test/Analysis/ScalarEvolution/pr117133.ll

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -disable-output "-passes=print<scalar-evolution>" < %s 2>&1 | FileCheck %s
3+
4+
define i32 @widget() {
5+
; CHECK-LABEL: 'widget'
6+
; CHECK-NEXT: Classifying expressions for: @widget
7+
; CHECK-NEXT: %phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ]
8+
; CHECK-NEXT: --> %phi U: [0,1) S: [0,1) Exits: <<Unknown>> LoopDispositions: { %b1: Variant }
9+
; CHECK-NEXT: %phi2 = phi i32 [ 1, %b ], [ %add, %b5 ]
10+
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%b1> U: [1,2) S: [1,2) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
11+
; CHECK-NEXT: %udiv = udiv i32 10, %phi2
12+
; CHECK-NEXT: --> (10 /u {1,+,1}<nuw><nsw><%b1>) U: [10,11) S: [10,11) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
13+
; CHECK-NEXT: %urem = urem i32 %udiv, 10
14+
; CHECK-NEXT: --> ((-10 * ((10 /u {1,+,1}<nuw><nsw><%b1>) /u 10))<nuw><nsw> + (10 /u {1,+,1}<nuw><nsw><%b1>)) U: [0,1) S: [0,1) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
15+
; CHECK-NEXT: %udiv6 = udiv i32 %phi2, 0
16+
; CHECK-NEXT: --> ({1,+,1}<nuw><nsw><%b1> /u 0) U: empty-set S: empty-set Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
17+
; CHECK-NEXT: %add = add i32 %phi2, 1
18+
; CHECK-NEXT: --> {2,+,1}<nuw><nsw><%b1> U: [2,3) S: [2,3) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
19+
; CHECK-NEXT: Determining loop execution counts for: @widget
20+
; CHECK-NEXT: Loop %b1: <multiple exits> Unpredictable backedge-taken count.
21+
; CHECK-NEXT: exit count for b1: ***COULDNOTCOMPUTE***
22+
; CHECK-NEXT: exit count for b3: i32 0
23+
; CHECK-NEXT: Loop %b1: constant max backedge-taken count is i32 0
24+
; CHECK-NEXT: Loop %b1: symbolic max backedge-taken count is i32 0
25+
; CHECK-NEXT: symbolic max exit count for b1: ***COULDNOTCOMPUTE***
26+
; CHECK-NEXT: symbolic max exit count for b3: i32 0
27+
;
28+
b:
29+
br label %b1
30+
31+
b1: ; preds = %b5, %b
32+
%phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ]
33+
%phi2 = phi i32 [ 1, %b ], [ %add, %b5 ]
34+
%icmp = icmp eq i32 %phi, 0
35+
br i1 %icmp, label %b3, label %b8
36+
37+
b3: ; preds = %b1
38+
%udiv = udiv i32 10, %phi2
39+
%urem = urem i32 %udiv, 10
40+
%icmp4 = icmp eq i32 %urem, 0
41+
br i1 %icmp4, label %b7, label %b5
42+
43+
b5: ; preds = %b3
44+
%udiv6 = udiv i32 %phi2, 0
45+
%add = add i32 %phi2, 1
46+
br label %b1
47+
48+
b7: ; preds = %b3
49+
ret i32 5
50+
51+
b8: ; preds = %b1
52+
ret i32 7
53+
}
54+
55+
; Don't fold %indvar2 into (zext {0,+,1}) * %a
56+
define i64 @test_poisonous(i64 %a, i32 %n) {
57+
; CHECK-LABEL: 'test_poisonous'
58+
; CHECK-NEXT: Classifying expressions for: @test_poisonous
59+
; CHECK-NEXT: %indvar1 = phi i32 [ 0, %entry ], [ %indvar1.next, %loop.body ]
60+
; CHECK-NEXT: --> {0,+,1}<%loop.body> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
61+
; CHECK-NEXT: %indvar2 = phi i64 [ 0, %entry ], [ %mul, %loop.body ]
62+
; CHECK-NEXT: --> %indvar2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Variant }
63+
; CHECK-NEXT: %indvar1.next = add i32 %indvar1, 1
64+
; CHECK-NEXT: --> {1,+,1}<%loop.body> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
65+
; CHECK-NEXT: %ext = zext i32 %indvar1.next to i64
66+
; CHECK-NEXT: --> (zext i32 {1,+,1}<%loop.body> to i64) U: [0,4294967296) S: [0,4294967296) Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
67+
; CHECK-NEXT: %mul = mul i64 %ext, %a
68+
; CHECK-NEXT: --> ((zext i32 {1,+,1}<%loop.body> to i64) * %a) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
69+
; CHECK-NEXT: Determining loop execution counts for: @test_poisonous
70+
; CHECK-NEXT: Loop %loop.body: Unpredictable backedge-taken count.
71+
; CHECK-NEXT: Loop %loop.body: Unpredictable constant max backedge-taken count.
72+
; CHECK-NEXT: Loop %loop.body: Unpredictable symbolic max backedge-taken count.
73+
; CHECK-NEXT: Loop %loop.body: Predicated backedge-taken count is (-1 + (1 smax (1 + (sext i32 %n to i64))<nsw>))<nsw>
74+
; CHECK-NEXT: Predicates:
75+
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
76+
; CHECK-NEXT: Loop %loop.body: Predicated constant max backedge-taken count is i64 2147483647
77+
; CHECK-NEXT: Predicates:
78+
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
79+
; CHECK-NEXT: Loop %loop.body: Predicated symbolic max backedge-taken count is (-1 + (1 smax (1 + (sext i32 %n to i64))<nsw>))<nsw>
80+
; CHECK-NEXT: Predicates:
81+
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
82+
;
83+
entry:
84+
br label %loop.body
85+
86+
loop.body:
87+
%indvar1 = phi i32 [ 0, %entry ], [ %indvar1.next, %loop.body ]
88+
%indvar2 = phi i64 [ 0, %entry ], [ %mul, %loop.body ]
89+
%indvar1.next = add i32 %indvar1, 1
90+
%ext = zext i32 %indvar1.next to i64
91+
%mul = mul i64 %ext, %a
92+
%exitcond = icmp sgt i32 %indvar1.next, %n
93+
br i1 %exitcond, label %loop.exit, label %loop.body
94+
95+
loop.exit:
96+
ret i64 %mul
97+
}

‎llvm/test/Transforms/IndVarSimplify/pr117133.ll

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -passes=indvars < %s | FileCheck %s
3+
4+
define i32 @widget() {
5+
; CHECK-LABEL: define i32 @widget() {
6+
; CHECK-NEXT: [[B:.*:]]
7+
; CHECK-NEXT: br label %[[B1:.*]]
8+
; CHECK: [[B1]]:
9+
; CHECK-NEXT: br i1 true, label %[[B3:.*]], label %[[B8:.*]]
10+
; CHECK: [[B3]]:
11+
; CHECK-NEXT: br i1 true, label %[[B7:.*]], label %[[B5:.*]]
12+
; CHECK: [[B5]]:
13+
; CHECK-NEXT: br label %[[B1]]
14+
; CHECK: [[B7]]:
15+
; CHECK-NEXT: ret i32 5
16+
; CHECK: [[B8]]:
17+
; CHECK-NEXT: ret i32 7
18+
;
19+
b:
20+
br label %b1
21+
22+
b1:
23+
%phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ]
24+
%phi2 = phi i32 [ 1, %b ], [ %add, %b5 ]
25+
%icmp = icmp eq i32 %phi, 0
26+
br i1 %icmp, label %b3, label %b8
27+
28+
b3:
29+
%udiv = udiv i32 10, %phi2
30+
%urem = urem i32 %udiv, 10
31+
%icmp4 = icmp eq i32 %urem, 0
32+
br i1 %icmp4, label %b7, label %b5
33+
34+
b5:
35+
%udiv6 = udiv i32 %phi2, 0
36+
%add = add i32 %phi2, 1
37+
br label %b1
38+
39+
b7:
40+
ret i32 5
41+
42+
b8:
43+
ret i32 7
44+
}

0 commit comments

Comments
 (0)