[PATCH] D88447: [VPlan] Switch VPWidenRecipe to be a VPValue

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Thanks for putting up this patch. This is indeed a conversion that also needs to be done and for which I didn't put up patches in the series.

Some comments inline. I think this might also need a rebase.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7912
           UseReductionIntrinsic);
+      if (VPValue* V = WidenRecipe->toVPValue()) {
+        V->replaceAllUsesWith(RedRecipe);
----------------
`WidenRecipe` is always guaranteed to be a `VPValue` here because it is either a VPWidenSelect or VPWiden recipe, right? I think it should be possible to call toVPValue unconditionally, or better just `cast<VPValue>(WidenRecipe)`, possible with a comment why it is safe?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7968
 void VPWidenRecipe::execute(VPTransformState &State) {
-  State.ILV->widenInstruction(Ingredient, *this, State);
+  State.ILV->widenInstruction(*getUnderlyingInstr(), *this, State);
 }
----------------
We also need to update widenInstruction to update `State` to register the result for the `VPValue` (replace `VectorLoopValueMap.setVectorValue(&I, Part, V);` with something like `State.set(Def, &I, V, Part);`, where `Def` is the `VPValue` of the widen recipe.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:847
   O << "\"WIDEN\\l\"";
-  O << "\"  " << VPlanIngredient(&Ingredient);
+  O << "\"  " << VPlanIngredient(getUnderlyingInstr());
 }
----------------
I think this needs updating to print something like `VPValue result = Opcode VPValue operands`.

Should be possible by using `printAsOperand(O, SlotTracker)` for the VPValue result, `Instruction::getOpcodeName` for the opcode and `printOperands` for the operands.


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

https://reviews.llvm.org/D88447