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

[James Molloy via llvm-commits]

jmolloy added inline comments.

================
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");
----------------
anemet wrote:
> 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.
Hi Adam,

This was a very good catch. It turns out there are no regression tests that can trigger this code, but 3 tests in the test-suite do.

It's possible for Count to have pointer type. But in fact, we calculate exactly the same value above (ExitCountValue) and test that for pointer-typedness. "Count" isn't needed.

So i'll remove this code, use ExitCountValue instead of Count and I'll also add a new testcase to make sure this case is triggered.


Repository:
  rL LLVM

http://reviews.llvm.org/D12286