[PATCH] D46826: [VPlan] Add VPlan based sinkInstructions utility.

[Diego Caballero via Phabricator via llvm-commits]

dcaballe accepted this revision.
dcaballe added a comment.
This revision is now accepted and ready to land.

Much better with this VPlanHCFGTransforms! Thanks a lot!
Just some minor comments. LGTM. Again, let's wait for https://reviews.llvm.org/D46827 to see if this code is still necessary.

Thanks!



================
Comment at: lib/Transforms/Vectorize/VPlanHCFGTransforms.cpp:26
+  DenseMap<Instruction *, VPInstruction *> SinkAfterInverse;
+  VPRegionBlock *TopRegion = dyn_cast<VPRegionBlock>(Plan->getEntry());
+  ReversePostOrderTraversal<VPBlockBase *> RPOT(TopRegion->getEntry());
----------------
assert(isa) + cast?


================
Comment at: lib/Transforms/Vectorize/VPlanHCFGTransforms.cpp:35-36
+
+      Instruction *Instr = dyn_cast<Instruction>(VPInst->getUnderlyingValue());
+      assert(Instr && "Need Instruction as underlying value");
+
----------------
same here? assert(isa) + cast?


================
Comment at: lib/Transforms/Vectorize/VPlanValue.h:41
   friend class VPBuilder;
+  friend class VPlanHCFGTransforms;
+
----------------
Hopefully this won't promote the use of the underlying IR for other VPlan-to-VPlan transformations when it's not necessary.


https://reviews.llvm.org/D46826