[SCEV] Check correct value for UB (#124302)

This is a followup to #117152. That patch introduced a check for
UB/poison on BEValue. However, the SCEV we're actually going to use is
Shifted. In some cases, it's possible for Shifted to contain UB, while
BEValue doesn't.

In the test case the values are:

BEValue: (-1 * (zext i8 (-83 + ((-83 /u {1,+,1}<%loop>) *
{-1,+,-1}<%loop>)) to i32))<nuw><nsw>
Shifted: (-173 + (-1 * (zext i8 ((-83 /u {0,+,1}<%loop>) *
{0,+,-1}<%loop>) to i32))<nuw><nsw>)<nuw><nsw>

Fixes #123550.

(cherry picked from commit 07efe2c)

Author: nikic

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;
       }
     }
   }

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