[VPlan] Use VPValue def for VPMemoryInstructionRecipe.

[fhahn]

Summary

This patch turns VPMemoryInstructionRecipe into a VPValue and uses it
during VPlan construction and codegeneration instead of the plain IR
reference where possible.

[dmgreen]

I could not find a review for VPWidenRecipe, so I put one up in D88447, that also has some adjustment for how VPValues are registered.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

513

There's an extra newline here, that looks like it came from a merge conflict.

6958

It might be worth thinking about doing this after the call to RecipeBuilder.tryToCreateWidenRecipe, so that it's done for all recipes at once.

llvm/lib/Transforms/Vectorize/VPlan.h

1167

Would we want to make this inherit from VPUser as well?

1582

Most variables here have a comment.

[fhahn]

Rebased & addressed comments, thanks!

In D84680#2299210, @dmgreen wrote:

I could not find a review for VPWidenRecipe, so I put one up in D88447, that also has some adjustment for how VPValues are registered.

That was indeed missing, thanks for putting up the patch.

[fhahn]

Move adding VPValue to plan up.

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

513

That's odd, it seems like clang-format did miss this one. Should be fixed now.

6958

We should definitely do that, but I'd thought it would be best to do so once we migrated most/all recipes.

I updated the code to try to convert the recipe to a VPValue after tryToCreateWidenRecipe. Unfortunately we won't eb able to use isa/cast/dyn_cast until VPRecipeBase inherits from VPValue, because otherwise we would cast between types that are not connected through inheritance. The main advantage would be to have a single place to update after the switch.

llvm/lib/Transforms/Vectorize/VPlan.h

1167

I've pushed a commit updating all classes that have a VPUser member to instead inherit from VPUser, which we should be able to do independently now that VPuser is not a VPValue (for now)

1582

I added a comment.

[dmgreen]

Thanks. This looks good to me, and I like the direction we are heading towards.

You might of course want to wait a day or two in case others disagree.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

6958

The new code looks OK I think, at least in the sort term.

llvm/lib/Transforms/Vectorize/VPlan.h

1167

Yeah I saw. Looks good.

[fhahn]

Thanks for taking a look! I rebased the patch and updated it with a change to replace all uses of VPWidenMemoryInstructionRecipe VPValues when replacing it with a VPInterleaveRecipe.

I'll update the follow-on patches shortly as well, they should now be finished in terms of functionality.

[fhahn]

Add getUnderlyingInstr to VPRecipeBase, using toVPValue.

[dmgreen]

llvm/lib/Transforms/Vectorize/VPlan.cpp

821

This looks like it will print mask twice.

llvm/lib/Transforms/Vectorize/VPlan.h

681

Hmm. I had not considered exposing getUnderlyingInstr at this level. I suppose that should work. Not all recipes with have Instructions and some of them might want different accessors returning the correct type (Phi's or GEP's etc).

The cast<Instruction> below should be dyn_cast_or_null<Instruction> if the method is expected to return nullptr.

There is a comment above getUnderlyingValue that says

// DESIGN PRINCIPLE: Access to the underlying IR must be strictly limited to
// the front-end and back-end of VPlan so that the middle-end is as
// independent as possible of the underlying IR. We grant access to the
// underlying IR using friendship. In that way, we should be able to use VPlan
// for multiple underlying IRs (Polly?) by providing a new VPlan front-end,
// back-end and analysis information for the new IR.

I'm not sure if I agree with that design principle, and wouldn't mind that comment being removed and getUnderlyingValue being exposed as public (it can make matching things like constants easier to recognise). If not then the same would apply here.

[fhahn]

Remove additional printing of the mask. Should be included when printing all operands

[fhahn]

make getUnderlyingInstr protected.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.cpp

821

Oh right, that's not need any more.

llvm/lib/Transforms/Vectorize/VPlan.h

681

Hmm. I had not considered exposing getUnderlyingInstr at this level. I suppose that should work. Not all recipes with have Instructions and some of them might want different accessors returning the correct type (Phi's or GEP's etc).

The cast<Instruction> below should be dyn_cast_or_null<Instruction> if the method is expected to return nullptr.

I think this should only be called from inside recipes that actually set the underlying value. I think it's fine to crash/assert otherwise. I think we can even push the responsibility to the caller to only use it for VPValues with an underlying value that is an instruction.

