[RemoveDIs][NFC] Introduce DbgRecord base class [1/3] (#78252)

Patch 1 of 3 to add llvm.dbg.label support to the RemoveDIs project. The
patch stack adds a new base class

    -> 1. Add DbgRecord base class for DPValue and the not-yet-added
          DPLabel class.
       2. Add the DPLabel class.
       3. Enable dbg.label conversion and add support to passes.

Patches 1 and 2 are NFC.

In the near future we also will rename DPValue to DbgVariableRecord and
DPLabel to DbgLabelRecord, at which point we'll overhaul the function
names too. The name DPLabel keeps things consistent for now.

Author: OCHyams

llvm/include/llvm/IR/BasicBlock.h

@@ -130,10 +130,10 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   DPMarker *getNextMarker(Instruction *I);
 
   /// Insert a DPValue into a block at the position given by \p I.
-  void insertDPValueAfter(DPValue *DPV, Instruction *I);
+  void insertDPValueAfter(DbgRecord *DPV, Instruction *I);
 
   /// Insert a DPValue into a block at the position given by \p Here.
-  void insertDPValueBefore(DPValue *DPV, InstListType::iterator Here);
+  void insertDPValueBefore(DbgRecord *DPV, InstListType::iterator Here);
 
   /// Eject any debug-info trailing at the end of a block. DPValues can
   /// transiently be located "off the end" of a block if the blocks terminator
@@ -147,7 +147,7 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   /// occur: inserting into the middle of a sequence of dbg.value intrinsics
   /// does not have an equivalent with DPValues.
   void reinsertInstInDPValues(Instruction *I,
-                              std::optional<DPValue::self_iterator> Pos);
+                              std::optional<DbgRecord::self_iterator> Pos);
 
 private:
   void setParent(Function *parent);
@@ -194,8 +194,9 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
   friend void Instruction::moveBeforeImpl(BasicBlock &BB,
                                           InstListType::iterator I,
                                           bool Preserve);
