[PATCH] D17153: [LV] Identify more induction PHIs by coercing expressions to AddRecExprs

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

sbaranga added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4594-4627
@@ -4584,36 +4593,36 @@
         InductionDescriptor ID;
-        if (InductionDescriptor::isInductionPHI(Phi, PSE.getSE(), ID)) {
+        auto ValidateInductionPhi = [&]() {
           Inductions[Phi] = ID;
           // Get the widest type.
           if (!WidestIndTy)
             WidestIndTy = convertPointerToIntegerType(DL, PhiTy);
           else
             WidestIndTy = getWiderType(DL, PhiTy, WidestIndTy);
 
           // Int inductions are special because we only allow one IV.
           if (ID.getKind() == InductionDescriptor::IK_IntInduction &&
               ID.getStepValue()->isOne() &&
               isa<Constant>(ID.getStartValue()) &&
                 cast<Constant>(ID.getStartValue())->isNullValue()) {
             // Use the phi node with the widest type as induction. Use the last
             // one if there are multiple (no good reason for doing this other
             // than it is expedient). We've checked that it begins at zero and
             // steps by one, so this is a canonical induction variable.
             if (!Induction || PhiTy == WidestIndTy)
               Induction = Phi;
           }
 
           DEBUG(dbgs() << "LV: Found an induction variable.\n");
 
           // Until we explicitly handle the case of an induction variable with
           // an outside loop user we have to give up vectorizing this loop.
           if (hasOutsideLoopUser(TheLoop, &*it, AllowedExit)) {
             emitAnalysis(VectorizationReport(&*it) <<
                          "use of induction value outside of the "
                          "loop is not handled by vectorizer");
             return false;
           }
 
-          continue;
-        }
+          return true;
+        };
 
----------------
anemet wrote:
> Why did you choose a lambda rather than a helper for this?  This is a pretty big function and the lambda is not small either.  Unless there is  a strong reason I would prefer factoring this part out into a helper.
There's no strong reason for this (it just avoided passing a lot of arguments). I'll convert it to a helper function.

================
Comment at: test/Transforms/LoopVectorize/induction.ll:171
@@ +170,3 @@
+; The SCEV expression of %sphi is (zext i8 {%t,+,1}<%loop> to i32)
+; In order to recognize it as an induction PHI and vectorize this loop,
+; we need to convert the SCEV expression into an AddRecExpr.
----------------
anemet wrote:
> Please comment which induction PHI you mean here?  Same on the other testcase.
Sure! (it's %sphi in both tests)


http://reviews.llvm.org/D17153