[VPlan] Disconnect VPValue and VPUser.

[fhahn]

Summary

This refactors VPuser to not inherit from VPValue to facilitate
introducing operations that introduce multiple VPValues (e.g.
VPInterleaveRecipe).

[Ayal]

This looks good to me, as part of the effort to support VPlan def-use modeling and traversals.
Note that top-down traversal from VPValue to its VPUsers, will now need to check if each VPUser isa single-def recipe/VPInstruction in order to continue downwards to its VPUsers, etc., facilitating multi-def recipes.

llvm/lib/Transforms/Vectorize/VPlanSLP.cpp

164–165

nit: maybe worth reiterating next to the cast that

// Currently we only support VPInstructions.

225

nit: worth simplifying by setting here:

auto I1 = dyn_cast<VPInstruction>(V1);
auto I2 = dyn_cast<VPInstruction>(V2);

and reuse them below instead of isa<> and cast<>'s?

llvm/lib/Transforms/Vectorize/VPlanValue.h

12–13

The documentation of this class hierarchy deserves an update, as does docs/Proposals/VectorizationPlan.rst

[dmgreen]

I was trying very recently to get VPlan to perform some vector combines. Things like recognizing vmulh patterns.

The code I had was still in a very rough state but I ended up going in completely the opposite direction. I had VPRecipeBase inherit from VPUser, which in turn inherited from VPValue. Like I said it was all very rough and I'm not sure yet if it was necessary, but I felt like it simplified things. It was useful to know you could inspect operands and uses of any recipe, and properly walk up and down the def-uses chain. Perform things like replaceAllUsesWith and recursively delete dead recipes after replacing them with others.

Perhaps I need to look at that again, as I certainly hadn't tried to tackle multiple producing values. My gut instinct would be to use something like the SDValue from ISel, but I hadn't looked at all into what that would really look like. This way might well be better in the long run, I'm not sure. I had gotten distracted into cost modelling instead..

[fhahn]

This looks good to me, as part of the effort to support VPlan def-use modeling and traversals.
Note that top-down traversal from VPValue to its VPUsers, will now need to check if each VPUser isa single-def recipe/VPInstruction in order to continue downwards to its VPUsers, etc., facilitating multi-def recipes.

Thanks Ayal! I'll address the comments shortly.

In D84679#2285679, @dmgreen wrote:

I was trying very recently to get VPlan to perform some vector combines. Things like recognizing vmulh patterns.

The code I had was still in a very rough state but I ended up going in completely the opposite direction. I had VPRecipeBase inherit from VPUser, which in turn inherited from VPValue. Like I said it was all very rough and I'm not sure yet if it was necessary, but I felt like it simplified things. It was useful to know you could inspect operands and uses of any recipe, and properly walk up and down the def-uses chain. Perform things like replaceAllUsesWith and recursively delete dead recipes after replacing them with others.

I am not sure if it is completely the opposite direction.

With this patch, VPRecipeBase is also a VPUser and a VPValue, it just inherits from both directly, rather than indirectly through VPValue. Note that 'replacing all uses with & co' are provided by VPValue, so all instances that could previously be RAUW'd can still be RAUW'd. The only major difference should be that in the future we may need some extra code to get the uses of a VPUser.

This will help finishing the move to model the use-def chains entirely in VPlan, which in turn should make it possible to easily do VPlan-to-VPlan transformations, like the vector combines you mentioned.

Does that make sense?

[dmgreen]

With this patch, VPRecipeBase is also a VPUser and a VPValue, it just inherits from both directly, rather than indirectly through VPValue. Note that 'replacing all uses with & co' are provided by VPValue, so all instances that could previously be RAUW'd can still be RAUW'd. The only major difference should be that in the future we may need some extra code to get the uses of a VPUser.

Do you mean VPInstruction is a VPUser and a VPValue? I really meant that _all_ recipes were VPValues and VPUsers, moving it to VPRecipeBase.

