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

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added inline comments.


================
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())
----------------
jmorse wrote:
> 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.
I agree actually; I folded it this way for simplicity's sake, but we can really handle ValueAsMetadata and DIArgList as separate cases. Even with inlining, I worry that the use of SmallVector here might obstruct potential optimizations for the non-DIArgList case. It might only have a small impact, but it's easy to fix nonetheless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100572