[PATCH] D90564: [VPlan] Make VPRecipeBase inherit from VPDef.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:622
+class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock>,
+                     public VPDef {
   friend VPBasicBlock;
----------------
gilr wrote:
> Should there be a VPRecipeBase::classof(VPDef *) to support dyn_cast from VPDef to VPRecipeBase?
yes I added one. For now, it just returns true, as all VPDefs are also VPRecipeBases at the moment. If they diverge, we need to check the IDs.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:693
 
-inline bool VPUser::classof(const VPRecipeBase *Recipe) {
-  return Recipe->getVPRecipeID() == VPRecipeBase::VPInstructionSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPWidenSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPWidenCallSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPWidenSelectSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPWidenGEPSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPBlendSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPInterleaveSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPReplicateSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPBranchOnMaskSC ||
-         Recipe->getVPRecipeID() == VPRecipeBase::VPWidenMemoryInstructionSC;
+inline bool VPUser::classof(const VPDef *Recipe) {
+  return Recipe->getVPDefID() == VPRecipeBase::VPInstructionSC ||
----------------
gilr wrote:
> Recipe -> Def
I also updated the other `classof` calls which were using `VPDef *V`.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:710
 /// the VPInstruction is also a single def-use vertex.
-class VPInstruction : public VPUser, public VPValue, public VPRecipeBase {
+class VPInstruction : public VPValue, public VPUser, public VPRecipeBase {
   friend class VPlanSlp;
----------------
gilr wrote:
> Worth documenting here too the importance of the inheritance order.
I added a comment to `VPRecipeBase`. Otherwise we would have to add the same comment to all subclasses of VPRecipeBase. Do you think that is sufficient or should I add the comment to the subclasses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90564