Like I said, the code I have is very rough, and I'm not sure it's in any way mutually exclusive with going in this direction. I will try and rebase what I have onto your patches, working in the same way. This is the kind of transform it was doing (which was working pretty well it seemed, minus the costing not being accounted for):

static bool tryCombineToMulh(VPBasicBlock::reverse_iterator &It, VPlanPtr &Plan,
                             VPRecipeBuilder &RecipeBuilder) {
  VPWidenRecipe *RV = dyn_cast<VPWidenRecipe>(&*It);
  if (!RV)
    return false;

  // trunc(lsr(mul(ext, ext)), BW)) -> mulhs/mulhu
  Type *Ty = RV->getType();
  unsigned BW = Ty->getScalarSizeInBits();

  VPValue *Mul;
  // TODOD: One use checks?
  if (!match(RV, vp_Trunc(vp_LShr(vp_Value(Mul), vp_SpecificInt(BW)))))
    return false;

  VPValue *A, *B;
  unsigned Opcode = 0;
  if (match(Mul, vp_Mul(vp_SExt(vp_Value(A)), vp_SExt(vp_Value(B)))))
    Opcode = VPInstruction::Mulhs;
  else if (match(Mul, vp_Mul(vp_ZExt(vp_Value(A)), vp_ZExt(vp_Value(B)))))
    Opcode = VPInstruction::Mulhu;
  else
    return false;

  unsigned MulBW = Mul->getType()->getScalarSizeInBits();
  if (MulBW > BW * 2)
    return false;

  if (A->getType() != Ty || B->getType() != Ty) // TODOD: Generalize
    return false;

  auto *Mulh = new VPInstruction(Opcode, {A, B});
  RV->getParent()->insert(Mulh, RV->getIterator());
  RV->replaceAllUsesWith(Mulh);
  RV->recursivelyDeleteUnusedRecipes();
  It = Mulh->getReverseIterator();
  return true;
}

bool LoopVectorizationPlanner::adjustVPlanForVectorPatterns(
    VPlanPtr &Plan, VPRecipeBuilder &RecipeBuilder) {
  bool Changed = false;
  for (VPBlockBase *Block : depth_first(Plan->getEntry())) {
    if (auto *BB = dyn_cast<VPBasicBlock>(Block)) {
      for(VPBasicBlock::reverse_iterator It = BB->rbegin(), E = BB->rend(); It != E; ++It) {
        if (true /*TODOD: TTI->hasVectorPattern(Mulh)*/)
          Changed |= tryCombineToMulh(It, Plan, RecipeBuilder);
      }
    }
  }

  return Changed;
}

As all the vp_Mul etc are really looking at VPWidenRecipe, perhaps this can work simply enough. The idea was to probably make them match VPWidenRecipe or VPInstruction. recursivelyDeleteUnusedRecipes might be tougher, but perhaps still possible. I will see.

[fhahn]

In D84679#2285752, @dmgreen wrote:

With this patch, VPRecipeBase is also a VPUser and a VPValue, it just inherits from both directly, rather than indirectly through VPValue. Note that 'replacing all uses with & co' are provided by VPValue, so all instances that could previously be RAUW'd can still be RAUW'd. The only major difference should be that in the future we may need some extra code to get the uses of a VPUser.

Do you mean VPInstruction is a VPUser and a VPValue? I really meant that _all_ recipes were VPValues and VPUsers, moving it to VPRecipeBase.

Sorry, I did not mean that everything will be changed in this patch. But the linked patches gradually turn all the recipes into VPValues and update VPInterleaveRecipe to produce multiple values (and moves VPlan codegen to use that). So with the current patch alone, it is not yet possible to traverse def-use chains in recipes, but once the transition is done, it should be possible to traverse in both directions. (I think most recipes should already be ready to be turned into VPUser, let me check which ones are still missing)

