[PATCH] D68577: [LV] Apply sink-after & interleave-groups as VPlan transformations (NFC)

[Florian Hahn via Phabricator via llvm-commits]

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

LGTM, thanks for getting things started in that direction!

Just a few optional small nits.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6895
+  // to avoid having to split recipes later.
+  bool Singleton = Ingredient2Recipe.count(I);
+
----------------
nit: could be changed to IsSingleton, to make it clearer we this is a boolean.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7065
+  // ---------------------------------------------------------------------------
+  // Pre-construction: record ingredients whose Recipes we'll need to further
+  // process after constructing the initial VPlan.
----------------
nit: [r]ecipes?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7069
+
+  // Mark Instructions we'll need to sink later and their targets as
+  // ingredients whose Recipe we'll need to record.
----------------
nit:  [i]nstructions? 


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:53
+  // those ingredients get a VPWidenRecipe, also avoid compressing other
+  // ingredients into it to avoid having to split such recipes later.
+  DenseMap<Instruction *, VPRecipeBase *> Ingredient2Recipe;
----------------
It may be worth mentioning the implicit contract: to mark a recipe for recording, the instruction needs to be added with a nullptr value.


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:57
+
+  /// Set the recipe created for given ingredient.
+  void setRecipe(Instruction *I, VPRecipeBase *R) {
----------------
maybe mention that we only record only if the instruction is already in the map.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68577/new/

https://reviews.llvm.org/D68577