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

[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!



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7782
-          if (NeedDef.contains(Instr))
-            Plan->addOrReplaceVPValue(Instr, MaybeVPValue);
-          else
----------------
I think this is the only user of `addOrReplaceVPValue`. If so, it would be good to remove it.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7932
           UseReductionIntrinsic);
+      WidenRecipe->toVPValue()->replaceAllUsesWith(cast<VPValue>(RedRecipe));
+      Plan->removeVPValueFor(R);
----------------
`cast<VPValue>(RedRecipe)` should not be needed, because `RedRecipe` is of type `VPReductionRecipe`, which in turn inherits from VPValue?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:847
   O << "\"WIDEN\\l\"";
-  O << "\"  " << VPlanIngredient(&Ingredient);
+  O << "\"  " << VPlanIngredient(getUnderlyingInstr());
 }
----------------
fhahn wrote:
> 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.
Thanks! There's now a `printOperands()` function that could be used to print the operands here.


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

https://reviews.llvm.org/D88447