[PATCH] D104827: [DebugInfo] Enforce implicit constraints on `distinct` MDNodes

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D104827#3128572 <https://reviews.llvm.org/D104827#3128572>, @scott.linder wrote:

> Update DIArgList::handleChangedOperand to propagate in-place changes which
> would require the DIArgList become distinct up to the owning MetadataAsValue.
>
> This feels a bit odd, but so does allowing DIArgList to be `distinct` as an
> implementation detail, while never serializing that fact (and so making a
> round-trip lossy).

Seems reasonable to try to fix this.

> Semantically this may be OK, but unless we have a good
> reason not to I feel like finding a way to ensure these "inline" nodes are
> always uniqued will make the relationship between IR/bitcode and the in-memory
> representation more obvious.

The tricky constraint is that (most) references to Metadata are not in any use-list.



================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1668-1673
+  auto *Uniqued = uniquify();
+  if (Uniqued != this) {
+    auto &Store = getContext().pImpl->MetadataAsValues;
+    auto MAV = Store.find(this);
+    if (MAV != Store.end())
+      MAV->second->handleChangedMetadata(Uniqued);
----------------
I'm sorry; I'm lacking some context here; I'd appreciate a bit of hand-holding.

I see how this updates the MetadataAsValue reference.

Can you walk me through in way way it's enforced that DIArgList is only referenced by MetadataAsValue? It sounds like textual IR has something, but can you confirm? What about the C++ API? The verifier? And what about historical bitcode... i.e., what can be found there?
- If there are holes in enforcement somehow, what's going to happen to the dangling references to this node?
- Can/should we delete this node now? And/or somehow mark it as dead, so that ValueAsMetadata will look through it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104827/new/

https://reviews.llvm.org/D104827