[PATCH] D88380: [VPlan] Extend VPValue to also model sub- & 'virtual' values.

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:69
+//  Virtual VPValues are used to model instructions/recipes that either produce
+//  multiple subvalues or no values at all. A virtual VPValue does not refer to
+//  a concrete value, which means it cannot be used like concrete or subvalues.
----------------
"no values at all" could be concrete VPValues, just with void return type.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:129
     VPValueSC,
+    VPVirtualValueSC,
+    VPMultiValueSC,
----------------
When would VPVirtualValueSC be used?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:158-159
+    Users.push_back(&User);
+    if (isSubValue())
+      getDefiningValue()->Users.push_back(&User);
+  }
----------------
The defining recipe would be another user of the value? The sounds like it would complicate the number of uses. When would it be useful to store this in both places?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:129
+    VPVirtualValueSC,
+    VPMultiValueSC,
     VPInstructionSC,
----------------
Would a VPMultiValueSC ever be used, or would it always be a  VPVInterleaveSC? (Or whatever other recipe type it became)


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanValue.h:191
+
+  bool isSubValue() const { return UnderlyingOrProducer.is<true>(); }
+
----------------
Could this function just be checking for a new type of SubclassID?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88380