[llvm-branch-commits] [llvm] release/20.x: [SCEV] Check correct value for UB (#124302) (PR #124895)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: None (llvmbot)

<details>
<summary>Changes</summary>

Backport 07efe2c18a63423943a4f9d9daeada23601f84c8

Requested by: @<!-- -->nikic

---
Full diff: https://github.com/llvm/llvm-project/pull/124895.diff


2 Files Affected:

- (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+12-14) 
- (modified) llvm/test/Analysis/ScalarEvolution/pr123550.ll (+4-4) 


``````````diff
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 7d7d37b3d228dd..2ce40877b523e1 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -5917,20 +5917,18 @@ const SCEV *ScalarEvolution::createAddRecFromPHI(PHINode *PN) {
     //   PHI(f(0), f({1,+,1})) --> f({0,+,1})
 
     // 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;
-        }
+    const SCEV *Shifted = SCEVShiftRewriter::rewrite(BEValue, L, *this);
+    const SCEV *Start = SCEVInitRewriter::rewrite(Shifted, L, *this, false);
+    if (Shifted != getCouldNotCompute() && Start != getCouldNotCompute() &&
+        isGuaranteedNotToCauseUB(Shifted) && ::impliesPoison(Shifted, 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;
       }
     }
   }
diff --git a/llvm/test/Analysis/ScalarEvolution/pr123550.ll b/llvm/test/Analysis/ScalarEvolution/pr123550.ll
index c1f2051248a12d..709da00935ef3a 100644
--- a/llvm/test/Analysis/ScalarEvolution/pr123550.ll
+++ b/llvm/test/Analysis/ScalarEvolution/pr123550.ll
@@ -1,16 +1,16 @@
 ; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -disable-output -passes='print<scalar-evolution>' < %s 2>&1 | FileCheck %s
 
-; FIXME: This is a miscompile.
+; %srem should have exit value 130.
 define i32 @test() {
 ; CHECK-LABEL: 'test'
 ; CHECK-NEXT:  Classifying expressions for: @test
 ; CHECK-NEXT:    %phi = phi i32 [ -173, %bb ], [ %sub, %loop ]
-; CHECK-NEXT:    --> (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) * {0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw> U: empty-set S: empty-set Exits: -173 LoopDispositions: { %loop: Computable }
+; CHECK-NEXT:    --> %phi U: [-173,1) S: [-173,1) Exits: -173 LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %iv2 = phi i32 [ 1, %bb ], [ %iv2.inc, %loop ]
 ; CHECK-NEXT:    --> {1,+,1}<nuw><nsw><%loop> U: [1,2) S: [1,2) Exits: 1 LoopDispositions: { %loop: Computable }
 ; CHECK-NEXT:    %srem = srem i32 729259140, %phi
-; CHECK-NEXT:    --> (729259140 + (-1 * (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) * {0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw> * (729259140 /u (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) * {0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw>)))<nuw><nsw> U: empty-set S: empty-set Exits: 729259140 LoopDispositions: { %loop: Computable }
+; CHECK-NEXT:    --> %srem U: [0,1073741824) S: [0,1073741824) Exits: 130 LoopDispositions: { %loop: Variant }
 ; CHECK-NEXT:    %trunc = trunc i32 %iv2 to i8
 ; CHECK-NEXT:    --> {1,+,1}<%loop> U: [1,2) S: [1,2) Exits: 1 LoopDispositions: { %loop: Computable }
 ; CHECK-NEXT:    %urem = urem i8 -83, %trunc
@@ -22,7 +22,7 @@ define i32 @test() {
 ; CHECK-NEXT:    %iv2.inc = add i32 %iv2, 1
 ; CHECK-NEXT:    --> {2,+,1}<nuw><nsw><%loop> U: [2,3) S: [2,3) Exits: 2 LoopDispositions: { %loop: Computable }
 ; CHECK-NEXT:    %srem.lcssa = phi i32 [ %srem, %loop ]
-; CHECK-NEXT:    --> (729259140 + (-1 * (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) * {0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw> * (729259140 /u (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) * {0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw>)))<nuw><nsw> U: empty-set S: empty-set --> 729259140 U: [729259140,729259141) S: [729259140,729259141)
+; CHECK-NEXT:    --> %srem U: [0,1073741824) S: [0,1073741824) --> 130 U: [130,131) S: [130,131)
 ; CHECK-NEXT:  Determining loop execution counts for: @test
 ; CHECK-NEXT:  Loop %loop: backedge-taken count is i32 0
 ; CHECK-NEXT:  Loop %loop: constant max backedge-taken count is i32 0

``````````

</details>


https://github.com/llvm/llvm-project/pull/124895