[PATCH] D44338: [LV][VPlan] Build plain CFG with simple VPInstructions for outer loops.

[Diego Caballero via Phabricator via llvm-commits]

dcaballe marked 9 inline comments as done.
dcaballe added a comment.

Thanks for spending time on this review, Florian! I addressed all your comments. I'll wait for one day or so before committing.
Regarding the DEBUG_TYPE, I think your suggestion makes more sense. I think it's better to have the complete picture.

Thanks,
Diego



================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:150
+    VPBuilder &Builder;
+    VPBasicBlock* Block;
+    VPBasicBlock::iterator Point;
----------------
fhahn wrote:
> nit: VPBasicBlock *Block?
Thanks! I'm applying formatting to all this code.


================
Comment at: lib/Transforms/Vectorize/VPlan.h:1042
+  // (operators '==' and '<').
+  SmallSet<VPValue *, 32> VPExternalDefs;
+
----------------
fhahn wrote:
> nit: 32 seems quite large
Ok, let's use 16. It would be easy to go over 8 for a double loop nest using a few memory references.


================
Comment at: lib/Transforms/Vectorize/VPlanVerifier.cpp:19
+
+#define DEBUG_TYPE "vplan-verifier"
+
----------------
fhahn wrote:
> I am not entirely sure how this will interact with `-debug-only`. IIUC if we do not use loop-vectorize here, those messages will be excluded from `-debug-only=loop-vectorize`. IMO it is convenient to get the complete picture with `-debug-only=loop-vectorize`.
Ok, it makes sense. I guess when you know exactly what your are looking for, having independent debug types helps you but we definitely lose the complete picture. Let me change it to loop-vectorize.

Thanks!


https://reviews.llvm.org/D44338