-  friend iterator_range<DPValue::self_iterator> Instruction::cloneDebugInfoFrom(
-      const Instruction *From, std::optional<DPValue::self_iterator> FromHere,
+  friend iterator_range<DbgRecord::self_iterator>
+  Instruction::cloneDebugInfoFrom(
+      const Instruction *From, std::optional<DbgRecord::self_iterator> FromHere,
       bool InsertAtHead);
 
   /// Creates a new BasicBlock.

llvm/include/llvm/IR/DebugInfo.h

@@ -111,6 +111,8 @@ class DebugInfoFinder {
   void processLocation(const Module &M, const DILocation *Loc);
   // Process a DPValue, much like a DbgVariableIntrinsic.
   void processDPValue(const Module &M, const DPValue &DPV);
+  /// Dispatch to DbgRecord subclasses handlers.
+  void processDbgRecord(const Module &M, const DbgRecord &DPE);
 
   /// Process subprogram.
   void processSubprogram(DISubprogram *SP);

llvm/include/llvm/IR/DebugProgramInstruction.h

@@ -53,6 +53,7 @@
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/SymbolTableListTraits.h"
+#include "llvm/Support/Casting.h"
 
 namespace llvm {
 
@@ -66,43 +67,105 @@ class DPMarker;
 class DPValue;
 class raw_ostream;
 
-/// Record of a variable value-assignment, aka a non instruction representation
-/// of the dbg.value intrinsic. Features various methods copied across from the
-/// Instruction class to aid ease-of-use. DPValue objects should always be
-/// linked into a DPMarker's StoredDPValues list. The marker connects a DPValue
-/// back to it's position in the BasicBlock.
+/// Base class for non-instruction debug metadata records that have positions
+/// within IR. Features various methods copied across from the Instruction
+/// class to aid ease-of-use. DbgRecords should always be linked into a
+/// DPMarker's StoredDPValues list. The marker connects a DbgRecord back to
+/// it's position in the BasicBlock.
 ///
-/// This class inherits from DebugValueUser to allow LLVM's metadata facilities
-/// to update our references to metadata beneath our feet.
-class DPValue : public ilist_node<DPValue>, private DebugValueUser {
-  friend class DebugValueUser;
-
-  // NB: there is no explicit "Value" field in this class, it's effectively the
-  // DebugValueUser superclass instead. The referred to Value can either be a
-  // ValueAsMetadata or a DIArgList.
+/// We need a discriminator for dyn/isa casts. In order to avoid paying for a
+/// vtable for "virtual" functions too, subclasses must add a new discriminator
+/// value (RecordKind) and cases to a few functions in the base class:
+///   deleteRecord
+///   clone
+///   isIdenticalToWhenDefined
+///   isEquivalentTo
+///   both print methods
+class DbgRecord : public ilist_node<DbgRecord> {
+public:
+  /// Marker that this DbgRecord is linked into.
+  DPMarker *Marker = nullptr;
+  /// Subclass discriminator.
+  enum Kind : uint8_t { ValueKind };
 
-  DILocalVariable *Variable;
-  DIExpression *Expression;
+protected:
   DebugLoc DbgLoc;
-  DIExpression *AddressExpression;
+  Kind RecordKind; ///< Subclass discriminator.
 
 public:
-  void deleteInstr();
+  DbgRecord(Kind RecordKind, DebugLoc DL)
+      : DbgLoc(DL), RecordKind(RecordKind) {}
+
+  /// Methods that dispatch to subclass implementations. These need to be
+  /// manually updated when a new subclass is added.
+  ///@{
+  void deleteRecord();
+  DbgRecord *clone() const;
+  void print(raw_ostream &O, bool IsForDebug = false) const;
+  void print(raw_ostream &O, ModuleSlotTracker &MST, bool IsForDebug) const;
+  bool isIdenticalToWhenDefined(const DbgRecord &R) const;
+  bool isEquivalentTo(const DbgRecord &R) const;
+  ///@}
+
+  Kind getRecordKind() const { return RecordKind; }
+
+  void setMarker(DPMarker *M) { Marker = M; }
+
+  DPMarker *getMarker() { return Marker; }
+  const DPMarker *getMarker() const { return Marker; }
+
+  BasicBlock *getBlock();
+  const BasicBlock *getBlock() const;
+
+  Function *getFunction();
+  const Function *getFunction() const;
+
+  Module *getModule();
+  const Module *getModule() const;
+
+  LLVMContext &getContext();
+  const LLVMContext &getContext() const;
 
   const Instruction *getInstruction() const;
   const BasicBlock *getParent() const;
   BasicBlock *getParent();
-  void dump() const;
+
   void removeFromParent();
   void eraseFromParent();
 
-  DPValue *getNextNode() { return &*std::next(getIterator()); }
-  DPValue *getPrevNode() { return &*std::prev(getIterator()); }
+  DbgRecord *getNextNode() { return &*std::next(getIterator()); }
+  DbgRecord *getPrevNode() { return &*std::prev(getIterator()); }
+  void insertBefore(DbgRecord *InsertBefore);
+  void insertAfter(DbgRecord *InsertAfter);
+  void moveBefore(DbgRecord *MoveBefore);
+  void moveAfter(DbgRecord *MoveAfter);
+
+  DebugLoc getDebugLoc() const { return DbgLoc; }
+  void setDebugLoc(DebugLoc Loc) { DbgLoc = std::move(Loc); }
+
+  void dump() const;
+
+  using self_iterator = simple_ilist<DbgRecord>::iterator;
+  using const_self_iterator = simple_ilist<DbgRecord>::const_iterator;
 
-  using self_iterator = simple_ilist<DPValue>::iterator;
-  using const_self_iterator = simple_ilist<DPValue>::const_iterator;
+protected:
+  /// Similarly to Value, we avoid paying the cost of a vtable
+  /// by protecting the dtor and having deleteRecord dispatch
+  /// cleanup.
+  /// Use deleteRecord to delete a generic record.
+  ~DbgRecord() = default;
+};
+
+/// Record of a variable value-assignment, aka a non instruction representation
+/// of the dbg.value intrinsic.
+///
+/// This class inherits from DebugValueUser to allow LLVM's metadata facilities
+/// to update our references to metadata beneath our feet.
+class DPValue : public DbgRecord, protected DebugValueUser {
+  friend class DebugValueUser;
 
-  enum class LocationType {
+public:
+  enum class LocationType : uint8_t {
     Declare,
     Value,
     Assign,
@@ -113,11 +176,18 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   /// Classification of the debug-info record that this DPValue represents.
   /// Essentially, "is this a dbg.value or dbg.declare?". dbg.declares are not
   /// currently supported, but it would be trivial to do so.
+  /// FIXME: We could use spare padding bits from DbgRecord for this.
   LocationType Type;
 
-  /// Marker that this DPValue is linked into.
-  DPMarker *Marker = nullptr;
+  // NB: there is no explicit "Value" field in this class, it's effectively the
+  // DebugValueUser superclass instead. The referred to Value can either be a
+  // ValueAsMetadata or a DIArgList.
+
+  DILocalVariable *Variable;
+  DIExpression *Expression;
+  DIExpression *AddressExpression;
 
+public:
   /// Create a new DPValue representing the intrinsic \p DVI, for example the
   /// assignment represented by a dbg.value.
   DPValue(const DbgVariableIntrinsic *DVI);
@@ -235,9 +305,6 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   bool isAddressOfVariable() const { return Type == LocationType::Declare; }
   LocationType getType() const { return Type; }
 
-  DebugLoc getDebugLoc() const { return DbgLoc; }
-  void setDebugLoc(DebugLoc Loc) { DbgLoc = std::move(Loc); }
-
   void setKillLocation();
   bool isKillLocation() const;
 
@@ -270,12 +337,12 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   /// is described.
   std::optional<uint64_t> getFragmentSizeInBits() const;
 
-  bool isEquivalentTo(const DPValue &Other) {
+  bool isEquivalentTo(const DPValue &Other) const {
     return DbgLoc == Other.DbgLoc && isIdenticalToWhenDefined(Other);
   }
   // Matches the definition of the Instruction version, equivalent to above but
   // without checking DbgLoc.
-  bool isIdenticalToWhenDefined(const DPValue &Other) {
+  bool isIdenticalToWhenDefined(const DPValue &Other) const {
     return std::tie(Type, DebugValues, Variable, Expression,
                     AddressExpression) ==
            std::tie(Other.Type, Other.DebugValues, Other.Variable,
@@ -314,43 +381,37 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   /// \returns A new dbg.value intrinsic representiung this DPValue.
   DbgVariableIntrinsic *createDebugIntrinsic(Module *M,
                                              Instruction *InsertBefore) const;
-  void setMarker(DPMarker *M) { Marker = M; }
-
-  DPMarker *getMarker() { return Marker; }
-  const DPMarker *getMarker() const { return Marker; }
-
-  BasicBlock *getBlock();
-  const BasicBlock *getBlock() const;
-
-  Function *getFunction();
-  const Function *getFunction() const;
 
-  Module *getModule();
-  const Module *getModule() const;
-
-  LLVMContext &getContext();
-  const LLVMContext &getContext() const;
-
-  /// Insert this DPValue prior to \p InsertBefore. Must not be called if this
-  /// is already contained in a DPMarker.
-  void insertBefore(DPValue *InsertBefore);
-  void insertAfter(DPValue *InsertAfter);
-  void moveBefore(DPValue *MoveBefore);
-  void moveAfter(DPValue *MoveAfter);
+  /// Handle changes to the location of the Value(s) that we refer to happening
+  /// "under our feet".
+  void handleChangedLocation(Metadata *NewLocation);
 
   void print(raw_ostream &O, bool IsForDebug = false) const;
   void print(raw_ostream &ROS, ModuleSlotTracker &MST, bool IsForDebug) const;
+
+  /// Filter the DbgRecord range to DPValue types only and downcast.
+  static inline auto
+  filter(iterator_range<simple_ilist<DbgRecord>::iterator> R) {
+    return map_range(
+        make_filter_range(R, [](DbgRecord &E) { return isa<DPValue>(E); }),
+        [](DbgRecord &E) { return std::ref(cast<DPValue>(E)); });
+  }
+
+  /// Support type inquiry through isa, cast, and dyn_cast.
+  static bool classof(const DbgRecord *E) {
+    return E->getRecordKind() == ValueKind;
+  }
 };
 
 /// Per-instruction record of debug-info. If an Instruction is the position of
 /// some debugging information, it points at a DPMarker storing that info. Each
 /// marker points back at the instruction that owns it. Various utilities are
-/// provided for manipulating the DPValues contained within this marker.
+/// provided for manipulating the DbgRecords contained within this marker.
 ///
-/// This class has a rough surface area, because it's needed to preserve the one
-/// arefact that we can't yet eliminate from the intrinsic / dbg.value
-/// debug-info design: the order of DPValues/records is significant, and
-/// duplicates can exist. Thus, if one has a run of debug-info records such as:
+/// This class has a rough surface area, because it's needed to preserve the
+/// one arefact that we can't yet eliminate from the intrinsic / dbg.value
+/// debug-info design: the order of records is significant, and duplicates can
+/// exist. Thus, if one has a run of debug-info records such as:
 ///    dbg.value(...
 ///    %foo = barinst
 ///    dbg.value(...
@@ -370,12 +431,11 @@ class DPMarker {
   /// operations that move a marker from one instruction to another.
   Instruction *MarkedInstr = nullptr;
 
-  /// List of DPValues, each recording a single variable assignment, the
-  /// equivalent of a dbg.value intrinsic. There is a one-to-one relationship
-  /// between each dbg.value in a block and each DPValue once the
-  /// representation has been converted, and the ordering of DPValues is
-  /// meaningful in the same was a dbg.values.
-  simple_ilist<DPValue> StoredDPValues;
+  /// List of DbgRecords, the non-instruction equivalent of llvm.dbg.*
+  /// intrinsics. There is a one-to-one relationship between each debug
+  /// intrinsic in a block and each DbgRecord once the representation has been
+  /// converted, and the ordering is meaningful in the same way.
+  simple_ilist<DbgRecord> StoredDPValues;
   bool empty() const { return StoredDPValues.empty(); }
 
   const BasicBlock *getParent() const;
@@ -395,40 +455,40 @@ class DPMarker {
   void print(raw_ostream &ROS, ModuleSlotTracker &MST, bool IsForDebug) const;
 
   /// Produce a range over all the DPValues in this Marker.
-  iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange();
-  iterator_range<simple_ilist<DPValue>::const_iterator>
+  iterator_range<simple_ilist<DbgRecord>::iterator> getDbgValueRange();
+  iterator_range<simple_ilist<DbgRecord>::const_iterator>
   getDbgValueRange() const;
   /// Transfer any DPValues from \p Src into this DPMarker. If \p InsertAtHead
   /// is true, place them before existing DPValues, otherwise afterwards.
   void absorbDebugValues(DPMarker &Src, bool InsertAtHead);
   /// Transfer the DPValues in \p Range from \p Src into this DPMarker. If
   /// \p InsertAtHead is true, place them before existing DPValues, otherwise
   // afterwards.
-  void absorbDebugValues(iterator_range<DPValue::self_iterator> Range,
+  void absorbDebugValues(iterator_range<DbgRecord::self_iterator> Range,
                          DPMarker &Src, bool InsertAtHead);
   /// Insert a DPValue into this DPMarker, at the end of the list. If
   /// \p InsertAtHead is true, at the start.
-  void insertDPValue(DPValue *New, bool InsertAtHead);
+  void insertDPValue(DbgRecord *New, bool InsertAtHead);
   /// Insert a DPValue prior to a DPValue contained within this marker.
-  void insertDPValue(DPValue *New, DPValue *InsertBefore);
+  void insertDPValue(DbgRecord *New, DbgRecord *InsertBefore);
   /// Insert a DPValue after a DPValue contained within this marker.
-  void insertDPValueAfter(DPValue *New, DPValue *InsertAfter);
+  void insertDPValueAfter(DbgRecord *New, DbgRecord *InsertAfter);
   /// Clone all DPMarkers from \p From into this marker. There are numerous
   /// options to customise the source/destination, due to gnarliness, see class
   /// comment.
   /// \p FromHere If non-null, copy from FromHere to the end of From's DPValues
   /// \p InsertAtHead Place the cloned DPValues at the start of StoredDPValues
   /// \returns Range over all the newly cloned DPValues
-  iterator_range<simple_ilist<DPValue>::iterator>
+  iterator_range<simple_ilist<DbgRecord>::iterator>
   cloneDebugInfoFrom(DPMarker *From,
-                     std::optional<simple_ilist<DPValue>::iterator> FromHere,
+                     std::optional<simple_ilist<DbgRecord>::iterator> FromHere,
                      bool InsertAtHead = false);
   /// Erase all DPValues in this DPMarker.
-  void dropDPValues();
-  /// Erase a single DPValue from this marker. In an ideal future, we would
+  void dropDbgValues();
+  /// Erase a single DbgRecord from this marker. In an ideal future, we would
   /// never erase an assignment in this way, but it's the equivalent to
-  /// erasing a dbg.value from a block.
-  void dropOneDPValue(DPValue *DPV);
+  /// erasing a debug intrinsic from a block.
+  void dropOneDbgValue(DbgRecord *DR);
 
   /// We generally act like all llvm Instructions have a range of DPValues
   /// attached to them, but in reality sometimes we don't allocate the DPMarker
@@ -438,8 +498,10 @@ class DPMarker {
   /// DPValue in that range, but they should be using the Official (TM) API for
   /// that.
   static DPMarker EmptyDPMarker;
-  static iterator_range<simple_ilist<DPValue>::iterator> getEmptyDPValueRange(){
-    return make_range(EmptyDPMarker.StoredDPValues.end(), EmptyDPMarker.StoredDPValues.end());
+  static iterator_range<simple_ilist<DbgRecord>::iterator>
+  getEmptyDPValueRange() {
+    return make_range(EmptyDPMarker.StoredDPValues.end(),
+                      EmptyDPMarker.StoredDPValues.end());
   }
 };
 
@@ -457,7 +519,7 @@ inline raw_ostream &operator<<(raw_ostream &OS, const DPValue &Value) {
 /// to be inlined as it's frequently called, but also come after the declaration
 /// of DPMarker. Thus: it's pre-declared by users like Instruction, then an
 /// inlineable body defined here.
-inline iterator_range<simple_ilist<DPValue>::iterator>
+inline iterator_range<simple_ilist<DbgRecord>::iterator>
 getDbgValueRange(DPMarker *DbgMarker) {
   if (!DbgMarker)
     return DPMarker::getEmptyDPValueRange();