This should allow us to re-use the functionality in a
VPInstructionToVPRecipe transformation. I think it also makes sense to
move those functions out of LoopVectorizationPlanner, which should do
the high-level orchestration of the transformations.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Thanks for doing this refactoring! In general, moving all this code outside of the planner makes sense to me.
create*Mask and getDecisionAndClampRange are not strictly creating recipes but I understand the dependencies.
Minor comments inline.
Thanks,
Diego
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
6481 ↗ | (On Diff #149311) | Not a big deal but I wonder if it would be better to move this outside of VPRecipeBuilder since it's not strictly creating or manipulating any recipe and it will be needed eventually in the VPlan native path. |
lib/Transforms/Vectorize/VPRecipeBuilder.h | ||
25 ↗ | (On Diff #149311) | Add brief description of this class? |
79 ↗ | (On Diff #149311) | // -> ///? |
lib/Transforms/Vectorize/VPlan.h | ||
72 ↗ | (On Diff #149311) | OK, this makes sense to me. |
Thanks for having a look Diego!
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
6481 ↗ | (On Diff #149311) | Yep I think it would be better to move it somewhere else. I've moved it back to a static method in LoopVectorizationPlanner. Do you think there's a better place? |
Thanks, Florian! LGTM!
Please wait until we finish the other two related reviews to commit it, in case we have to change something else.
Thanks,
Diego
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
6481 ↗ | (On Diff #149311) | I think it's OK for now. We already have a VPBlockUtils and I envision a VPInstructionUtils. We should also have something similar for more generic stuff but we can introduce it when we have a clearer picture of it. |
Will do, thanks for having a look Diego!
lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
6481 ↗ | (On Diff #149311) | Yep I think that will be helpful in the future. |
As I updated D46827 to not rely on VPRecipeBuilder, but instead create a limited subset of recipes directly, do you think this is good to go?