Skip to content

[SCEV] Do not allow refinement in the rewriting of BEValue #117152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/include/llvm/Analysis/ScalarEvolution.h
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,9 @@ class ScalarEvolution {
bool isGuaranteedToTransferExecutionTo(const Instruction *A,
const Instruction *B);

/// Returns true if \p Op is guaranteed not to cause immediate UB.
bool isGuaranteedNotToCauseUB(const SCEV *Op);

/// Returns true if \p Op is guaranteed to not be poison.
static bool isGuaranteedNotToBePoison(const SCEV *Op);

Expand Down
48 changes: 27 additions & 21 deletions llvm/lib/Analysis/ScalarEvolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4307,15 +4307,7 @@ ScalarEvolution::getSequentialMinMaxExpr(SCEVTypes Kind,
}

for (unsigned i = 1, e = Ops.size(); i != e; ++i) {
bool MayBeUB = SCEVExprContains(Ops[i], [this](const SCEV *S) {
auto *UDiv = dyn_cast<SCEVUDivExpr>(S);
// The UDiv may be UB if the divisor is poison or zero. Unless the divisor
// is a non-zero constant, we have to assume the UDiv may be UB.
return UDiv && (!isKnownNonZero(UDiv->getOperand(1)) ||
!isGuaranteedNotToBePoison(UDiv->getOperand(1)));
});

if (MayBeUB)
if (!isGuaranteedNotToCauseUB(Ops[i]))
continue;
// We can replace %x umin_seq %y with %x umin %y if either:
// * %y being poison implies %x is also poison.
Expand Down Expand Up @@ -5933,18 +5925,22 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
// We can generalize this saying that i is the shifted value of BEValue
// by one iteration:
// PHI(f(0), f({1,+,1})) --> f({0,+,1})
const SCEV *Shifted = SCEVShiftRewriter::rewrite(BEValue, L, *this);
const SCEV *Start = SCEVInitRewriter::rewrite(Shifted, L, *this, false);
if (Shifted != getCouldNotCompute() &&
Start != getCouldNotCompute()) {
const SCEV *StartVal = getSCEV(StartValueV);
if (Start == StartVal) {
// Okay, for the entire analysis of this edge we assumed the PHI
// to be symbolic. We now need to go back and purge all of the
// entries for the scalars that use the symbolic expression.
forgetMemoizedResults(SymbolicName);
insertValueToMap(PN, Shifted);
return Shifted;

// Do not allow refinement in rewriting of BEValue.
if (isGuaranteedNotToCauseUB(BEValue)) {
const SCEV *Shifted = SCEVShiftRewriter::rewrite(BEValue, L, *this);
const SCEV *Start = SCEVInitRewriter::rewrite(Shifted, L, *this, false);
if (Shifted != getCouldNotCompute() && Start != getCouldNotCompute() &&
::impliesPoison(BEValue, Start)) {
const SCEV *StartVal = getSCEV(StartValueV);
if (Start == StartVal) {
// Okay, for the entire analysis of this edge we assumed the PHI
// to be symbolic. We now need to go back and purge all of the
// entries for the scalars that use the symbolic expression.
forgetMemoizedResults(SymbolicName);
insertValueToMap(PN, Shifted);
return Shifted;
}
}
}
}
Expand Down Expand Up @@ -7322,6 +7318,16 @@ bool ScalarEvolution::isGuaranteedNotToBePoison(const SCEV *Op) {
return PC.MaybePoison.empty();
}

bool ScalarEvolution::isGuaranteedNotToCauseUB(const SCEV *Op) {
return !SCEVExprContains(Op, [this](const SCEV *S) {
auto *UDiv = dyn_cast<SCEVUDivExpr>(S);
// The UDiv may be UB if the divisor is poison or zero. Unless the divisor
// is a non-zero constant, we have to assume the UDiv may be UB.
return UDiv && (!isKnownNonZero(UDiv->getOperand(1)) ||
!isGuaranteedNotToBePoison(UDiv->getOperand(1)));
});
}

bool ScalarEvolution::isSCEVExprNeverPoison(const Instruction *I) {
// Only proceed if we can prove that I does not yield poison.
if (!programUndefinedIfPoison(I))
Expand Down
97 changes: 97 additions & 0 deletions llvm/test/Analysis/ScalarEvolution/pr117133.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
; RUN: opt -S -disable-output "-passes=print<scalar-evolution>" < %s 2>&1 | FileCheck %s

define i32 @widget() {
; CHECK-LABEL: 'widget'
; CHECK-NEXT: Classifying expressions for: @widget
; CHECK-NEXT: %phi = phi i32 [ 0, %b ], [ %udiv6, %b5 ]
; CHECK-NEXT: --> %phi U: [0,1) S: [0,1) Exits: <<Unknown>> LoopDispositions: { %b1: Variant }
; CHECK-NEXT: %phi2 = phi i32 [ 1, %b ], [ %add, %b5 ]
; CHECK-NEXT: --> {1,+,1}<nuw><nsw><%b1> U: [1,2) S: [1,2) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
; CHECK-NEXT: %udiv = udiv i32 10, %phi2
; CHECK-NEXT: --> (10 /u {1,+,1}<nuw><nsw><%b1>) U: [10,11) S: [10,11) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
; CHECK-NEXT: %urem = urem i32 %udiv, 10
; 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 }
; CHECK-NEXT: %udiv6 = udiv i32 %phi2, 0
; CHECK-NEXT: --> ({1,+,1}<nuw><nsw><%b1> /u 0) U: empty-set S: empty-set Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
; CHECK-NEXT: %add = add i32 %phi2, 1
; CHECK-NEXT: --> {2,+,1}<nuw><nsw><%b1> U: [2,3) S: [2,3) Exits: <<Unknown>> LoopDispositions: { %b1: Computable }
; CHECK-NEXT: Determining loop execution counts for: @widget
; CHECK-NEXT: Loop %b1: <multiple exits> Unpredictable backedge-taken count.
; CHECK-NEXT: exit count for b1: ***COULDNOTCOMPUTE***
; CHECK-NEXT: exit count for b3: i32 0
; CHECK-NEXT: Loop %b1: constant max backedge-taken count is i32 0
; CHECK-NEXT: Loop %b1: symbolic max backedge-taken count is i32 0
; CHECK-NEXT: symbolic max exit count for b1: ***COULDNOTCOMPUTE***
; CHECK-NEXT: symbolic max exit count for b3: i32 0
;
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
}

; Don't fold %indvar2 into (zext {0,+,1}) * %a
define i64 @test_poisonous(i64 %a, i32 %n) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before:

Printing analysis 'Scalar Evolution Analysis' for function 'test':
Classifying expressions for: @test
  %indvar1 = phi i32 [ 0, %entry ], [ %indvar1.next, %loop.body ]
  -->  {0,+,1}<%loop.body> U: full-set S: full-set              Exits: <<Unknown>>              LoopDispositions: { %loop.body: Computable }
  %indvar2 = phi i64 [ 0, %entry ], [ %mul, %loop.body ]
  -->  ((zext i32 {0,+,1}<%loop.body> to i64) * %a) U: full-set S: full-set             Exits: <<Unknown>>              LoopDispositions: { %loop.body: Computable }
  %indvar1.next = add i32 %indvar1, 1
  -->  {1,+,1}<%loop.body> U: full-set S: full-set              Exits: <<Unknown>>              LoopDispositions: { %loop.body: Computable }
  %ext = zext i32 %indvar1.next to i64
  -->  (zext i32 {1,+,1}<%loop.body> to i64) U: [0,4294967296) S: [0,4294967296)                Exits: <<Unknown>>              LoopDispositions: { %loop.body: Computable }
  %mul = mul i64 %ext, %a
  -->  ((zext i32 {1,+,1}<%loop.body> to i64) * %a) U: full-set S: full-set             Exits: <<Unknown>>              LoopDispositions: { %loop.body: Computable }

; CHECK-LABEL: 'test_poisonous'
; CHECK-NEXT: Classifying expressions for: @test_poisonous
; CHECK-NEXT: %indvar1 = phi i32 [ 0, %entry ], [ %indvar1.next, %loop.body ]
; CHECK-NEXT: --> {0,+,1}<%loop.body> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
; CHECK-NEXT: %indvar2 = phi i64 [ 0, %entry ], [ %mul, %loop.body ]
; CHECK-NEXT: --> %indvar2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Variant }
; CHECK-NEXT: %indvar1.next = add i32 %indvar1, 1
; CHECK-NEXT: --> {1,+,1}<%loop.body> U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
; CHECK-NEXT: %ext = zext i32 %indvar1.next to i64
; CHECK-NEXT: --> (zext i32 {1,+,1}<%loop.body> to i64) U: [0,4294967296) S: [0,4294967296) Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
; CHECK-NEXT: %mul = mul i64 %ext, %a
; CHECK-NEXT: --> ((zext i32 {1,+,1}<%loop.body> to i64) * %a) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.body: Computable }
; CHECK-NEXT: Determining loop execution counts for: @test_poisonous
; CHECK-NEXT: Loop %loop.body: Unpredictable backedge-taken count.
; CHECK-NEXT: Loop %loop.body: Unpredictable constant max backedge-taken count.
; CHECK-NEXT: Loop %loop.body: Unpredictable symbolic max backedge-taken count.
; CHECK-NEXT: Loop %loop.body: Predicated backedge-taken count is (-1 + (1 smax (1 + (sext i32 %n to i64))<nsw>))<nsw>
; CHECK-NEXT: Predicates:
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
; CHECK-NEXT: Loop %loop.body: Predicated constant max backedge-taken count is i64 2147483647
; CHECK-NEXT: Predicates:
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
; CHECK-NEXT: Loop %loop.body: Predicated symbolic max backedge-taken count is (-1 + (1 smax (1 + (sext i32 %n to i64))<nsw>))<nsw>
; CHECK-NEXT: Predicates:
; CHECK-NEXT: {1,+,1}<%loop.body> Added Flags: <nssw>
;
entry:
br label %loop.body

loop.body:
%indvar1 = phi i32 [ 0, %entry ], [ %indvar1.next, %loop.body ]
%indvar2 = phi i64 [ 0, %entry ], [ %mul, %loop.body ]
%indvar1.next = add i32 %indvar1, 1
%ext = zext i32 %indvar1.next to i64
%mul = mul i64 %ext, %a
%exitcond = icmp sgt i32 %indvar1.next, %n
br i1 %exitcond, label %loop.exit, label %loop.body

loop.exit:
ret i64 %mul
}
44 changes: 44 additions & 0 deletions llvm/test/Transforms/IndVarSimplify/pr117133.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt -S -passes=indvars < %s | FileCheck %s

define i32 @widget() {
; CHECK-LABEL: define i32 @widget() {
; CHECK-NEXT: [[B:.*:]]
; CHECK-NEXT: br label %[[B1:.*]]
; CHECK: [[B1]]:
; CHECK-NEXT: br i1 true, label %[[B3:.*]], label %[[B8:.*]]
; CHECK: [[B3]]:
; CHECK-NEXT: br i1 true, label %[[B7:.*]], label %[[B5:.*]]
; CHECK: [[B5]]:
; CHECK-NEXT: br label %[[B1]]
; CHECK: [[B7]]:
; CHECK-NEXT: ret i32 5
; CHECK: [[B8]]:
; CHECK-NEXT: ret i32 7
;
b:
br label %b1

b1:
%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:
%udiv = udiv i32 10, %phi2
%urem = urem i32 %udiv, 10
%icmp4 = icmp eq i32 %urem, 0
br i1 %icmp4, label %b7, label %b5

b5:
%udiv6 = udiv i32 %phi2, 0
%add = add i32 %phi2, 1
br label %b1

b7:
ret i32 5

b8:
ret i32 7
}