[VPlan] Add getLiveInIRValue accessor to VPValue.

[fhahn]

Summary

This patch adds a new getLiveInIRValue accessor to VPValue, which
returns the underlying value, if the VPValue is defined outside of
VPlan. This is required to handle scalars in VPTransformState, which
requires dealing with scalars defined outside of VPlan.

We can simply check VPValue::Def to determine if the value is defined
inside a VPlan.

[fhahn]

ping

[fhahn]

ping :)

[Ayal]

This currently requires giving access to VPValue to those classes, so we
can call the protected constructor. But most of them can be removed in
the future, once the remaining single value def recipes inherit from
VPValue.

Such access seems unneeded as commented below; but in general may have been better to handle these remaining recipes first and avoid multiple befriendings, albeit temporary.

llvm/lib/Transforms/Vectorize/VPlanValue.h

64

As explained above, Def is null iff this VPValue is a "live-in", i.e., defined outside the loop (and used inside the loop).
So getOutOfScopeIRValue() (getLiveInValue()?) could simply return (getDef()? getUnderlyingValue() : nullptr), or its users could do so themselves directly?

[fhahn]

Thanks for taking a look!

In D92281#2479130, @Ayal wrote:

This currently requires giving access to VPValue to those classes, so we
can call the protected constructor. But most of them can be removed in
the future, once the remaining single value def recipes inherit from
VPValue.

Such access seems unneeded as commented below; but in general may have been better to handle these remaining recipes first and avoid multiple befriendings, albeit temporary.

Yes, originally this was not checking for VPDef, because when I posted the patch initially I did not want to block it on completing the transition. But that is done now, so just checking Def is much simpler.

I updated the code to just check VPDef (no need to befriend classes) and removed the function to getLiveInIRValue.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanValue.h

64

I did that and updated the function name. I think it is still worth to have it as a separate function, because there will be a few users in the following patches. Also, I think granting access to live-in IR values makes sense in general, even once we push back/reduce uses of getUnderlyingValue elsewhere.

[fhahn]

Add assert to make sure getLiveInIRValue is only called for 'live-in' VPValues.

[Ayal]

(how) are the VPPredInstPHIRecipe changes related to the introduction of getLiveInIRValue()?

Patch is (NFC).

[fhahn]

In D92281#2480441, @Ayal wrote:

(how) are the VPPredInstPHIRecipe changes related to the introduction of getLiveInIRValue()?

Patch is (NFC).

They are not directly related, i didn’t realise they were already there before undoing the changes that were part of the original patch.

I can split them off and land them separately

[Ayal]

ok, please land independent changes separately, thanks!

[fhahn]

Thanks Ayal! I landed the VPPredInstPHIRecipe changes separately in f73c09caa2a8.

llvm/lib/Transforms/Vectorize/VPlan.h

 /// This recipe works in concert with VPBranchOnMaskRecipe.
 class VPPredInstPHIRecipe : public VPRecipeBase, public VPUser {
 
 public:
   /// Construct a VPPredInstPHIRecipe given \p PredInst whose value needs a phi
   /// nodes after merging back from a Branch-on-Mask.
   VPPredInstPHIRecipe(VPValue *PredV)
       : VPRecipeBase(VPPredInstPHISC), VPUser(PredV) {
-    new VPValue(VPValue::VPValueSC, PredV->getUnderlyingValue(), this);
+    new VPValue(PredV->getUnderlyingValue(), this);
   }
   ~VPPredInstPHIRecipe() override = default;
 
   /// Method to support type inquiry through isa, cast, and dyn_cast.
   static inline bool classof(const VPDef *D) {
     return D->getVPDefID() == VPRecipeBase::VPPredInstPHISC;
   }
 

llvm/lib/Transforms/Vectorize/VPlanValue.h

 
 // Forward declarations.
 class raw_ostream;
 class Value;
 class VPDef;
 class VPSlotTracker;
 class VPUser;
 class VPRecipeBase;
-class VPPredInstPHIRecipe;
 class VPWidenMemoryInstructionRecipe;
 
 // This is the base class of the VPlan Def/Use graph, used for modeling the data
 // flow into, within and out of the VPlan. VPValues can stand for live-ins
 // coming from the input IR, instructions which VPlan will generate if executed
 // and live-outs which the VPlan will need to fix accordingly.
 class VPValue {
   friend class VPBuilder;
   friend class VPDef;
   friend class VPInstruction;
   friend struct VPlanTransforms;
   friend class VPBasicBlock;
   friend class VPInterleavedAccessInfo;
   friend class VPSlotTracker;
   friend class VPRecipeBase;
-  friend class VPPredInstPHIRecipe;
   friend class VPWidenMemoryInstructionRecipe;
 
   const unsigned char SubclassID; ///< Subclass identifier (for isa/dyn_cast).
 
   SmallVector<VPUser *, 1> Users;
 
 protected:
   // Hold the underlying Value, if any, attached to this VPValue.
   Value *UnderlyingVal;
 
   /// Pointer to the VPDef that defines this VPValue. If it is nullptr, the
   /// VPValue is not defined by any recipe modeled in VPlan.
   VPDef *Def;
 
   VPValue(const unsigned char SC, Value *UV = nullptr, VPDef *Def = nullptr);
 
   // 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
bool hasMoreThanOneUniqueUser() {
     while (Current != user_end() && *user_begin() == *Current)
       Current++;
     return Current != user_end();
   }
 
   void replaceAllUsesWith(VPValue *New);
 
   VPDef *getDef() { return Def; }
+
+  /// Returns the underlying IR value, if this VPValue is defined outside the
+  /// scope of VPlan. Returns nullptr if the VPValue is defined by a VPDef
+  /// inside a VPlan.
+  Value *getLiveInIRValue() {
+    return !getDef() ? getUnderlyingValue() : nullptr;
+  }
 };
 
 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

 
 /// This class can be used to assign consecutive numbers to all VPValues in a
 /// VPlan and allows querying the numbering for printing, similar to the
 /// ModuleSlotTracker for IR values.
 class VPSlotTracker {
   DenseMap<const VPValue *, unsigned> Slots;
   unsigned NextSlot = 0;
 
   void assignSlots(const VPBlockBase *VPBB);
   void assignSlots(const VPRegionBlock *Region);
   void assignSlots(const VPBasicBlock *VPBB);
   void assignSlot(const VPValue *V);
 
   void assignSlots(const VPlan &Plan);
 
 public:
   VPSlotTracker(const VPlan *Plan) {