[PATCH] D32871: [LV] Using VPlan to model the vectorized code and drive its transformation

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7805
+  VPWidenIntOrFpInductionRecipe(PHINode *IV, TruncInst *Trunc = nullptr)
+      : VPRecipeBase(VPWidenIntOrFpInductionSC), IV(IV), Trunc(Trunc) {}
+
----------------
shouldn't you have some asserts here to make sure the PHI node is what you need it to be?

Same for other VPlans.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:8017
+void LoopVectorizationPlanner::buildVPlans(unsigned MinVF, unsigned MaxVF) {
+  while (MinVF < MaxVF + 1) {
+    VFRange Range = {MinVF, MaxVF + 1};
----------------
I know you can re-use MinVF, but it would be more readable if you created a new variable VF to use inside the loop.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:8022
+    std::string PlanName;
+    raw_string_ostream RSO(PlanName);
+    unsigned VF = MinVF;
----------------
This is confusing. Is this RSO *just* for the plan name?

It'd seem more appropriate to have the name be a property of the plans, not some outside property.


https://reviews.llvm.org/D32871