As Ayal mentioned, when walking from defs to users there walker may need to check the user in order to continue traversing, but it should be relatively straight forward to provide some nice helpers for that.

It would be great to have some concrete simple transforms using it as soon as its ready to make sure everything works well together, so if we could collaborate on that, that would be great!

[dmgreen]

It would be great to have some concrete simple transforms using it as soon as its ready to make sure everything works well together, so if we could collaborate on that, that would be great!

Yeah, that sounds good. vmulh worked well I though, but I ended up making some fairly major modifications. And it did need to handle costing through vplan to really get anywhere. I will see if I can come up with anything for that too. I think just having a conversation about some of these design details is helpful in itself.

Sorry, I did not mean that everything will be changed in this patch. But the linked patches gradually turn all the recipes into VPValues and update VPInterleaveRecipe to produce multiple values (and moves VPlan codegen to use that). So with the current patch alone, it is not yet possible to traverse def-use chains in recipes, but once the transition is done, it should be possible to traverse in both directions. (I think most recipes should already be ready to be turned into VPUser, let me check which ones are still missing)

Ok. It sounds like you envision eventually that all VPRecipeBases' are VPValues then. I was jumping to that goal in one step. I liked making is part of VPRecipeBases as it simplifies the inheritance a lot.

I will try to at least update VPReductionRecipe, and we can see where we can go from there.

[fhahn]

Address Ayal's comments, thanks!

[fhahn]

In D84679#2286345, @dmgreen wrote:

It would be great to have some concrete simple transforms using it as soon as its ready to make sure everything works well together, so if we could collaborate on that, that would be great!

Yeah, that sounds good. vmulh worked well I though, but I ended up making some fairly major modifications. And it did need to handle costing through vplan to really get anywhere. I will see if I can come up with anything for that too. I think just having a conversation about some of these design details is helpful in itself.

Yes, VPlan based cost modeling is another big missing piece! But it should be possible to make progress in relatively small steps I think starting with the pattern matching & transformations?

Sorry, I did not mean that everything will be changed in this patch. But the linked patches gradually turn all the recipes into VPValues and update VPInterleaveRecipe to produce multiple values (and moves VPlan codegen to use that). So with the current patch alone, it is not yet possible to traverse def-use chains in recipes, but once the transition is done, it should be possible to traverse in both directions. (I think most recipes should already be ready to be turned into VPUser, let me check which ones are still missing)

Ok. It sounds like you envision eventually that all VPRecipeBases' are VPValues then. I was jumping to that goal in one step. I liked making is part of VPRecipeBases as it simplifies the inheritance a lot.

*Almost* all recipes can be VPValues (which the linked patches do), but some might not be VPValues directly but something that produces multiple VPValues (like an interleave group currently but there may be more in the future, the modeling should be able to capture this naturally). But that should make traversing only slightly more complicated. When you are just interested in recipes that are also VPValues, nothing should really change.

[Ayal]

This looks good to me, thanks!

[dmgreen]

Hmm. OK. I do strong believe that we should be moving towards def-use chains, so in general I'm glad to see that happening.

I'm less convinced about the multiple inheritance. That seems like an antipattern. We can likely make it work either way, but can you try and explain why you think that way is better? As opposed to making all VPRecipe's inherit from VPValue from VPRecipeBase? Perhaps I'm too stuck in the "llvm-ir" way of thinking, with VPValue being Value, VPUser being User, VPRecipeBase being Instruction and the individual recipes being the different instructions.

