[PATCH] D100572: [DebugInfo] Ensure DIArgList in bitcode has no null or forward metadata refs

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

LGTM, I enjoy the minimal extension of a test to improve coverage too.

I'm a little twitchy about whether this will affect performance in what's an important path for LTO builds: on balance, I don't think it will to any significant extent.



================
Comment at: llvm/lib/Bitcode/Writer/ValueEnumerator.cpp:84-94
+static SmallVector<const Value *> skipMetadataWrapper(const Value *V) {
+  SmallVector<const Value *> WrappedValues;
+  if (const auto *MAV = dyn_cast<MetadataAsValue>(V)) {
+    if (const auto *VAM = dyn_cast<ValueAsMetadata>(MAV->getMetadata())) {
+      WrappedValues.push_back(VAM->getValue());
+    } else if (const auto *AL = dyn_cast<DIArgList>(MAV->getMetadata())) {
+      for (auto *VAM : AL->getArgs())
----------------
I worry about putting some extra loops and bigger return value in what seems to be a hot path (touches all Values once). On the other hand, this should be trivially inlined and DIArgLists won't be common.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100572