[RemoveDIs][NFC] Introduce DbgRecord base class [1/3]

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

@@ -109,6 +109,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

@@ -47,10 +47,12 @@
 #ifndef LLVM_IR_DEBUGPROGRAMINSTRUCTION_H
 #define LLVM_IR_DEBUGPROGRAMINSTRUCTION_H
 
-#include "llvm/ADT/ilist_node.h"
 #include "llvm/ADT/ilist.h"
+#include "llvm/ADT/ilist_node.h"
 #include "llvm/ADT/iterator.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
+#include "llvm/Support/Casting.h"
 
 namespace llvm {
 
@@ -63,38 +65,92 @@ 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()
+///   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;
+  Kind RecordKind; ///< Subclass discriminator.
 
 public:
-  void deleteInstr();
+  DbgRecord(Kind RecordKind, DebugLoc DL)
+      : DbgLoc(DL), RecordKind(RecordKind) {}
+
+  /// Methods requiring subclass implementations.
+  ///@{
+  void deleteRecord();
+  DbgRecord *clone() const;
+  void print(raw_ostream &O, bool IsForDebug = false) const;
+  void print(raw_ostream &O, ModuleSlotTracker &MST, bool IsForDebug) 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 BasicBlock *getParent() const;
   BasicBlock *getParent();
-  void dump() const;
+
   void removeFromParent();
   void eraseFromParent();
 
-  using self_iterator = simple_ilist<DPValue>::iterator;
-  using const_self_iterator = simple_ilist<DPValue>::const_iterator;
+  DebugLoc getDebugLoc() const { return DbgLoc; }
+  void setDebugLoc(DebugLoc Loc) { DbgLoc = std::move(Loc); }
+
+  void dump() const;
 
-  enum class LocationType {
+  using self_iterator = simple_ilist<DbgRecord>::iterator;
+  using const_self_iterator = simple_ilist<DbgRecord>::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;
+
+public:
+  enum class LocationType : uint8_t {
     Declare,
     Value,
 
@@ -104,11 +160,17 @@ 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;
+
+public:
   /// Create a new DPValue representing the intrinsic \p DVI, for example the
   /// assignment represented by a dbg.value.
   DPValue(const DbgVariableIntrinsic *DVI);
@@ -197,9 +259,6 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   bool isAddressOfVariable() const { return Type != LocationType::Value; }
   LocationType getType() const { return Type; }
 
-  DebugLoc getDebugLoc() const { return DbgLoc; }
-  void setDebugLoc(DebugLoc Loc) { DbgLoc = std::move(Loc); }
-
   void setKillLocation();
   bool isKillLocation() const;
 
@@ -230,40 +289,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;
+
   /// Handle changes to the location of the Value(s) that we refer to happening
   /// "under our feet".
   void handleChangedLocation(Metadata *NewLocation);
 
-  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;
-
   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(...
@@ -283,12 +339,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;
@@ -308,34 +363,34 @@ 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<DbgRecord>::iterator> getDbgValueRange();
   /// 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);
   /// 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 *DPE);
 
   /// 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
@@ -345,8 +400,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());
   }
 };
 

llvm/include/llvm/IR/Instruction.h

@@ -35,6 +35,7 @@ class MDNode;
 class Module;
 struct AAMDNodes;
 class DPMarker;
+class DbgRecord;
 
 template <> struct ilist_alloc_traits<Instruction> {
   static inline void deleteNode(Instruction *V);
@@ -70,18 +71,18 @@ class Instruction : public User,
   /// \p InsertAtHead Whether the cloned DPValues should be placed at the end
   ///    or the beginning of existing DPValues attached to this.
   /// \returns A range over the newly cloned DPValues.
-  iterator_range<simple_ilist<DPValue>::iterator> cloneDebugInfoFrom(
+  iterator_range<simple_ilist<DbgRecord>::iterator> cloneDebugInfoFrom(
       const Instruction *From,
-      std::optional<simple_ilist<DPValue>::iterator> FromHere = std::nullopt,
+      std::optional<simple_ilist<DbgRecord>::iterator> FromHere = std::nullopt,
       bool InsertAtHead = false);
 
   /// Return a range over the DPValues attached to this instruction.
-  iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange() const;
+  iterator_range<simple_ilist<DbgRecord>::iterator> getDbgValueRange() const;
 
   /// Return an iterator to the position of the "Next" DPValue after this
   /// instruction, or std::nullopt. This is the position to pass to
   /// BasicBlock::reinsertInstInDPValues when re-inserting an instruction.
-  std::optional<simple_ilist<DPValue>::iterator> getDbgReinsertionPosition();
+  std::optional<simple_ilist<DbgRecord>::iterator> getDbgReinsertionPosition();
 
   /// Returns true if any DPValues are attached to this instruction.
   bool hasDbgValues() const;
@@ -90,7 +91,7 @@ class Instruction : public User,
   void dropDbgValues();
 
   /// Erase a single DPValue \p I that is attached to this instruction.
-  void dropOneDbgValue(DPValue *I);
+  void dropOneDbgValue(DbgRecord *I);
 
   /// Handle the debug-info implications of this instruction being removed. Any
   /// attached DPValues need to "fall" down onto the next instruction.

llvm/include/llvm/IR/Metadata.h

@@ -44,6 +44,7 @@ class Module;
 class ModuleSlotTracker;
 class raw_ostream;
 class DPValue;
+class DbgRecord;
 template <typename T> class StringMapEntry;
 template <typename ValueTy> class StringMapEntryStorage;
 class Type;