[PATCH] D12286: [LV] Never widen an induction variable.

[Adam Nemet via llvm-commits]

anemet added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:471-472
@@ -470,4 +470,4 @@
   PHINode *OldInduction;
-  /// Holds the extended (to the widest induction type) start index.
-  Value *ExtendedIdx;
+  /// Holds the start index.
+  Value *StartIdx;
   /// Maps scalars to widened vectors.
----------------
It's pretty strange to store the start idx when it's always zero...

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2618-2620
@@ +2617,5 @@
+  
+  // Legal->getInduction() checks the first two properties but not the third.
+  if (OldInduction && IdxTy != OldInduction->getType())
+    OldInduction = nullptr;
+  
----------------
This logic belongs to LoopVectorizationLegality::canVectorizeInstrs together with all the other conditions.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2683-2692
@@ -2682,12 +2693,12 @@
   } else {
     LI->addTopLevelLoop(Lp);
   }
   Lp->addBasicBlockToLoop(VecBody, *LI);
 
   // Use this IR builder to create the loop instructions (Phi, Br, Cmp)
   // inside the loop.
   Builder.SetInsertPoint(VecBody->getFirstNonPHI());
 
   // Generate the induction variable.
   setDebugLocFromInst(Builder, getDebugLocFromInstOrOperands(OldInduction));
   Induction = Builder.CreatePHI(IdxTy, 2, "index");
----------------
jmolloy wrote:
> (now refers to L2174)
> 
> This is becoming dead as part of this patch. This is the patch that enforces that IdxTy is no different from Induction->getType(). The previous patch only enforced that Induction->getStart() == 0.
I don't think so.  Count is derived from this:

  const SCEV *BackedgeTakeCount = SE->getNoopOrZeroExtend(ExitCount, IdxTy);

What worries me that if this code weren't dead and this comment was true:

  // The exit count can be of pointer type. Convert it to the correct
  // integer type.

then the code after the patch wouldn't handle this case.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3494
@@ -3553,3 +3493,3 @@
       Value *NormalizedIdx =
-          Builder.CreateSub(Induction, ExtendedIdx, "normalized.idx");
+          Builder.CreateSub(Induction, StartIdx, "normalized.idx");
       NormalizedIdx =
----------------
Aren't we subtracting zero here now?

Does this mean that we don't need to stash StartIdx?


Repository:
  rL LLVM

http://reviews.llvm.org/D12286