I have put up D88152 and D88153, to try and show the difference between what I came up with for the two approaches. (They are still very rough. Both can probably be made to work, I'm just not sure what the advantage of the multiple inheritance version is)

[fhahn]

In D84679#2290130, @dmgreen wrote:

Hmm. OK. I do strong believe that we should be moving towards def-use chains, so in general I'm glad to see that happening.

I'm less convinced about the multiple inheritance. That seems like an antipattern. We can likely make it work either way, but can you try and explain why you think that way is better? As opposed to making all VPRecipe's inherit from VPValue from VPRecipeBase? Perhaps I'm too stuck in the "llvm-ir" way of thinking, with VPValue being Value, VPUser being User, VPRecipeBase being Instruction and the individual recipes being the different instructions.

The main reason for migrating a recipe at a time is to keep the reviews small so we can make sure we properly update all places in the code that previously referenced the underlying instruction, e.g. during codegen. Another example is VPUser. I think we only want to make VPRecipeBase inherit from VPUser once *all* recipes manage their operands via VPUser. Otherwise you cannot rely on the VPUser operands being the source of truth.

In particular, just making VPRecipeBase inherit from VPValue without updating the existing code will result in subtle problems, e.g. when updating VPValue operands of a recipe, but codegen still using the original IR references.

Once all recipes are updated and inherit from VPValue we can move it to VPRecipeBase. The main caveat is VPInterleaveRecipe, which is not really a VPValue, but rather something that produces multiple values. We can also make that a VPValue, but not reference that particular value anywhere else, add all users of sub-values to the users of the 'virtual' VPValue and treat it as a convenient way to access all users of all sub-values. That's not completely clean, but we could also extract the user part into something else that can be inherited more cleanly from VPRecipeBase

I have put up D88152 and D88153, to try and show the difference between what I came up with for the two approaches. (They are still very rough. Both can probably be made to work, I'm just not sure what the advantage of the multiple inheritance version is)

Thanks for sharing. As mentioned earlier, the main reason at the moment is to ensure a step-by-step transition and once all recipes are migrated the common class can be hoisted to VPRecipeBase. I hope the explanation makes sense.

[dmgreen]

The main reason for migrating a recipe at a time is to keep the reviews small so we can make sure we properly update all places in the code that previously referenced the underlying instruction, e.g. during codegen. Another example is VPUser. I think we only want to make VPRecipeBase inherit from VPUser once *all* recipes manage their operands via VPUser. Otherwise you cannot rely on the VPUser operands being the source of truth.

Splitting things up sounds very smart. My question was less about doing that and more about whether we are heading towards the correct final design. And trying to show how that might look, when we work it though in a real problem. After all, the longer we go on using it, the harder it will become to change.

We could consider removing the VPUser concept entirely, for example. If there is only ever one thing that inherits from it (unlike llvm's User), the Operands could just be could be folded into VPRecipeBase.

In particular, just making VPRecipeBase inherit from VPValue without updating the existing code will result in subtle problems, e.g. when updating VPValue operands of a recipe, but codegen still using the original IR references.

Yep. That will certainly be very important for replacing recipes, as the vmulh patch does. Some ways to test some of the invariants would be useful as well.

Once all recipes are updated and inherit from VPValue we can move it to VPRecipeBase. The main caveat is VPInterleaveRecipe, which is not really a VPValue, but rather something that produces multiple values. We can also make that a VPValue, but not reference that particular value anywhere else, add all users of sub-values to the users of the 'virtual' VPValue and treat it as a convenient way to access all users of all sub-values. That's not completely clean, but we could also extract the user part into something else that can be inherited more cleanly from VPRecipeBase

I was thinking of complex nodes too, which may have multiple outputs. It will depend on how they are represented though, they may just remain as single output nodes, but you match them to operate differently on different lanes.

Thanks for sharing. As mentioned earlier, the main reason at the moment is to ensure a step-by-step transition and once all recipes are migrated the common class can be hoisted to VPRecipeBase. I hope the explanation makes sense.

Like I said I'm happy that we are moving towards the ability to do this better. It will be good to see VPlan being able to start to make some real improvements! If we are happy enough with this way of doing things, at least currently, then I can go and make some more sensibly sized patches :)

[fhahn]

Thanks for sharing. As mentioned earlier, the main reason at the moment is to ensure a step-by-step transition and once all recipes are migrated the common class can be hoisted to VPRecipeBase. I hope the explanation makes sense.

Like I said I'm happy that we are moving towards the ability to do this better. It will be good to see VPlan being able to start to make some real improvements! If we are happy enough with this way of doing things, at least currently, then I can go and make some more sensibly sized patches :)

I experimented a bit more and came up with another alternative: allow VPValue to model 3 different things: 1) concrete VPValues, 2) sub VPValues and 3) virtual VPValue: D88380

