[PATCH] D84684: [VPlan] Use VPValue def for VPInterleaveRecipe.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

> Agreed, we could also model it so that VPInterleaveRecipe creates a single vector with all loaded values and then use extract instructions for the different parts of the interleave group. This keeps the modeling in the VPValue classes & co a bit simpler. But it has the cost that we need to add additional instructions/recipes to the plan, which need to be considered by other analyses and transformations.  Another problem is that we would have to materialize a vector with all loaded values, which is completely artificial.

I was considering that the VPInterleaveRecipe and the VPExtract's would work in tandem. You would never get the "all loaded values" vector, it would just create the interleaving gorup as it does at the moment, with the VPExtracts representing the values for each item. They would still be special values, VPExtracts would be the only type of recipe that could use a VPInterleave, and the VPExtract would not really contain anything in it's execute method.

You could just consider the VPExtract as a new type of VPUser, not a VPRecipe directly. It's really a convenient way of using the operands to join up the two elements, not having to rely upon UnderlyingOrProducerTy. I guess that's a lot like the VPMultiValue concept. Using a new type of VPUser removes the complexity from VPValue at least!

> I think we have the make the following trade-off. Either we add additional complexity to VPValue & co to model recipes that produce multiple VPValues or we add additional instructions/recipes to the plans we generate.
>
> Personally I think the additional complexity in VPValue & co is worth having simpler plans. Also, there might be additional complex recipes that produce multiple values in the future, which would also benefit from dedicated multi-value support.

It feels like we have one type system (VPValueSC/VPInstructionSC/VPWidenSC/etc), and we are adding another system on top of it, for different types of VPValue via UnderlyingOrProducerTy being virtual or subvalues or concrete.

Is there a nice way at the moment of detecting _which_ output you are seeing from a subvalue VPValue? As in which member index it would be from a interleaved group.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:966
 void VPValue::replaceAllUsesWith(VPValue *New) {
+  if (isVirtual()) {
+    auto *IR = cast<VPInterleaveRecipe>(this);
----------------
Is this used at the moment? Should we be blindly replacing all the different uses with the same value?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1035
                      VPValue *Mask)
-      : VPRecipeBase(VPInterleaveSC), VPUser({Addr}), IG(IG) {
+      : VPRecipeBase(VPInterleaveSC), VPValue(VPValue::VPVInterleaveSC),
+        VPUser({Addr}), IG(IG) {
----------------
-> VPRecipeBase::VPInterleaveSC


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1040
+    for (unsigned i = 0; i < IG->getNumMembers(); i++)
+      Defs.push_back(new VPValue(this));
   }
----------------
Does anything delete these Defs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84684