[RemoveDIs][DebugInfo] Make DIAssignID always replaceable

[SLTozer]

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

[llvmbot]

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Stephen Tozer (SLTozer)

Changes

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 --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) {

[SLTozer]

The reason for this to be added here in a mostly unrelated-looking patch is because of the other verifier change below - now that we can lookup DPValue users of a DIAssignID, the verifier should be checking them the same as it does for debug intrinsic users, and as this is the first time DPValue has come up in the verifier, it needs a Write implementation for the purpose of printing errors.

[jmorse]

This LGTM, presumably it works just as well in the non-remove-DIs mode? (Which needs to keep working for a decent period of time).

[SLTozer]

This LGTM, presumably it works just as well in the non-remove-DIs mode? (Which needs to keep working for a decent period of time).

It does, the only related change is the removal of the "rauw for mav" part at the start for at::rauw - but that's redundant now that we actually call rauw on the DIAssignID directly.