[PATCH] D26185: [ARM] Loop Strength Reduction crashes when targeting ARM or Thumb if the LLVM-IR is not in LCSSA form.

[Alexandros Lamprineas via llvm-commits]

labrinea created this revision.
labrinea added reviewers: sanjoy, jmolloy, silviu.baranga, llvm-commits.
Herald added subscribers: mzolotukhin, rengolin, aemerson.

Scalar Evolution asserts when not all the operands of an add-recurrence-expression are loop invariants. Loop Strength Reduction seems to assume that the LLVM-IR input is under LCSSA form which is not always the case.

This patchs prevents the code path from reaching the assertion by adding a couple of checks in Loop Strength Reduction which ensure that before creating an add-recurrence-expression all its operands are loop invariants. However, it might be safe to remove the assertion instead:

//lib/Analysis/ScalarEvolution.cpp//

  const SCEV *
  ScalarEvolution::getAddRecExpr(SmallVectorImpl<const SCEV *> &Operands,
                                  const Loop *L, SCEV::NoWrapFlags Flags) {
     if (Operands.size() == 1) return Operands[0];
   #ifndef NDEBUG
     Type *ETy = getEffectiveSCEVType(Operands[0]->getType());
     for (unsigned i = 1, e = Operands.size(); i != e; ++i)
       assert(getEffectiveSCEVType(Operands[i]->getType()) == ETy &&
              "SCEVAddRecExpr operand types don't match!");
     for (unsigned i = 0, e = Operands.size(); i != e; ++i)
       assert(isLoopInvariant(Operands[i], L) &&
              "SCEVAddRecExpr operand is not loop-invariant!");
   #endif

A corresponding bug has been raised here: https://llvm.org/bugs/show_bug.cgi?id=30325


https://reviews.llvm.org/D26185

Files:
  lib/Transforms/Scalar/LoopStrengthReduce.cpp
  test/Transforms/LoopStrengthReduce/ARM/addrec-is-loop-invariant.ll


Index: test/Transforms/LoopStrengthReduce/ARM/addrec-is-loop-invariant.ll
===================================================================
--- /dev/null
+++ test/Transforms/LoopStrengthReduce/ARM/addrec-is-loop-invariant.ll
@@ -0,0 +1,35 @@
+; RUN: llc -mtriple=armv8-eabi -verify-machineinstrs %s -o /dev/null
+
+; This test checks that when the IR is not under LCSSA form,
+; Loop Strength Reduction will not create an Add Reccurence
+; Expression if not all its operands are loop invariants.
+
+define void @add_rec_expr() {
+entry:
+  br label %loop0
+
+loop0:
+  %c.0 = phi i32 [ 0, %entry ], [ %inc.0, %loop0 ]
+  %inc.0 = add nuw i32 %c.0, 1
+  br i1 undef, label %loop0, label %bb1
+
+bb1:
+  %mul.0 = mul i32 %c.0, %c.0
+  %gelptr.0 = getelementptr inbounds i16, i16* undef, i32 %mul.0
+  br label %loop1
+
+loop1:
+  %inc.1 = phi i32 [ %inc.2, %bb4 ], [ 0, %bb1 ]
+  %mul.1 = mul i32 %inc.1, %c.0
+  br label %bb3
+
+bb3:
+  %add.0 = add i32 undef, %mul.1
+  %gelptr.1 = getelementptr inbounds i16, i16* %gelptr.0, i32 %add.0
+  store i16 undef, i16* %gelptr.1, align 2
+  br label %bb4
+
+bb4:
+  %inc.2 = add nuw i32 %inc.1, 1
+  br label %loop1
+}
Index: lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -325,7 +325,8 @@
 
   // Look at addrec operands.
   if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(S))
-    if (!AR->getStart()->isZero()) {
+    if (!AR->getStart()->isZero() &&
+        SE.isLoopInvariant(AR->getStepRecurrence(SE), AR->getLoop())) {
       DoInitialMatch(AR->getStart(), L, Good, Bad, SE);
       DoInitialMatch(SE.getAddRecExpr(SE.getConstant(AR->getType(), 0),
                                       AR->getStepRecurrence(SE),
@@ -578,7 +579,8 @@
       // FlagNW is independent of the start value, step direction, and is
       // preserved with smaller magnitude steps.
       // FIXME: AR->getNoWrapFlags(SCEV::FlagNW)
-      return SE.getAddRecExpr(Start, Step, AR->getLoop(), SCEV::FlagAnyWrap);
+      if (SE.isLoopInvariant(Step, AR->getLoop()))
+        return SE.getAddRecExpr(Start, Step, AR->getLoop(), SCEV::FlagAnyWrap);
     }
     return nullptr;
   }
@@ -3202,7 +3204,8 @@
       Ops.push_back(C ? SE.getMulExpr(C, Remainder) : Remainder);
       Remainder = nullptr;
     }
-    if (Remainder != AR->getStart()) {
+    if (Remainder != AR->getStart() &&
+        SE.isLoopInvariant(AR->getStepRecurrence(SE), AR->getLoop())) {
       if (!Remainder)
         Remainder = SE.getConstant(AR->getType(), 0);
       return SE.getAddRecExpr(Remainder,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26185.76547.patch
Type: text/x-patch
Size: 2680 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161101/b2a3d5a2/attachment.bin>