[llvm] [RemoveDIs][DebugInfo] Add DPVAssign variant of DPValue (PR #77912)

via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 12 04:24:45 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Stephen Tozer (SLTozer)

<details>
<summary>Changes</summary>

This implements the DbgAssignIntrinsic class as a variant of DPValues - unfortunately this involves increasing the size of the `DebugValueUser` storage by 3x, but this is necessary to enable assigns to be represented, and can be offset in a future patch by splitting DPValue into subclasses such that each variant can store only the fields it needs. This patch does not actually create DPVAssigns in any case; future patches will handle this variant in all cases where generic DPValue handling does not. This patch also does not implement tracking support for DIAssignIDs, which is necessary to find DPVAssigns that reference a given DIAssignID; that is added in a subsequent patch.

---

Patch is 21.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77912.diff


5 Files Affected:

- (modified) llvm/include/llvm/IR/DebugProgramInstruction.h (+52-7) 
- (modified) llvm/include/llvm/IR/Metadata.h (+42-28) 
- (modified) llvm/lib/IR/AsmWriter.cpp (+27-1) 
- (modified) llvm/lib/IR/DebugProgramInstruction.cpp (+105-16) 
- (modified) llvm/lib/IR/Metadata.cpp (+36-15) 


``````````diff
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index 8230070343e0c1..66453c1a2c1a5e 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -59,6 +59,7 @@ class BasicBlock;
 class MDNode;
 class Module;
 class DbgVariableIntrinsic;
+class DIAssignID;
 class DPMarker;
 class DPValue;
 class raw_ostream;
@@ -81,6 +82,7 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   DILocalVariable *Variable;
   DIExpression *Expression;
   DebugLoc DbgLoc;
+  DIExpression *AddressExpression;
 
 public:
   void deleteInstr();
@@ -97,6 +99,7 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   enum class LocationType {
     Declare,
     Value,
+    Assign,
 
     End, ///< Marks the end of the concrete types.
     Any, ///< To indicate all LocationTypes in searches.
@@ -117,6 +120,22 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
   /// assigning \p Location to the DV / Expr / DI variable.
   DPValue(Metadata *Location, DILocalVariable *DV, DIExpression *Expr,
           const DILocation *DI, LocationType Type = LocationType::Value);
+  DPValue(Metadata *Value, DILocalVariable *Variable, DIExpression *Expression,
+          DIAssignID *AssignID, Metadata *Address,
+          DIExpression *AddressExpression, const DILocation *DI);
+
+  static DPValue *createDPVAssign(Metadata *Value, DILocalVariable *Variable,
+                                  DIExpression *Expression,
+                                  DIAssignID *AssignID, Metadata *Address,
+                                  DIExpression *AddressExpression,
+                                  const DILocation *DI,
+                                  Instruction *InsertBefore = nullptr);
+  static DPValue *createLinkedDPVAssign(Instruction *LinkedInstr, Value *Val,
+                                        DILocalVariable *Variable,
+                                        DIExpression *Expression,
+                                        Value *Address,
+                                        DIExpression *AddressExpression,
+                                        const DILocation *DI);
 
   /// Iterator for ValueAsMetadata that internally uses direct pointer iteration
   /// over either a ValueAsMetadata* or a ValueAsMetadata**, dereferencing to the
@@ -194,7 +213,7 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
 
   /// Does this describe the address of a local variable. True for dbg.addr
   /// and dbg.declare, but not dbg.value, which describes its value.
-  bool isAddressOfVariable() const { return Type != LocationType::Value; }
+  bool isAddressOfVariable() const { return Type == LocationType::Declare; }
   LocationType getType() const { return Type; }
 
   DebugLoc getDebugLoc() const { return DbgLoc; }
@@ -207,7 +226,11 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
 
   DIExpression *getExpression() const { return Expression; }
 
-  Metadata *getRawLocation() const { return DebugValue; }
+  /// Returns the metadata operand for the first location description. i.e.,
+  /// dbg intrinsic dbg.value,declare operand and dbg.assign 1st location
+  /// operand (the "value componenet"). Note the operand (singular) may be
+  /// a DIArgList which is a list of values.
+  Metadata *getRawLocation() const { return DebugValues[0]; }
 
   /// Use of this should generally be avoided; instead,
   /// replaceVariableLocationOp and addVariableLocationOps should be used where
@@ -217,23 +240,45 @@ class DPValue : public ilist_node<DPValue>, private DebugValueUser {
         (isa<ValueAsMetadata>(NewLocation) || isa<DIArgList>(NewLocation) ||
          isa<MDNode>(NewLocation)) &&
         "Location for a DPValue must be either ValueAsMetadata or DIArgList");
-    resetDebugValue(NewLocation);
+    resetDebugValue(0, NewLocation);
   }
 
   /// Get the size (in bits) of the variable, or fragment of the variable that
   /// is described.
   std::optional<uint64_t> getFragmentSizeInBits() const;
 
+  /// @name DbgAssign Methods
+  /// @{
+  bool isDbgAssign() const { return getType() == LocationType::Assign; }
+
+  Value *getAddress() const;
+  Metadata *getRawAddress() const {
+    return isDbgAssign() ? DebugValues[1] : DebugValues[0];
+  }
+  Metadata *getRawAssignID() const { return DebugValues[2]; }
+  DIAssignID *getAssignID() const;
+  DIExpression *getAddressExpression() const { return AddressExpression; }
+  void setAddressExpression(DIExpression *NewExpr) {
+    AddressExpression = NewExpr;
+  }
+  void setAssignId(DIAssignID *New);
+  void setAddress(Value *V) { resetDebugValue(1, ValueAsMetadata::get(V)); }
+  /// Kill the address component.
+  void setKillAddress();
+  /// Check whether this kills the address component. This doesn't take into
+  /// account the position of the intrinsic, therefore a returned value of false
+  /// does not guarentee the address is a valid location for the variable at the
+  /// intrinsic's position in IR.
+  bool isKillAddress() const;
+
+  /// @}
+public:
   DPValue *clone() const;
   /// Convert this DPValue back into a dbg.value intrinsic.
   /// \p InsertBefore Optional position to insert this intrinsic.
   /// \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; }
diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h
index 4498423c4c460d..365820b95b298d 100644
--- a/llvm/include/llvm/IR/Metadata.h
+++ b/llvm/include/llvm/IR/Metadata.h
@@ -211,31 +211,39 @@ class MetadataAsValue : public Value {
 /// lookup and callback handling.
 class DebugValueUser {
 protected:
-  Metadata *DebugValue;
+  // Capacity to store 3 debug values.
+  // TODO: Not all DebugValueUser instances need all 3 elements, if we
+  // restructure the DPValue class then we can template parameterize this array
+  // size.
+  std::array<Metadata *, 3> DebugValues;
+
+  ArrayRef<Metadata *> getDebugValues() const { return DebugValues; }
 
 public:
   DPValue *getUser();
   const DPValue *getUser() const;
-  void handleChangedValue(Metadata *NewDebugValue);
+  void handleChangedValue(void *Old, Metadata *NewDebugValue);
   DebugValueUser() = default;
-  explicit DebugValueUser(Metadata *DebugValue) : DebugValue(DebugValue) {
-    trackDebugValue();
+  explicit DebugValueUser(std::array<Metadata *, 3> DebugValues)
+      : DebugValues(DebugValues) {
+    trackDebugValues();
   }
-
-  DebugValueUser(DebugValueUser &&X) : DebugValue(X.DebugValue) {
-    retrackDebugValue(X);
+  DebugValueUser(DebugValueUser &&X) {
+    DebugValues = X.DebugValues;
+    retrackDebugValues(X);
   }
-  DebugValueUser(const DebugValueUser &X) : DebugValue(X.DebugValue) {
-    trackDebugValue();
+  DebugValueUser(const DebugValueUser &X) {
+    DebugValues = X.DebugValues;
+    trackDebugValues();
   }
 
   DebugValueUser &operator=(DebugValueUser &&X) {
     if (&X == this)
       return *this;
 
-    untrackDebugValue();
-    DebugValue = X.DebugValue;
-    retrackDebugValue(X);
+    untrackDebugValues();
+    DebugValues = X.DebugValues;
+    retrackDebugValues(X);
     return *this;
   }
 
@@ -243,35 +251,41 @@ class DebugValueUser {
     if (&X == this)
       return *this;
 
-    untrackDebugValue();
-    DebugValue = X.DebugValue;
-    trackDebugValue();
+    untrackDebugValues();
+    DebugValues = X.DebugValues;
+    trackDebugValues();
     return *this;
   }
 
-  ~DebugValueUser() { untrackDebugValue(); }
+  ~DebugValueUser() { untrackDebugValues(); }
 
-  void resetDebugValue() {
-    untrackDebugValue();
-    DebugValue = nullptr;
+  void resetDebugValues() {
+    untrackDebugValues();
+    DebugValues.fill(nullptr);
   }
-  void resetDebugValue(Metadata *DebugValue) {
-    untrackDebugValue();
-    this->DebugValue = DebugValue;
-    trackDebugValue();
+
+  void resetDebugValue(size_t Idx, Metadata *DebugValue) {
+    assert(Idx < 3 && "Invalid debug value index.");
+    untrackDebugValue(Idx);
+    DebugValues[Idx] = DebugValue;
+    trackDebugValue(Idx);
   }
 
   bool operator==(const DebugValueUser &X) const {
-    return DebugValue == X.DebugValue;
+    return DebugValues == X.DebugValues;
   }
   bool operator!=(const DebugValueUser &X) const {
-    return DebugValue != X.DebugValue;
+    return DebugValues != X.DebugValues;
   }
 
 private:
-  void trackDebugValue();
-  void untrackDebugValue();
-  void retrackDebugValue(DebugValueUser &X);
+  void trackDebugValue(size_t Idx);
+  void trackDebugValues();
+
+  void untrackDebugValue(size_t Idx);
+  void untrackDebugValues();
+
+  void retrackDebugValues(DebugValueUser &X);
 };
 
 /// API for tracking metadata references through RAUW and deletion.
diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp
index 278cdfce411050..3c15784a0ed5eb 100644
--- a/llvm/lib/IR/AsmWriter.cpp
+++ b/llvm/lib/IR/AsmWriter.cpp
@@ -1140,6 +1140,9 @@ void SlotTracker::processFunctionMetadata(const Function &F) {
 void SlotTracker::processDPValueMetadata(const DPValue &DPV) {
   CreateMetadataSlot(DPV.getVariable());
   CreateMetadataSlot(DPV.getDebugLoc());
+  if (DPV.isDbgAssign()) {
+    CreateMetadataSlot(DPV.getAssignID());
+  }
 }
 
 void SlotTracker::processInstructionMetadata(const Instruction &I) {
@@ -4571,7 +4574,22 @@ void AssemblyWriter::printDPMarker(const DPMarker &Marker) {
 void AssemblyWriter::printDPValue(const DPValue &Value) {
   // There's no formal representation of a DPValue -- print purely as a
   // debugging aid.
-  Out << "  DPValue { ";
+  Out << "  DPValue ";
+
+  switch (Value.getType()) {
+  case DPValue::LocationType::Value:
+    Out << "value";
+    break;
+  case DPValue::LocationType::Declare:
+    Out << "declare";
+    break;
+  case DPValue::LocationType::Assign:
+    Out << "assign";
+    break;
+  default:
+    llvm_unreachable("Tried to print a DPValue with an invalid LocationType!");
+  }
+  Out << " { ";
   auto WriterCtx = getContext();
   WriteAsOperandInternal(Out, Value.getRawLocation(), WriterCtx, true);
   Out << ", ";
@@ -4579,6 +4597,14 @@ void AssemblyWriter::printDPValue(const DPValue &Value) {
   Out << ", ";
   WriteAsOperandInternal(Out, Value.getExpression(), WriterCtx, true);
   Out << ", ";
+  if (Value.isDbgAssign()) {
+    WriteAsOperandInternal(Out, Value.getAssignID(), WriterCtx, true);
+    Out << ", ";
+    WriteAsOperandInternal(Out, Value.getRawAddress(), WriterCtx, true);
+    Out << ", ";
+    WriteAsOperandInternal(Out, Value.getAddressExpression(), WriterCtx, true);
+    Out << ", ";
+  }
   WriteAsOperandInternal(Out, Value.getDebugLoc().get(), WriterCtx, true);
   Out << " marker @" << Value.getMarker();
   Out << " }";
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 7b709a2de0335f..1c1c965b1c09d7 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -14,8 +14,9 @@
 namespace llvm {
 
 DPValue::DPValue(const DbgVariableIntrinsic *DVI)
-    : DebugValueUser(DVI->getRawLocation()), Variable(DVI->getVariable()),
-      Expression(DVI->getExpression()), DbgLoc(DVI->getDebugLoc()) {
+    : DebugValueUser({DVI->getRawLocation(), nullptr, nullptr}),
+      Variable(DVI->getVariable()), Expression(DVI->getExpression()),
+      DbgLoc(DVI->getDebugLoc()), AddressExpression(nullptr) {
   switch (DVI->getIntrinsicID()) {
   case Intrinsic::dbg_value:
     Type = LocationType::Value;
@@ -23,6 +24,15 @@ DPValue::DPValue(const DbgVariableIntrinsic *DVI)
   case Intrinsic::dbg_declare:
     Type = LocationType::Declare;
     break;
+  case Intrinsic::dbg_assign: {
+    Type = LocationType::Assign;
+    const DbgAssignIntrinsic *Assign =
+        static_cast<const DbgAssignIntrinsic *>(DVI);
+    resetDebugValue(1, Assign->getRawAddress());
+    AddressExpression = Assign->getAddressExpression();
+    setAssignId(Assign->getAssignID());
+    break;
+  }
   default:
     llvm_unreachable(
         "Trying to create a DPValue with an invalid intrinsic type!");
@@ -30,17 +40,54 @@ DPValue::DPValue(const DbgVariableIntrinsic *DVI)
 }
 
 DPValue::DPValue(const DPValue &DPV)
-    : DebugValueUser(DPV.getRawLocation()),
-      Variable(DPV.getVariable()), Expression(DPV.getExpression()),
-      DbgLoc(DPV.getDebugLoc()), Type(DPV.getType()) {}
+    : DebugValueUser(DPV.DebugValues), Variable(DPV.getVariable()),
+      Expression(DPV.getExpression()), DbgLoc(DPV.getDebugLoc()),
+      AddressExpression(DPV.AddressExpression), Type(DPV.getType()) {}
 
 DPValue::DPValue(Metadata *Location, DILocalVariable *DV, DIExpression *Expr,
                  const DILocation *DI, LocationType Type)
-    : DebugValueUser(Location), Variable(DV), Expression(Expr), DbgLoc(DI),
-      Type(Type) {}
+    : DebugValueUser({Location, nullptr, nullptr}), Variable(DV),
+      Expression(Expr), DbgLoc(DI), Type(Type) {}
+
+DPValue::DPValue(Metadata *Value, DILocalVariable *Variable,
+                 DIExpression *Expression, DIAssignID *AssignID,
+                 Metadata *Address, DIExpression *AddressExpression,
+                 const DILocation *DI)
+    : DebugValueUser({Value, Address, AssignID}), Variable(Variable),
+      Expression(Expression), DbgLoc(DI), AddressExpression(AddressExpression),
+      Type(LocationType::Assign) {}
 
 void DPValue::deleteInstr() { delete this; }
 
+DPValue *DPValue::createDPVAssign(Metadata *Value, DILocalVariable *Variable,
+                                  DIExpression *Expression,
+                                  DIAssignID *AssignID, Metadata *Address,
+                                  DIExpression *AddressExpression,
+                                  const DILocation *DI,
+                                  Instruction *InsertBefore) {
+  auto *NewDPVAssign = new DPValue(Value, Variable, Expression, AssignID,
+                                   Address, AddressExpression, DI);
+  if (InsertBefore) {
+    InsertBefore->getParent()->insertDPValueBefore(NewDPVAssign,
+                                                   InsertBefore->getIterator());
+  }
+  return NewDPVAssign;
+}
+DPValue *DPValue::createLinkedDPVAssign(Instruction *LinkedInstr, Value *Val,
+                                        DILocalVariable *Variable,
+                                        DIExpression *Expression,
+                                        Value *Address,
+                                        DIExpression *AddressExpression,
+                                        const DILocation *DI) {
+  auto *Link = LinkedInstr->getMetadata(LLVMContext::MD_DIAssignID);
+  assert(Link && "Linked instruction must have DIAssign metadata attached");
+  auto *NewDPVAssign = new DPValue(
+      ValueAsMetadata::get(Val), Variable, Expression, cast<DIAssignID>(Link),
+      ValueAsMetadata::get(Address), AddressExpression, DI);
+  LinkedInstr->getParent()->insertDPValueAfter(NewDPVAssign, LinkedInstr);
+  return NewDPVAssign;
+}
+
 iterator_range<DPValue::location_op_iterator> DPValue::location_ops() const {
   auto *MD = getRawLocation();
   // If a Value has been deleted, the "location" for this DPValue will be
@@ -96,10 +143,15 @@ static ValueAsMetadata *getAsMetadata(Value *V) {
 void DPValue::replaceVariableLocationOp(Value *OldValue, Value *NewValue,
                                         bool AllowEmpty) {
   assert(NewValue && "Values must be non-null");
+
+  bool DbgAssignAddrReplaced = isDbgAssign() && OldValue == getAddress();
+  if (DbgAssignAddrReplaced)
+    setAddress(NewValue);
+
   auto Locations = location_ops();
   auto OldIt = find(Locations, OldValue);
   if (OldIt == Locations.end()) {
-    if (AllowEmpty)
+    if (AllowEmpty || DbgAssignAddrReplaced)
       return;
     llvm_unreachable("OldValue must be a current location");
   }
@@ -190,9 +242,6 @@ DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
          "Cannot clone from BasicBlock that is not part of a Module or "
          "DICompileUnit!");
   LLVMContext &Context = getDebugLoc()->getContext();
-  Value *Args[] = {MetadataAsValue::get(Context, getRawLocation()),
-                   MetadataAsValue::get(Context, getVariable()),
-                   MetadataAsValue::get(Context, getExpression())};
   Function *IntrinsicFn;
 
   // Work out what sort of intrinsic we're going to produce.
@@ -203,16 +252,34 @@ DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
   case DPValue::LocationType::Value:
     IntrinsicFn = Intrinsic::getDeclaration(M, Intrinsic::dbg_value);
     break;
+  case DPValue::LocationType::Assign:
+    IntrinsicFn = Intrinsic::getDeclaration(M, Intrinsic::dbg_assign);
+    break;
   case DPValue::LocationType::End:
   case DPValue::LocationType::Any:
     llvm_unreachable("Invalid LocationType");
-    break;
   }
 
   // Create the intrinsic from this DPValue's information, optionally insert
   // into the target location.
-  DbgVariableIntrinsic *DVI = cast<DbgVariableIntrinsic>(
-      CallInst::Create(IntrinsicFn->getFunctionType(), IntrinsicFn, Args));
+  DbgVariableIntrinsic *DVI;
+  if (isDbgAssign()) {
+    Value *AssignArgs[] = {
+        MetadataAsValue::get(Context, getRawLocation()),
+        MetadataAsValue::get(Context, getVariable()),
+        MetadataAsValue::get(Context, getExpression()),
+        MetadataAsValue::get(Context, getAssignID()),
+        MetadataAsValue::get(Context, getRawAddress()),
+        MetadataAsValue::get(Context, getAddressExpression())};
+    DVI = cast<DbgVariableIntrinsic>(CallInst::Create(
+        IntrinsicFn->getFunctionType(), IntrinsicFn, AssignArgs));
+  } else {
+    Value *Args[] = {MetadataAsValue::get(Context, getRawLocation()),
+                     MetadataAsValue::get(Context, getVariable()),
+                     MetadataAsValue::get(Context, getExpression())};
+    DVI = cast<DbgVariableIntrinsic>(
+        CallInst::Create(IntrinsicFn->getFunctionType(), IntrinsicFn, Args));
+  }
   DVI->setTailCall();
   DVI->setDebugLoc(getDebugLoc());
   if (InsertBefore)
@@ -221,8 +288,30 @@ DPValue::createDebugIntrinsic(Module *M, Instruction *InsertBefore) const {
   return DVI;
 }
 
-void DPValue::handleChangedLocation(Metadata *NewLocation) {
-  resetDebugValue(NewLocation);
+Value *DPValue::getAddress() const {
+  auto *MD = getRawAddress();
+  if (auto *V = dyn_cast<ValueAsMetadata>(MD))
+    return V->getValue();
+
+  // When the value goes to null, it gets replaced by an empty MDNode.
+  assert(!cast<MDNode>(MD)->getNumOperands() && "Expected an empty MDNode");
+  return nullptr;
+}
+
+DIAssignID *DPValue::getAssignID() const {
+  return cast<DIAssignID>(DebugValues[2]);
+}
+
+void DPValue::setAssignId(DIAssignID *New) { resetDebugValue(2, New); }
+
+void DPValue::setKillAddress() {
+  resetDebugValue(
+      1, ValueAsMetadata::get(UndefValue::get(getAddress()->getType())));
+}
+
+bool DPValue::isKillAddress() const {
+  Value *Addr = getAddress();
+  return !Addr || isa<UndefValue>(Addr);
 }
 
 const BasicBlock *DPValue::getParent() const {
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index 515893d079b8cb..da5d4940a6ab9b 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -152,26 +152,47 @@ DPValue *DebugValueUser::getUser() { return static_cast<DPValue *>(this); }
 const DPValue *DebugValueUser::getUser() const {
   return static_cast<const DPValue *>(this);
 }
-void DebugValueUser::handleChangedValue(Metadata *NewMD) {
-  getUser()->handleChangedLocation(NewMD);
+
+void DebugValueUser::handleChangedValue(void *Old, Metadata *New) {
+  // NOTE: We could inform the "owner" that a value has changed through
+  // getOwner, if needed.
+  auto OldMD = static_cast<Metadata **>(Old);
+  ptrdiff_t Idx = std::distance(DebugValues.begin(), OldMD);
+  resetDebugValue(Idx, New);
 }
 
-void DebugValueUser::trackDebugValue() {
-  if (DebugValue)
-    MetadataTracking::track(&DebugValue, *DebugValue, *this);
+void DebugValueUser::trackDebugValue(size_t Idx) {
+  assert(Idx < 3 && "I...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/77912


More information about the llvm-commits mailing list