I'm not sure if I agree with that design principle, and wouldn't mind that comment being removed and getUnderlyingValue being exposed as public (it can make matching things like constants easier to recognise). If not then the same would apply here.

yeah, we should probably at least make those helpers protected, so they can only be used from within recipes. I think the long term goal is to de-couple the vpvalues/recipes so far from IR that we can generate IR without relying on references to the original IR.

Unfortunately, many recipes rely on some information attached to the original IR instructions (mostly for type info, but some other things too). I think it is beneficial to use VPValue's underlying value for the related IR instruction, as it allows us to get rid of the duplicated recipe-specific IR pointer and we also do not have to worry about the underlying value and other reference to diverge.

I made the functions protected, so the comment above still kind-of applies (and we were doing the same thing for VPInstruction already).

[fhahn]

use cast_or_null in getUnderlyingInstr.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.h

681

I think this should only be called from inside recipes that actually set the underlying value. I think it's fine to crash/assert otherwise. I think we can even push the responsibility to the caller to only use it for VPValues with an underlying value that is an instruction.

On second though, I think in some later patches actually make use of getUnderlyingInstr to return nullptr for non-VPValue. This should lead to slightly simpler code during the transition. Also made it cast_or_null.

[fhahn]

Rebased again. I plan to land this shortly to get things rolling.

[vitalybuka]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7424

this is a memory leak
http://lab.llvm.org:8014/#/builders/68/builds/1/steps/12/logs/stdio

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7424

Thanks! Recommitted with a fix and it seems like the staging msan bit is happy now

[fhahn]

Just realized I never closed this one with the re-commit. Recommitted a while ago as 93f6c6b79c500

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

public:
   /// the corresponding vector entry in VectorLoopValueMap is returned. If,
   /// however, the value has a scalar entry in VectorLoopValueMap, we construct
   /// a new vector value on-demand by inserting the scalar values into a vector
   /// with an insertelement sequence. If the value has been neither vectorized
   /// nor scalarized, it must be loop invariant, so we simply broadcast the
   /// value into a vector.
   Value *getOrCreateVectorValue(Value *V, unsigned Part);
 
+  void setVectorValue(Value *Scalar, unsigned Part, Value *Vector) {
+    VectorLoopValueMap.setVectorValue(Scalar, Part, Vector);
+  }
   /// Return a value in the new loop corresponding to \p V from the original
   /// loop at unroll and vector indices \p Instance. If the value has been
   /// vectorized but not scalarized, the necessary extractelement instruction
   /// will be generated.
   Value *getOrCreateScalarValue(Value *V, const VPIteration &Instance);
 
   /// Construct the vector value of a scalarized value \p V one lane at a time.
   void packScalarIntoVectorValue(Value *V, const VPIteration &Instance);
 
   /// Try to vectorize interleaved access group \p Group with the base address
   /// given in \p Addr, optionally masking the vector operations if \p
   /// BlockInMask is non-null. Use \p State to translate given VPValues to IR
   /// values in the vectorized loop.
   void vectorizeInterleaveGroup(const InterleaveGroup<Instruction> *Group,
                                 VPTransformState &State, VPValue *Addr,
                                 VPValue *BlockInMask = nullptr);
 
   /// Vectorize Load and Store instructions with the base address given in \p
   /// Addr, optionally masking the vector operations if \p BlockInMask is
   /// non-null. Use \p State to translate given VPValues to IR values in the
   /// vectorized loop.
   void vectorizeMemoryInstruction(Instruction *Instr, VPTransformState &State,
-                                  VPValue *Addr, VPValue *StoredValue,
-                                  VPValue *BlockInMask);
+
+                                  VPValue *Def, VPValue *Addr,
+                                  VPValue *StoredValue, VPValue *BlockInMask);
 
   /// Set the debug location in the builder using the debug location in
   /// the instruction.
   void setDebugLocFromInst(IRBuilder<> &B, const Value *Ptr);
 
   /// Fix the non-induction PHIs in the OrigPHIsToFix vector.
   void fixNonInductionPHIs(void);
 
for (unsigned Part = 0; Part < UF; Part++) {
     else
       NewStoreInstr =
           Builder.CreateAlignedStore(IVec, AddrParts[Part], Group->getAlign());
 
     Group->addMetadata(NewStoreInstr);
   }
 }
 
