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

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Hi Florian. Some comments, regardless my previous comments in `D46827`:

1. Location: Same as in `D46827`, we may need to find a better place. Maybe a VPlanHCFGTransforms class for now?
2. Just a thought for the future: I think `sinkAfter` could be one of those things that we may want to rethink/re-implement a bit in the VPlan native path. If it's only used for interleave groups, we may want to include the `sinkAfter` analysis and transformation steps as part of the future interleave groups VPlan-to-VPlan transformation.

Thanks,
Diego



================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.cpp:20
+
+#define DEBUG_TYPE "loop-vectorize"
+
----------------
I'm using independent DEBUG_TYPE in D44338 for VPlan verifier and HCFGConstruction files. I guess we should be consistent. We could use 'loop-vectorize' for all of them or also use an independent one for this.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:258
 
+  /// Sinks instructions in Plan, depending on their underlying values in
+  /// SinkAfter.
----------------
Is `\p` still used for documenting parameters?


================
Comment at: lib/Transforms/Vectorize/LoopVectorizationPlanner.h:260
+  /// SinkAfter.
+  // FIXME: Migrate to using a VPlan based mapping, once LoopVectorizationLegality::getSinkAfter
+  //        is moved the VPlan.
----------------
format: beyond 80 chars?


https://reviews.llvm.org/D46826