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

[Eli Friedman via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:571
   if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(LHS)) {
     if (IgnoreSignificantBits || isAddRecSExtable(AR, SE)) {
       const SCEV *Step = getExactSDiv(AR->getStepRecurrence(SE), RHS, SE,
----------------
labrinea wrote:
> Like this:
> ```
> -    if (IgnoreSignificantBits || isAddRecSExtable(AR, SE)) {
> +    if ((IgnoreSignificantBits || isAddRecSExtable(AR, SE)) && AR->isAffine()) {
> ```
> ?
Yes.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:3195
     // Split a non-zero base out of an addrec.
     if (AR->getStart()->isZero())
       return S;
----------------
labrinea wrote:
> Something like:
> ```
> -    if (AR->getStart()->isZero())
> +    if (AR->getStart()->isZero() || !AR->isAffine())
> ```
> ?
Yes.


================
Comment at: test/Transforms/LoopStrengthReduce/ARM/addrec-is-loop-invariant.ll:17
+bb1:
+  %mul.0 = mul i32 %c.0, %c.0
+  %gelptr.0 = getelementptr inbounds i16, i16* undef, i32 %mul.0
----------------
labrinea wrote:
> Here, %c.0 is used outside of loop0, where it is defined. Adding a PHI node for it prevents the issue. That's why I am mentioning the LCSSA form.
Oh, LCSSA hides the issue because SCEV doesn't look through LCSSA PHI nodes.  That doesn't really mean the lack of LCSSA is causing the problem, though... you could probably trigger this with IR in LCSSA form by messing with the loop nest (for example, make loop1 a subloop of loop0).


https://reviews.llvm.org/D26185