-void InnerLoopVectorizer::vectorizeMemoryInstruction(Instruction *Instr,
-                                                     VPTransformState &State,
-                                                     VPValue *Addr,
-                                                     VPValue *StoredValue,
-                                                     VPValue *BlockInMask) {
+void InnerLoopVectorizer::vectorizeMemoryInstruction(
+    Instruction *Instr, VPTransformState &State, VPValue *Def, VPValue *Addr,
+    VPValue *StoredValue, VPValue *BlockInMask) {
   // Attempt to issue a wide load.
   LoadInst *LI = dyn_cast<LoadInst>(Instr);
   StoreInst *SI = dyn_cast<StoreInst>(Instr);
 
   assert((LI || SI) && "Invalid Load/Store instruction");
   assert((!SI || StoredValue) && "No stored value provided for widened store");
   assert((!LI || !StoredValue) && "Stored value provided for widened load");
 
if (CreateGatherScatter) {
         NewLI =
             Builder.CreateAlignedLoad(DataTy, VecPtr, Alignment, "wide.load");
 
       // Add metadata to the load, but setVectorValue to the reverse shuffle.
       addMetadata(NewLI, LI);
       if (Reverse)
         NewLI = reverseVector(NewLI);
     }
-    VectorLoopValueMap.setVectorValue(Instr, Part, NewLI);
+
+    State.set(Def, Instr, NewLI, Part);
   }
 }
 
 void InnerLoopVectorizer::scalarizeInstruction(Instruction *Instr, VPUser &User,
                                                const VPIteration &Instance,
                                                bool IfPredicateInstr,
                                                VPTransformState &State) {
   assert(!Instr->getType()->isAggregateType() && "Can't handle vectors");
VPRecipeBuilder::tryToWidenMemory(Instruction *I, VFRange &Range,
   if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range))
     return nullptr;
 
   VPValue *Mask = nullptr;
   if (Legal->isMaskRequired(I))
     Mask = createBlockInMask(I->getParent(), Plan);
 
   VPValue *Addr = Plan->getOrAddVPValue(getLoadStorePointerOperand(I));
