[PATCH] D46827: [LV] Add VPInstruction to VPRecipe transformation.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D46827#1098894, @dcaballe wrote:

> I agree with you but I think it's too soon to enable some of this code in the inner loop vectorizer. I would wait at least until we have codegen support for outer loops so that we can do a more proper testing before we use it for production. In addition, another big concern is that introducing this code in the inner loop vectorizer too soon will limit the current development flexibility that we have in the VPlan native path. For those reasons, I would prefer this patch to just do the VPInstruction2VPRecipe transformation for the VPlan native path and leave the inner loop vectorizer changes for later. Does it make sense to you?


Yep, I think it would be better to move it to the VPlan native path, to have more flexibility when it comes to making changes.

The only problem is that the recipe generation depends on the cost model and we  probably have to duplicate some code from the inner loop vectorizer in the VPlan native path, until we gradually replace them by VPlan implementations. I think that would give us a stable starting point (the VPlan native path should pass all existing inner loop vectorizer tests) and would help us to work towards VPlan native inner loop vectorization at a steady pace. Does this make sense?



================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:363
+
+  VPlanPtr transformVPInstructionsToVPRecipies(VPlanPtr &OriginalPlan,
+                                               VFRange &Range);
----------------
dcaballe wrote:
> I think we should try to find a better place for this. The planner shouldn't be responsible for VPlan-to-VPlan transformations but just for doing the planning (orchestrating them). Otherwise, it will turn into a big brown bag of unrelated stuff.
> 
> Since this is a small and temporal thing and it's kind of related to code generation, maybe we could include it in ILV class as a "prepare" CG step?
> 
> Another option would be to create a specific class for it. We could rename VPlanHCFGBuilder.h/.cpp -> VPlanHCFGTransforms.h/.cpp and place there all the "small" VPlan-to-VPlan transformations.
Agreed I just put it there because I was not sure where to best put it. I will move it to VPlanHCFGTransforms.


https://reviews.llvm.org/D46827