InnerLoopVectorizer's code called during VPlan execution still relies on
original IR's def-use relations to decide which vector code to generate,
limiting VPlan transformations ability to modify def-use relations and still
have ILV generate the vector code.
This commit moves GEP operand queries controlling how GEPs are widened to a
dedicated recipe and extracts GEP widening code to its own ILV method taking
those recorded decisions as arguments. This reduces ingredient def-use usage by
ILV as a step towards full VPlan-based def-use relations.
Details
Diff Detail
Event Timeline
Having special treatment for GEPs here does follow the unique way in which GEPs tolerate both scalar/invariant operands and/or vector operands.
Separate the VPWidenMemoryInstructionRecipe changes from those of VPWidenGEPRecipe, into two patches?
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
3985 | A GEP having only loop-invariant operands should be treated like any other uniform instruction, i.e., by a VPReplicateRecipe rather than a VPWidenGEPRecipe. This TODO is related to D68831 BTW. | |
6931 | Would it be better and/or more in line with the current scheme, if this handling of GEPs by a designated Recipe took place outside the generic tryToWiden() which employs the default VPWidenRecipe? | |
llvm/lib/Transforms/Vectorize/VPlan.h | ||
595 | Keep in lex order. | |
745 | Update comment from phi to gep. | |
750 | Holding a LoopInvariantIndices set would be worse than IsIndexLoopInvariant bit vector, right? |
llvm/lib/Transforms/Vectorize/VPlan.h | ||
---|---|---|
750 | I think so. |
- Limit this commit to handling GEP widening only, per Ayal's comment.
- Bug fix: The all-operands-are-invariant check was not checking the pointer's invariance.
This looks good to me, thanks, plus a couple of comments.
Worth noting that GEPs will now stop the extensibility of WidenRecipe's compaction optimization. But this compaction should eventually be removed to represent def-use relations.
Let's wait a few days if anyone else would like to review.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
4011 | Can use GEP->indices() instead. | |
6899 | Checking if I isa PHINode here is completely redundant, given that all phi's were handled before tryToWiden(). But this cleanup calls for a separate patch. | |
6935–6936 | This setRecipe handles GEPs, and should continue to do so. | |
7059 | Missing setRecipe(...) of the new recipe here. | |
llvm/lib/Transforms/Vectorize/VPlan.cpp | ||
665 | Can also print the in/variance information, although printing the GEP seems sufficient. | |
llvm/lib/Transforms/Vectorize/VPlan.h | ||
758 | Can use GEP->indices() instead. |
LGTM, thanks for working on this!
llvm/lib/Transforms/Vectorize/VPlan.h | ||
---|---|---|
758 | nit: you could also use enumerate(GEP->indices()) and get rid of the extra counter I, which might be a bit more explicit |
A GEP having only loop-invariant operands should be treated like any other uniform instruction, i.e., by a VPReplicateRecipe rather than a VPWidenGEPRecipe. This TODO is related to D68831 BTW.