-  if (LoadInst *Load = dyn_cast<LoadInst>(I))
-    return new VPWidenMemoryInstructionRecipe(*Load, Addr, Mask);
+  if (LoadInst *Load = dyn_cast<LoadInst>(I)) {
+    auto *WidenLoad = new VPWidenMemoryInstructionRecipe(*Load, Addr, Mask);
+    Plan->addVPValue(Load, WidenLoad);
+    return WidenLoad;
+  }
 
   StoreInst *Store = cast<StoreInst>(I);
   VPValue *StoredValue = Plan->getOrAddVPValue(Store->getValueOperand());
   return new VPWidenMemoryInstructionRecipe(*Store, Addr, StoredValue, Mask);
 }
 
 VPWidenIntOrFpInductionRecipe *
 VPRecipeBuilder::tryToOptimizeInductionPHI(PHINode *Phi) const {
auto *Recipe = cast<VPWidenMemoryInstructionRecipe>(
         RecipeBuilder.getRecipe(IG->getInsertPos()));
     (new VPInterleaveRecipe(IG, Recipe->getAddr(), Recipe->getMask()))
         ->insertBefore(Recipe);
 
     for (unsigned i = 0; i < IG->getFactor(); ++i)
       if (Instruction *Member = IG->getMember(i)) {
         RecipeBuilder.getRecipe(Member)->eraseFromParent();
       }
   }
 
   // Finally, if tail is folded by masking, introduce selects between the phi
   // and the live-out instruction of each reduction, at the end of the latch.
   if (CM.foldTailByMasking()) {
     Builder.setInsertPoint(VPBB);
     auto *Cond = RecipeBuilder.createBlockInMask(OrigLoop->getHeader(), Plan);
     for (auto &Reduction : Legal->getReductionVars()) {
       VPValue *Phi = Plan->getVPValue(Reduction.first);
if (State.ValueMap.hasVectorValue(PredInst, Part)) {
     PHINode *Phi = State.Builder.CreatePHI(PredInstType, 2);
     Phi->addIncoming(UndefValue::get(ScalarPredInst->getType()), PredicatingBB);
     Phi->addIncoming(ScalarPredInst, PredicatedBB);
     State.ValueMap.resetScalarValue(PredInst, *State.Instance, Phi);
   }
 }
 
 void VPWidenMemoryInstructionRecipe::execute(VPTransformState &State) {
+  Instruction *Instr = getUnderlyingInstruction();
   VPValue *StoredValue = isa<StoreInst>(Instr) ? getStoredValue() : nullptr;
-  State.ILV->vectorizeMemoryInstruction(&Instr, State, getAddr(), StoredValue,
-                                        getMask());
+  State.ILV->vectorizeMemoryInstruction(Instr, State,
+                                        StoredValue ? nullptr : this, getAddr(),
+                                        StoredValue, getMask());
 }
 
 // Determine how to lower the scalar epilogue, which depends on 1) optimising
 // for minimum code-size, 2) predicate compiler options, 3) loop hints forcing
 // predication, and 4) a TTI hook that analyses whether the loop is suitable
 // for predication.
 static ScalarEpilogueLowering getScalarEpilogueLowering(
     Function *F, Loop *L, LoopVectorizeHints &Hints, ProfileSummaryInfo *PSI,
if (PreferPredicateOverEpilog ||
       (TTI->preferPredicateOverEpilogue(L, LI, *SE, *AC, TLI, DT,
                                         LVL.getLAI()) &&
        Hints.getPredicate() != LoopVectorizeHints::FK_Disabled))
     return CM_ScalarEpilogueNotNeededUsePredicate;
 
   return CM_ScalarEpilogueAllowed;
 }
 
+void VPTransformState::set(VPValue *Def, Value *IRDef, Value *V,
+                           unsigned Part) {
+  set(Def, V, Part);
+  ILV->setVectorValue(IRDef, Part, V);
+}
+
 // Process the loop in the VPlan-native vectorization path. This path builds
 // VPlan upfront in the vectorization pipeline, which allows to apply
 // VPlan-to-VPlan transformations from the very beginning without modifying the
 // input LLVM IR.
 static bool processLoopInVPlanNativePath(
     Loop *L, PredicatedScalarEvolution &PSE, LoopInfo *LI, DominatorTree *DT,
     LoopVectorizationLegality *LVL, TargetTransformInfo *TTI,
     TargetLibraryInfo *TLI, DemandedBits *DB, AssumptionCache *AC,

llvm/lib/Transforms/Vectorize/VPlan.h

struct VPTransformState {
   Value *get(VPValue *Def, const VPIteration &Instance) {
     // If the Def is managed directly by VPTransformState, extract the lane from
     // the relevant part. Note that currently only VPInstructions and external
     // defs are managed by VPTransformState. Other Defs are still created by ILV
     // and managed in its ValueMap. For those this method currently just
     // delegates the call to ILV below.
     if (Data.PerPartOutput.count(Def)) {
       auto *VecPart = Data.PerPartOutput[Def][Instance.Part];
+      if (!VecPart->getType()->isVectorTy()) {
+        assert(Instance.Lane == 0 && "cannot get lane > 0 for scalar");
+        return VecPart;
+      }
       // TODO: Cache created scalar values.
       return Builder.CreateExtractElement(VecPart,
                                           Builder.getInt32(Instance.Lane));
     }
 
     return Callback.getOrCreateScalarValue(VPValue2Value[Def], Instance);
   }
 
   /// Set the generated Value for a given VPValue and a given Part.
   void set(VPValue *Def, Value *V, unsigned Part) {
     if (!Data.PerPartOutput.count(Def)) {
       DataState::PerPartValuesTy Entry(UF);
       Data.PerPartOutput[Def] = Entry;
     }
     Data.PerPartOutput[Def][Part] = V;
   }
+  void set(VPValue *Def, Value *IRDef, Value *V, unsigned Part);
 
   /// Hold state information used when constructing the CFG of the output IR,
   /// traversing the VPBasicBlocks and generating corresponding IR BasicBlocks.
   struct CFGState {
     /// The previous VPBasicBlock visited. Initially set to null.
     VPBasicBlock *PrevVPBB = nullptr;
 
     /// The previous IR BasicBlock created or used. Initially set to the new
public:
   /// This method unlinks 'this' from the containing basic block and deletes it.
   ///
   /// \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 VPValue, public VPRecipeBase {
   friend class VPlanSlp;
 
 public:
   /// VPlan opcodes, extending LLVM IR with idiomatics instructions.
   enum {
     Not = Instruction::OtherOpsEnd + 1,

 };
 
 /// A Recipe for widening load/store operations.
 /// The recipe uses the following VPValues:
 /// - For load: Address, optional mask
 /// - For store: Address, stored value, optional mask
 /// TODO: We currently execute only per-part unless a specific instance is
 /// provided.
-class VPWidenMemoryInstructionRecipe : public VPRecipeBase {
-  Instruction &Instr;
+class VPWidenMemoryInstructionRecipe : public VPRecipeBase, public VPValue {
   VPUser User;
 
   void setMask(VPValue *Mask) {
     if (!Mask)
       return;
     User.addOperand(Mask);
   }
 
   bool isMasked() const {
-    return (isa<LoadInst>(Instr) && User.getNumOperands() == 2) ||
-           (isa<StoreInst>(Instr) && User.getNumOperands() == 3);
+    return (isa<LoadInst>(getUnderlyingInstruction()) &&
+            User.getNumOperands() == 2) ||
+           (isa<StoreInst>(getUnderlyingInstruction()) &&
+            User.getNumOperands() == 3);
   }
 
 public:
   VPWidenMemoryInstructionRecipe(LoadInst &Load, VPValue *Addr, VPValue *Mask)
-      : VPRecipeBase(VPWidenMemoryInstructionSC), Instr(Load), User({Addr}) {
+      : VPRecipeBase(VPWidenMemoryInstructionSC),
+        VPValue(VPValue::VPMemoryInstructionSC, &Load), User({Addr}) {
     setMask(Mask);
   }
 
   VPWidenMemoryInstructionRecipe(StoreInst &Store, VPValue *Addr,
                                  VPValue *StoredValue, VPValue *Mask)
-      : VPRecipeBase(VPWidenMemoryInstructionSC), Instr(Store),
+      : VPRecipeBase(VPWidenMemoryInstructionSC),
+        VPValue(VPValue::VPMemoryInstructionSC, &Store),
         User({Addr, StoredValue}) {
     setMask(Mask);
   }
 
   /// Method to support type inquiry through isa, cast, and dyn_cast.
   static inline bool classof(const VPRecipeBase *V) {
     return V->getVPRecipeID() == VPRecipeBase::VPWidenMemoryInstructionSC;
   }
 
   /// Return the address accessed by this recipe.
   VPValue *getAddr() const {
     return User.getOperand(0); // Address is the 1st, mandatory operand.
   }
 
   /// Return the mask used by this recipe. Note that a full mask is represented
   /// by a nullptr.
   VPValue *getMask() const {
     // Mask is optional and therefore the last operand.
     return isMasked() ? User.getOperand(User.getNumOperands() - 1) : nullptr;
   }
 
   /// Return the address accessed by this recipe.
   VPValue *getStoredValue() const {
-    assert(isa<StoreInst>(Instr) &&
+    assert(isa<StoreInst>(getUnderlyingInstruction()) &&
            "Stored value only available for store instructions");
     return User.getOperand(1); // Stored value is the 2nd, mandatory operand.
   }
 
   /// Generate the wide load/store.
   void execute(VPTransformState &State) override;
 
   /// Print the recipe.
   void print(raw_ostream &O, const Twine &Indent,
              VPSlotTracker &SlotTracker) const override;
+
+  Instruction *getUnderlyingInstruction() {
+    return cast<Instruction>(getUnderlyingValue());
+  }
+  const Instruction *getUnderlyingInstruction() const {
+    return cast<Instruction>(getUnderlyingValue());
+  }
 };
 
 /// A Recipe for widening the canonical induction variable of the vector loop.
 class VPWidenCanonicalIVRecipe : public VPRecipeBase {
   /// A VPValue representing the canonical vector IV.
   VPValue Val;
 
 public:
class VPlan {
   /// Represents the backedge taken count of the original loop, for folding
   /// the tail.
   VPValue *BackedgeTakenCount = nullptr;
 
   /// Holds a mapping between Values and their corresponding VPValue inside
   /// VPlan.
   Value2VPValueTy Value2VPValue;
 
+  SmallVector<VPValue *, 16> VPValuesToFree;
+
   /// Holds the VPLoopInfo analysis for this VPlan.
   VPLoopInfo VPLInfo;
 
   /// Holds the condition bit values built during VPInstruction to VPRecipe transformation.
   SmallVector<VPValue *, 4> VPCBVs;
 
 public:
   VPlan(VPBlockBase *Entry = nullptr) : Entry(Entry) {
     if (Entry)
       Entry->setPlan(this);
   }
 
   ~VPlan() {
     if (Entry)
       VPBlockBase::deleteCFG(Entry);
-    for (auto &MapEntry : Value2VPValue)
-      delete MapEntry.second;
+    for (VPValue *VPV : VPValuesToFree)
+      delete VPV;
     if (BackedgeTakenCount)
       delete BackedgeTakenCount;
     for (VPValue *Def : VPExternalDefs)
       delete Def;
     for (VPValue *CBV : VPCBVs)
       delete CBV;
   }
 
public:
   /// Add \p CBV to the vector of condition bit values.
   void addCBV(VPValue *CBV) {
     VPCBVs.push_back(CBV);
   }
 
   void addVPValue(Value *V) {
     assert(V && "Trying to add a null Value to VPlan");
     assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
-    Value2VPValue[V] = new VPValue(V);
+    VPValue *VPV = new VPValue(V);
+    Value2VPValue[V] = VPV;
+    VPValuesToFree.push_back(VPV);
+  }
+
+  void addVPValue(Value *V, VPValue *VPV) {
+    assert(V && "Trying to add a null Value to VPlan");
+    assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
+    Value2VPValue[V] = VPV;
   }
 
   VPValue *getVPValue(Value *V) {
     assert(V && "Trying to get the VPValue of a null Value");
     assert(Value2VPValue.count(V) && "Value does not exist in VPlan");
     return Value2VPValue[V];
   }
 
private:
   const Twine getUID(const VPBlockBase *Block);
 
   /// Print the information related to a CFG edge between two VPBlockBases.
   void drawEdge(const VPBlockBase *From, const VPBlockBase *To, bool Hidden,
                 const Twine &Label);
 
   void dump();
 
-  static void printAsIngredient(raw_ostream &O, Value *V);
+  static void printAsIngredient(raw_ostream &O, const Value *V);
 };
 
 struct VPlanIngredient {
-  Value *V;
+  const Value *V;
 
-  VPlanIngredient(Value *V) : V(V) {}
+  VPlanIngredient(const Value *V) : V(V) {}
 };
 
 inline raw_ostream &operator<<(raw_ostream &OS, const VPlanIngredient &I) {
   VPlanPrinter::printAsIngredient(OS, I.V);
   return OS;
 }
 
 inline raw_ostream &operator<<(raw_ostream &OS, const VPlan &Plan) {

llvm/lib/Transforms/Vectorize/VPlan.cpp

void VPlanPrinter::dumpRegion(const VPRegionBlock *Region) {
   assert(Region->getEntry() && "Region contains no inner blocks.");
   for (const VPBlockBase *Block : depth_first(Region->getEntry()))
     dumpBlock(Block);
   bumpIndent(-1);
   OS << Indent << "}\n";
   dumpEdges(Region);
 }
 
-void VPlanPrinter::printAsIngredient(raw_ostream &O, Value *V) {
+void VPlanPrinter::printAsIngredient(raw_ostream &O, const Value *V) {
   std::string IngredientString;
   raw_string_ostream RSO(IngredientString);
   if (auto *Inst = dyn_cast<Instruction>(V)) {
     if (!Inst->getType()->isVoidTy()) {
       Inst->printAsOperand(RSO, false);
       RSO << " = ";
     }
     RSO << Inst->getOpcodeName() << " ";

 
 void VPPredInstPHIRecipe::print(raw_ostream &O, const Twine &Indent,
                                 VPSlotTracker &SlotTracker) const {
   O << "\"PHI-PREDICATED-INSTRUCTION " << VPlanIngredient(PredInst);
 }
 
 void VPWidenMemoryInstructionRecipe::print(raw_ostream &O, const Twine &Indent,
                                            VPSlotTracker &SlotTracker) const {
-  O << "\"WIDEN " << VPlanIngredient(&Instr);
+  O << "\"WIDEN " << VPlanIngredient(getUnderlyingInstruction());
   O << ", ";
   getAddr()->printAsOperand(O, SlotTracker);
   VPValue *Mask = getMask();
   if (Mask) {
     O << ", ";
     Mask->printAsOperand(O, SlotTracker);
   }
 }
 
 void VPWidenCanonicalIVRecipe::execute(VPTransformState &State) {
   Value *CanonicalIV = State.CanonicalIV;

llvm/lib/Transforms/Vectorize/VPlanValue.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, VPInstructionSC };
+  enum { VPValueSC, VPInstructionSC, VPMemoryInstructionSC };
 
   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.