[PATCH] D76838: [LV][LoopInfo] Transform counting-down loops to counting-up loop

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:220
+//
+PHINode *Loop::reverseInductionVariable(ScalarEvolution &SE) {
+  BasicBlock *Preheader = getLoopPreheader();
----------------
Ayal wrote:
> SjoerdMeijer wrote:
> > Meinersbur wrote:
> > >  `Loop` is part of the `LLVMAnalysis` library. Transformations should be in the `LLVMTransform` library.
> > > 
> > > Also, it is untypical for the analysis results to have methods for modification as well. These are typically found in the LLVMTransform library such as `LoopUtils.cpp`
> > Thanks for the suggestion, I'm happy to move this to LoopUtils.cpp. Also checking with @Ayal, is this something we can all live with? 
> Constructing a canonical induction if one is missing and needed for LV's FoldTail, using SCEV::getBackedgeTakenCount() instead of pattern matching, could be placed in LoopUtils.cpp (or in LoopVectorize.cpp, using ILV::createInductionVariable()).
> 
> The concern is that doing so might cause slowdowns if the loop is not vectorized, something LV has been trying hard to avoid, so far successfully, which also motivated VPlan. (BTW, doing so may also cause slowdowns even if the loop is vectorized, but that's a different story ;-). Reverting control to the original IV if the loop is not vectorized seems awkward at best.
> 
> Let's try to think of alternative (1), i.e., of having LV represent in VPlan the new PIV that it will eventually create. A new PrimaryInductionRecipe  (or VPInstruction) can be introduced and placed at the beginning of the first VPBasicBlock; its execute() will create a Phi-starting-at-zero, set ILV::Induction and possibly a PIV VPValue to it; the bump and compare could be taken care of in ILV.fixVectorizedLoop().
> Interested in following this approach?
Thanks for the suggestion Ayal. My initial thought was that this sounds like a lot of different moving parts for a simple thing like this. But if it overcomes the problem of doing this transformation unnecessary, then that sounds like a good plan. And also, creating a vplan recipe migt not be that bad, i.e. actually a good fit for this. I need to get up to speed with how the vplan machinery works, which is what I am doing first at the moment. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76838/new/

https://reviews.llvm.org/D76838