[PATCH] D74695: [VPlan] Replace VPWidenRecipe with VPInstruction (WIP).

[Gil Rapaport via Phabricator via llvm-commits]

gilr added a comment.

In D74695#1923526 <https://reviews.llvm.org/D74695#1923526>, @fhahn wrote:

> I think doing so for VPWidenRecipe might be more tricky than other recipes, as VPWidenRecipe currently contains a list of instructions.
>  [snip]
>  I thought a little bit about how to migrate the def-use dependencies for VPWidenRecipe without breaking it up, but couldn't come up with a more straight-forward approach.


VPWidenRecipe wrapped intervals of ingredients just to keep VPlan's memory footprint low. Now that we begin to model def-use relations this mechanism will have to go (it's already suppressed for sink-after). We can start by removing compression altogether in a preliminary patch, turning VPWidenRecipe into a single-ingredient recipe, ready to take VPValue operands.
A following patch can add VPValue operands to VPWidenRecipe by calling `getOrAddVPValue()` for each IR operand, similar to D70865 <https://reviews.llvm.org/D70865>.
The next patch (perhaps also in parts) can replace VPWidenRecipes with VPInstructions by
(a) binding VP defs to IR users, as proposed
(b) extending the usage of `recordRecipe()` to ingredients covered by VPInstructions such that calling `getRecipe()` will return either a VPInstruction, another Recipe or nullptr. For the latter two cases, call `getOrAddVPValue()`.
(c) refactoring VPWidenRecipe's execute()
What do you think?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6982
+      VPV = new VPValue(Op);
+      Plan.addExternalDef(VPV);
+    } else
----------------
Not sure why ExternalDef is needed, as Value2VPValue is used for both variant and invariant values.


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:93
+  /// Return the recipe created for given ingredient.
+  VPInstruction *getVPInstruction(Value *V) {
+    auto I = dyn_cast<Instruction>(V);
----------------
Better reuse getRecipe(), allowing it to return nullptr in order to trivially support querying invariant values and ingredients whose recipes are not being recorded.


================
Comment at: llvm/lib/Transforms/Vectorize/VPRecipeBuilder.h:95
+    auto I = dyn_cast<Instruction>(V);
+    auto Iter = Ingredient2Recipe.find(I);
+    if (Iter == Ingredient2Recipe.end())
----------------
Requires recording the recipes for such ingredients.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:765
 
+class VPWidenInstruction : public VPInstruction {
 public:
----------------
Better avoid subclassing VPInstruction until scalar/type is modelled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74695