[PATCH] D49489: [VPlan] VPlan version of InterleavedAccessInfo.

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Just a few comments.

Thanks!
Diego



================
Comment at: lib/Transforms/Vectorize/VPlan.cpp:584
+      Old2New;
+
+  VPRegionBlock *TopRegion = dyn_cast<VPRegionBlock>(Plan.getEntry());
----------------
This algorithm assumes that there is only a region (top region) in the whole H-CFG but this is going to change pretty soon. Could you please change the implementation to recursively go inside other potential nested regions?


================
Comment at: lib/Transforms/Vectorize/VPlan.cpp:591
+      assert(isa<VPInstruction>(&*I) && "Can only handle VPInstructions");
+      VPInstruction *VPInst = cast<VPInstruction>(&*I);
+      Instruction *Inst = cast<Instruction>(VPInst->getUnderlyingValue());
----------------
`auto *` for casts and dyn_cast (585, 591, 592)?


================
Comment at: lib/Transforms/Vectorize/VPlan.h:1337
 };
 
+class VPInterleavedAccessInfo {
----------------
Just curious. How difficult would it be to templatize the IAI methods needed here? 


https://reviews.llvm.org/D49489