Managing those 3 in VPValue directly simplifies things further down a bit and I put up 2 more follow-on patches that turn VPReciepBase into a VPValue (D88379) and then also a VPUser (D88378). The last one also contains a unit test that looks upwards & downwards along the def-use chains. Note that we should probably move most/all remaining recipes to be VPValues and manage operands as VPUser before the last 2 patches.

Overall this probably results in a simpler structure overall. If we want, we could then also fold VPUser into VPRecipeBase.

[dmgreen]

I experimented a bit more and came up with another alternative: allow VPValue to model 3 different things: 1) concrete VPValues, 2) sub VPValues and 3) virtual VPValue: D88380

Managing those 3 in VPValue directly simplifies things further down a bit and I put up 2 more follow-on patches that turn VPReciepBase into a VPValue (D88379) and then also a VPUser (D88378). The last one also contains a unit test that looks upwards & downwards along the def-use chains. Note that we should probably move most/all remaining recipes to be VPValues and manage operands as VPUser before the last 2 patches.

Overall this probably results in a simpler structure overall. If we want, we could then also fold VPUser into VPRecipeBase.

Sounds good. Multiple results certainly does complicate things a bit. I have not thought a lot about them, but I agree this will be simpler overall from the things I was trying to write.

I've put up D88382 for VPReductionRecipe.

llvm/docs/Proposals/VectorizationPlan.rst

:VPRecipeBase:
   according to selected VF.
 
 :VPValue:
   The base of VPlan's def-use relations class hierarchy. When instantiated, it
   models a constant or a live-in Value in VPlan. It has users, which are of type
   VPUser, but no operands.
 
 :VPUser:
-  A VPValue representing a general vertex in the def-use graph of VPlan. It has
-  operands which are of type VPValue. When instantiated, it represents a
-  live-out Instruction that exists outside VPlan. VPUser is similar in some
-  aspects to LLVM's User class.
+  A VPUser represents an entity that uses a number of VPValues as operands.
+  VPUser is similar in some aspects to LLVM's User class.
 
 :VPInstruction:
   A VPInstruction is both a VPRecipe and a VPUser. It models a single
   VPlan-level instruction to be generated if the VPlan is executed, including
   its opcode and possibly additional characteristics. It is the basis for
   writing instruction-level analyses and optimizations in VPlan as creating,
   replacing or moving VPInstructions record both def-use and scheduling
   decisions. VPInstructions also extend LLVM IR's opcodes with idiomatic

llvm/lib/Transforms/Vectorize/VPlan.h

public:
   /// \returns an iterator pointing to the element after the erased one
   iplist<VPRecipeBase>::iterator eraseFromParent();
 };
 
 /// This is a concrete Recipe that models a single VPlan-level instruction.
 /// While as any Recipe it may generate a sequence of IR instructions when
 /// executed, these instructions would always form a single-def expression as
 /// the VPInstruction is also a single def-use vertex.
