[PATCH] D14230: Fix PR25372 - teach replaceCongruentPHIs to handle cases where SE evaluates a PHI to a SCEVConstant

[silviu.baranga@arm.com via llvm-commits]

sbaranga created this revision.
sbaranga added a reviewer: sanjoy.
sbaranga added subscribers: majnemer, llvm-commits.

Since now Scalar Evolution can create non-add rec expressions for PHI
nodes, it can also create SCEVConstant expressions. This will confuse
replaceCongruentPHIs, which previously relied on the fact that SCEV
could not produce constants in this case.

We will now replace the node with a constant in these cases - or avoid
processing the Phi in case of a type mismatch.

http://reviews.llvm.org/D14230

Files:
  lib/Analysis/ScalarEvolutionExpander.cpp
  test/Transforms/IndVarSimplify/const_phi.ll

Index: lib/Analysis/ScalarEvolutionExpander.cpp
===================================================================
--- lib/Analysis/ScalarEvolutionExpander.cpp
+++ lib/Analysis/ScalarEvolutionExpander.cpp
@@ -1737,7 +1737,20 @@
 
     // Fold constant phis. They may be congruent to other constant phis and
     // would confuse the logic below that expects proper IVs.
-    if (Value *V = SimplifyInstruction(Phi, DL, &SE.TLI, &SE.DT, &SE.AC)) {
+    Value *V = SimplifyInstruction(Phi, DL, &SE.TLI, &SE.DT, &SE.AC);
+    if (!V) {
+      const SCEVConstant *Const = nullptr;
+      if (SE.isSCEVable(Phi->getType())) {
+        Const = dyn_cast<SCEVConstant>(SE.getSCEV(Phi));
+        if (Const) {
+          V = Const->getValue();
+          if (V->getType() != Phi->getType())
+            continue;
+        }
+      }
+    }
+
+    if (V) {
       Phi->replaceAllUsesWith(V);
       DeadInsts.emplace_back(Phi);
       ++NumElim;
Index: test/Transforms/IndVarSimplify/const_phi.ll
===================================================================
--- /dev/null
+++ test/Transforms/IndVarSimplify/const_phi.ll
@@ -0,0 +1,33 @@
+; RUN: opt < %s -indvars -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; PR25372
+; We can compute the expression of %storemerge, and that is a SCEV
+; constant. However, instcombine can't deduce this, so we can
+; potentially end up trying to handle a constant when replacing 
+; congruent IVs.
+
+; CHECK-LABEL: crash
+define void @crash() {
+entry:
+  br i1 false, label %not_taken, label %pre
+
+not_taken:
+  br label %pre
+
+pre:
+; %phi0.pre and %phi1.pre are evaluated by SCEV to constant 0.
+  %phi0.pre = phi i32 [ 0, %entry ], [ 2, %not_taken ]
+  %phi1.pre = phi i32 [ 0, %entry ], [ 1, %not_taken ]
+  br label %loop
+
+loop:
+; %phi0 and %phi1 are evaluated by SCEV to constant 0.
+  %phi0 = phi i32 [ 0, %loop ], [ %phi0.pre, %pre ]
+  %phi1 = phi i32 [ 0, %loop ], [ %phi1.pre, %pre ]
+  br i1 undef, label %exit, label %loop
+
+exit:
+  ret void
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14230.38903.patch
Type: text/x-patch
Size: 2041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151102/85d4f893/attachment.bin>