[PATCH] D47595: [VPlan] Move recipe construction to VPRecipeBuilder.

Diego Caballero via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 31 17:10:14 PDT 2018


dcaballe added a comment.

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



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6481
 
-bool LoopVectorizationPlanner::getDecisionAndClampRange(
+bool VPRecipeBuilder::getDecisionAndClampRange(
     const std::function<bool(unsigned)> &Predicate, VFRange &Range) {
----------------
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.


================
Comment at: lib/Transforms/Vectorize/VPRecipeBuilder.h:25
+
+class VPRecipeBuilder {
+  /// The loop that we evaluate.
----------------
Add brief description of this class?


================
Comment at: lib/Transforms/Vectorize/VPRecipeBuilder.h:79
+
+  // Check if \I is a memory instruction to be widened for \p Range.Start and
+  // potentially masked. Such instructions are handled by a recipe that takes an
----------------
```
// -> ///?
```


================
Comment at: lib/Transforms/Vectorize/VPlan.h:72
+using VPlanPtr = std::unique_ptr<VPlan>;
 
 /// In what follows, the term "input IR" refers to code that is fed into the
----------------
OK, this makes sense to me.


https://reviews.llvm.org/D47595





More information about the llvm-commits mailing list