-class VPInstruction : public VPUser, public VPRecipeBase {
+class VPInstruction : public VPUser, public VPValue, public VPRecipeBase {
   friend class VPlanSlp;
 
 public:
   /// VPlan opcodes, extending LLVM IR with idiomatics instructions.
   enum {
     Not = Instruction::OtherOpsEnd + 1,
     ICmpULE,
     SLPLoad,
protected:
   Instruction *getUnderlyingInstr() {
     return cast_or_null<Instruction>(getUnderlyingValue());
   }
 
   void setUnderlyingInstr(Instruction *I) { setUnderlyingValue(I); }
 
 public:
   VPInstruction(unsigned Opcode, ArrayRef<VPValue *> Operands)
-      : VPUser(VPValue::VPInstructionSC, Operands),
+      : VPUser(Operands), VPValue(VPValue::VPInstructionSC),
         VPRecipeBase(VPRecipeBase::VPInstructionSC), Opcode(Opcode) {}
 
   VPInstruction(unsigned Opcode, std::initializer_list<VPValue *> Operands)
       : VPInstruction(Opcode, ArrayRef<VPValue *>(Operands)) {}
 
   /// Method to support type inquiry through isa, cast, and dyn_cast.
   static inline bool classof(const VPValue *V) {
     return V->getVPValueID() == VPValue::VPInstructionSC;

llvm/lib/Transforms/Vectorize/VPlanSLP.cpp

bool VPlanSlp::areVectorizable(ArrayRef<VPValue *> Operands) const {
 
   return true;
 }
 
 static SmallVector<VPValue *, 4> getOperands(ArrayRef<VPValue *> Values,
                                              unsigned OperandIndex) {
   SmallVector<VPValue *, 4> Operands;
   for (VPValue *V : Values) {
-    auto *U = cast<VPUser>(V);
+    // Currently we only support VPInstructions.
+    auto *U = cast<VPInstruction>(V);
     Operands.push_back(U->getOperand(OperandIndex));
   }
   return Operands;
 }
 
 static bool areCommutative(ArrayRef<VPValue *> Values) {
   return Instruction::isCommutative(
       cast<VPInstruction>(Values[0])->getOpcode());
static bool areConsecutiveOrMatch(VPInstruction *A, VPInstruction *B,
   auto *GB = IAI.getInterleaveGroup(B);
 
   return GA && GB && GA == GB && GA->getIndex(A) + 1 == GB->getIndex(B);
 }
 
 /// Implements getLAScore from Listing 7 in the paper.
 /// Traverses and compares operands of V1 and V2 to MaxLevel.
 static unsigned getLAScore(VPValue *V1, VPValue *V2, unsigned MaxLevel,
                            VPInterleavedAccessInfo &IAI) {
-  if (!isa<VPInstruction>(V1) || !isa<VPInstruction>(V2))
+  auto *I1 = dyn_cast<VPInstruction>(V1);
+  auto *I2 = dyn_cast<VPInstruction>(V2);
+  // Currently we only support VPInstructions.
+  if (!I1 || !I2)
     return 0;
 
   if (MaxLevel == 0)
-    return (unsigned)areConsecutiveOrMatch(cast<VPInstruction>(V1),
-                                           cast<VPInstruction>(V2), IAI);
+    return (unsigned)areConsecutiveOrMatch(I1, I2, IAI);
 
   unsigned Score = 0;
-  for (unsigned I = 0, EV1 = cast<VPUser>(V1)->getNumOperands(); I < EV1; ++I)
-    for (unsigned J = 0, EV2 = cast<VPUser>(V2)->getNumOperands(); J < EV2; ++J)
-      Score += getLAScore(cast<VPUser>(V1)->getOperand(I),
-                          cast<VPUser>(V2)->getOperand(J), MaxLevel - 1, IAI);
+  for (unsigned I = 0, EV1 = I1->getNumOperands(); I < EV1; ++I)
+    for (unsigned J = 0, EV2 = I2->getNumOperands(); J < EV2; ++J)
+      Score +=
+          getLAScore(I1->getOperand(I), I2->getOperand(J), MaxLevel - 1, IAI);
   return Score;
 }
 
 std::pair<VPlanSlp::OpMode, VPValue *>
 VPlanSlp::getBest(OpMode Mode, VPValue *Last,
                   SmallPtrSetImpl<VPValue *> &Candidates,
                   VPInterleavedAccessInfo &IAI) {
   assert((Mode == OpMode::Load || Mode == OpMode::Opcode) &&

llvm/lib/Transforms/Vectorize/VPlanValue.h

 //===- VPlanValue.h - Represent Values in Vectorizer Plan -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file contains the declarations of the entities induced by Vectorization
 /// Plans, e.g. the instructions the VPlan intends to generate if executed.
 /// VPlan models the following entities:
-/// VPValue
-///  |-- VPUser
-///  |    |-- VPInstruction
+/// VPValue   VPUser
+///    |        |
+///   VPInstruction
 /// These are documented in docs/VectorizationPlan.rst.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TRANSFORMS_VECTORIZE_VPLAN_VALUE_H
 #define LLVM_TRANSFORMS_VECTORIZE_VPLAN_VALUE_H
 
 #include "llvm/ADT/DenseMap.h"
void setUnderlyingValue(Value *Val) {
     UnderlyingVal = Val;
   }
 
 public:
   /// An enumeration for keeping track of the concrete subclass of VPValue that
   /// are actually instantiated. Values of this enumeration are kept in the
   /// SubclassID field of the VPValue objects. They are used for concrete
   /// type identification.
-  enum { VPValueSC, VPUserSC, VPInstructionSC };
+  enum { VPValueSC, VPInstructionSC };
 
   VPValue(Value *UV = nullptr) : VPValue(VPValueSC, UV) {}
   VPValue(const VPValue &) = delete;
   VPValue &operator=(const VPValue &) = delete;
 
   /// \return an ID for the concrete type of this object.
   /// This is used to implement the classof checks. This should not be used
   /// for any other purpose, as the values may change as LLVM evolves.

 
 typedef DenseMap<Value *, VPValue *> Value2VPValueTy;
 typedef DenseMap<VPValue *, Value *> VPValue2ValueTy;
 
 raw_ostream &operator<<(raw_ostream &OS, const VPValue &V);
 
 /// This class augments VPValue with operands which provide the inverse def-use
 /// edges from VPValue's users to their defs.
-class VPUser : public VPValue {
+class VPUser {
   SmallVector<VPValue *, 2> Operands;
 
-protected:
-  VPUser(const unsigned char SC) : VPValue(SC) {}
-  VPUser(const unsigned char SC, ArrayRef<VPValue *> Operands) : VPValue(SC) {
+public:
+  VPUser() {}
+  VPUser(ArrayRef<VPValue *> Operands) {
     for (VPValue *Operand : Operands)
       addOperand(Operand);
   }
 
-public:
-  VPUser() : VPValue(VPValue::VPUserSC) {}
-  VPUser(ArrayRef<VPValue *> Operands) : VPUser(VPValue::VPUserSC, Operands) {}
   VPUser(std::initializer_list<VPValue *> Operands)
       : VPUser(ArrayRef<VPValue *>(Operands)) {}
-  template <typename IterT>
-  VPUser(iterator_range<IterT> Operands) : VPValue(VPValue::VPUserSC) {
+  template <typename IterT> VPUser(iterator_range<IterT> Operands) {
     for (VPValue *Operand : Operands)
       addOperand(Operand);
   }
 
   VPUser(const VPUser &) = delete;
   VPUser &operator=(const VPUser &) = delete;
 
-  /// Method to support type inquiry through isa, cast, and dyn_cast.
-  static inline bool classof(const VPValue *V) {
-    return V->getVPValueID() >= VPUserSC &&
-           V->getVPValueID() <= VPInstructionSC;
-  }
-
   void addOperand(VPValue *Operand) {
     Operands.push_back(Operand);
     Operand->addUser(*this);
   }
 
   unsigned getNumOperands() const { return Operands.size(); }
   inline VPValue *getOperand(unsigned N) const {
     assert(N < Operands.size() && "Operand index out of bounds");