[llvm] [RemoveDIs][DebugInfo] Make DIAssignID always replaceable (PR #78300)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 16 07:55:28 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

<details>
<summary>Changes</summary>

This patch is a necessary step to allowing the new non-intrinsic debug info to replace llvm.dbg.assign intrinsics. DIAssignIDs must be able to look up the debug assigns that refer to them, which is currently done by finding the MetadataAsValue for that AssignID and then searching the Instruction users of that MAV. This doesn't work for the new debug info however, which does not store direct Value references, and so we can't make use of MAV's lookup, so we instead need to be able to find direct users of DIAssignID. The only way to lookup users of metadata by default is when that metadata is tracked by ReplaceableMetadataImpl (which also suits this case as we want to be able to replace instances of DIAssignID). Currently the only MDNodes that are tracked by ReplaceableMetadataImpl are temporaries, although until recently* DIArgList was an MDNode that was always marked as replaceable; this patch treats DIAssignID the same way, allowing us to track users and RAUW it at any time.

*Although there were performance issues when doing this with DIArgList that led me to make it no longer inherit from MDNode, the same is _not_ true for DIAssignID; the issue with DIArgList was related to it also being treated as a special case in the RAUW logic, which is not the case for DIAssignID (there is some performance cost to this change in optimized debug-info builds, but to a much lesser extent[0] than the DIArgList situation).

[0]http://llvm-compile-time-tracker.com/compare.php?from=63342e18dd9cd1d187898fe53a7adf44c2d4481d&to=386217ffee106c78fa2a189b97b31a46cad5e828&stat=instructions:u

---
Full diff: https://github.com/llvm/llvm-project/pull/78300.diff


5 Files Affected:

- (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+4) 
- (modified) llvm/include/llvm/IR/Metadata.h (+3-1) 
- (modified) llvm/lib/IR/DebugInfo.cpp (+2-7) 
- (modified) llvm/lib/IR/Metadata.cpp (+11-5) 
- (modified) llvm/lib/IR/Verifier.cpp (+11) 


``````````diff
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index f521862b1a54c2..156f6eb49253de 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -323,6 +323,10 @@ class DIAssignID : public MDNode {
   // This node has no operands to replace.
   void replaceOperandWith(unsigned I, Metadata *New) = delete;
 
+  SmallVector<DPValue *> getAllDPValueUsers() {
+    return Context.getReplaceableUses()->getAllDPValueUsers();
+  }
+
   static DIAssignID *getDistinct(LLVMContext &Context) {
     return getImpl(Context, Distinct);
   }
diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h
index b512451d385f63..befb6975ca18d4 100644
--- a/llvm/include/llvm/IR/Metadata.h
+++ b/llvm/include/llvm/IR/Metadata.h
@@ -1057,6 +1057,7 @@ struct TempMDNodeDeleter {
 class MDNode : public Metadata {
   friend class ReplaceableMetadataImpl;
   friend class LLVMContextImpl;
+  friend class DIAssignID;
 
   /// The header that is coallocated with an MDNode along with its "small"
   /// operands. It is located immediately before the main body of the node.
@@ -1239,7 +1240,8 @@ class MDNode : public Metadata {
   bool isDistinct() const { return Storage == Distinct; }
   bool isTemporary() const { return Storage == Temporary; }
 
-  bool isReplaceable() const { return isTemporary(); }
+  bool isReplaceable() const { return isTemporary() || isAlwaysReplaceable(); }
+  bool isAlwaysReplaceable() const { return getMetadataID() == DIAssignIDKind; }
 
   /// RAUW a temporary.
   ///
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 6b7dd74916517d..e4fcf7acebc061 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -1793,13 +1793,6 @@ void at::deleteAssignmentMarkers(const Instruction *Inst) {
 }
 
 void at::RAUW(DIAssignID *Old, DIAssignID *New) {
-  // Replace MetadataAsValue uses.
-  if (auto *OldIDAsValue =
-          MetadataAsValue::getIfExists(Old->getContext(), Old)) {
-    auto *NewIDAsValue = MetadataAsValue::get(Old->getContext(), New);
-    OldIDAsValue->replaceAllUsesWith(NewIDAsValue);
-  }
-
   // Replace attachments.
   AssignmentInstRange InstRange = getAssignmentInsts(Old);
   // Use intermediate storage for the instruction ptrs because the
@@ -1808,6 +1801,8 @@ void at::RAUW(DIAssignID *Old, DIAssignID *New) {
   SmallVector<Instruction *> InstVec(InstRange.begin(), InstRange.end());
   for (auto *I : InstVec)
     I->setMetadata(LLVMContext::MD_DIAssignID, New);
+
+  Old->replaceAllUsesWith(New);
 }
 
 void at::deleteAll(Function *F) {
diff --git a/llvm/lib/IR/Metadata.cpp b/llvm/lib/IR/Metadata.cpp
index c8f01da30b99ae..a9f64f5442341c 100644
--- a/llvm/lib/IR/Metadata.cpp
+++ b/llvm/lib/IR/Metadata.cpp
@@ -439,16 +439,22 @@ void ReplaceableMetadataImpl::resolveAllUses(bool ResolveUsers) {
 // commentry in DIArgList::handleChangedOperand for details. Hidden behind
 // conditional compilation to avoid a compile time regression.
 ReplaceableMetadataImpl *ReplaceableMetadataImpl::getOrCreate(Metadata &MD) {
-  if (auto *N = dyn_cast<MDNode>(&MD))
-    return N->isResolved() ? nullptr : N->Context.getOrCreateReplaceableUses();
+  if (auto *N = dyn_cast<MDNode>(&MD)) {
+    return !N->isResolved() || N->isAlwaysReplaceable()
+               ? N->Context.getOrCreateReplaceableUses()
+               : nullptr;
+  }
   if (auto ArgList = dyn_cast<DIArgList>(&MD))
     return ArgList;
   return dyn_cast<ValueAsMetadata>(&MD);
 }
 
 ReplaceableMetadataImpl *ReplaceableMetadataImpl::getIfExists(Metadata &MD) {
-  if (auto *N = dyn_cast<MDNode>(&MD))
-    return N->isResolved() ? nullptr : N->Context.getReplaceableUses();
+  if (auto *N = dyn_cast<MDNode>(&MD)) {
+    return !N->isResolved() || N->isAlwaysReplaceable()
+               ? N->Context.getReplaceableUses()
+               : nullptr;
+  }
   if (auto ArgList = dyn_cast<DIArgList>(&MD))
     return ArgList;
   return dyn_cast<ValueAsMetadata>(&MD);
@@ -456,7 +462,7 @@ ReplaceableMetadataImpl *ReplaceableMetadataImpl::getIfExists(Metadata &MD) {
 
 bool ReplaceableMetadataImpl::isReplaceable(const Metadata &MD) {
   if (auto *N = dyn_cast<MDNode>(&MD))
-    return !N->isResolved();
+    return !N->isResolved() || N->isAlwaysReplaceable();
   return isa<ValueAsMetadata>(&MD) || isa<DIArgList>(&MD);
 }
 
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 49241160456cb5..49106da7d2420b 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -172,6 +172,11 @@ struct VerifierSupport {
     }
   }
 
+  void Write(const DPValue *V) {
+    if (V)
+      V->print(*OS, MST, false);
+  }
+
   void Write(const Metadata *MD) {
     if (!MD)
       return;
@@ -4723,6 +4728,12 @@ void Verifier::visitDIAssignIDMetadata(Instruction &I, MDNode *MD) {
                 "dbg.assign not in same function as inst", DAI, &I);
     }
   }
+  for (DPValue *DPV : cast<DIAssignID>(MD)->getAllDPValueUsers()) {
+    CheckDI(DPV->isDbgAssign(),
+            "!DIAssignID should only be used by Assign DPVs.", MD, DPV);
+    CheckDI(DPV->getFunction() == I.getFunction(),
+            "DPVAssign not in same function as inst", DPV, &I);
+  }
 }
 
 void Verifier::visitCallStackMetadata(MDNode *MD) {

``````````

</details>


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


More information about the llvm-commits mailing list