[PATCH] D84680: [VPlan] Use VPValue def for VPMemoryInstructionRecipe.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I could not find a review for VPWidenRecipe, so I put one up in D88447 <https://reviews.llvm.org/D88447>, that also has some adjustment for how VPValues are registered.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:513
   void vectorizeMemoryInstruction(Instruction *Instr, VPTransformState &State,
-                                  VPValue *Addr, VPValue *StoredValue,
-                                  VPValue *BlockInMask);
+
+                                  VPValue *Def, VPValue *Addr,
----------------
There's an extra newline here, that looks like it came from a merge conflict.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6958
+    auto *WidenLoad = new VPWidenMemoryInstructionRecipe(*Load, Addr, Mask);
+    Plan->addVPValue(Load, WidenLoad);
+    return WidenLoad;
----------------
It might be worth thinking about doing this after the call to RecipeBuilder.tryToCreateWidenRecipe, so that it's done for all recipes at once.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1167
+class VPWidenMemoryInstructionRecipe : public VPRecipeBase, public VPValue {
   VPUser User;
 
----------------
Would we want to make this inherit from VPUser as well?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1582
 
+  SmallVector<VPValue *, 16> VPValuesToFree;
+
----------------
Most variables here have a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84680