[llvm] ba1349f - [SCEV] Fix "quick and dirty" difference that could lead to assert (#70688)

[via llvm-commits]

Author: Danila Malyutin
Date: 2023-10-31T00:50:57+03:00
New Revision: ba1349fc31295a3670b34c189838a133e18c0bed

URL: https://github.com/llvm/llvm-project/commit/ba1349fc31295a3670b34c189838a133e18c0bed
DIFF: https://github.com/llvm/llvm-project/commit/ba1349fc31295a3670b34c189838a133e18c0bed.diff

LOG: [SCEV] Fix "quick and dirty" difference that could lead to assert (#70688)

The old algorithm would remove all operands matching %step SCEV when it
intended to only remove a single one. This lead to assert when
SCEVAddExpr was of the form %step + %step and potential miscompiles in
similar cases. Such SCEVs could be created when construction reached
depth thresholds.

Fixes #70348

Added: 
    llvm/test/Transforms/LoopStrengthReduce/pr70348.ll

Modified: 
    llvm/lib/Analysis/ScalarEvolution.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2368003177e741c..f13e508a6c454bd 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -1335,11 +1335,14 @@ static const SCEV *getPreStartForExtend(const SCEVAddRecExpr *AR, Type *Ty,
 
   // Create an AddExpr for "PreStart" after subtracting Step. Full SCEV
   // subtraction is expensive. For this purpose, perform a quick and dirty
-  // 
diff erence, by checking for Step in the operand list.
-  SmallVector<const SCEV *, 4> DiffOps;
-  for (const SCEV *Op : SA->operands())
-    if (Op != Step)
-      DiffOps.push_back(Op);
+  // 
diff erence, by checking for Step in the operand list. Note, that
+  // SA might have repeated ops, like %a + %a + ..., so only remove one.
+  SmallVector<const SCEV *, 4> DiffOps(SA->operands());
+  for (auto It = DiffOps.begin(); It != DiffOps.end(); ++It)
+    if (*It == Step) {
+      DiffOps.erase(It);
+      break;
+    }
 
   if (DiffOps.size() == SA->getNumOperands())
     return nullptr;

diff  --git a/llvm/test/Transforms/LoopStrengthReduce/pr70348.ll b/llvm/test/Transforms/LoopStrengthReduce/pr70348.ll
new file mode 100644
index 000000000000000..35b48a03a14e435
--- /dev/null
+++ b/llvm/test/Transforms/LoopStrengthReduce/pr70348.ll
@@ -0,0 +1,23 @@
+; RUN: opt -S -passes=loop-reduce -scalar-evolution-max-arith-depth=0 %s | FileCheck %s
+;
+; Make sure we don't trigger an assertion in SCEV here.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @test(i32 %phi) {
+; CHECK-LABEL: test
+bb:
+  br label %bb6
+
+bb6:                                              ; preds = %bb6, %bb
+  %phi7 = phi i32 [ 1, %bb ], [ %add44, %bb6 ]
+  %mul13 = mul i32 %phi7, %phi
+  %mul16 = mul i32 %mul13, 0
+  %add44 = add i32 %phi7, 1
+  br i1 true, label %bb51, label %bb6
+
+bb51:                                             ; preds = %bb6
+  unreachable
+}
+