[DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

[dzhidzhoev]

Summary

RFC https://discourse.llvm.org/t/rfc-dwarfdebug-fix-and-improve-handling-imported-entities-types-and-static-local-in-subprogram-and-lexical-block-scopes/68544

Currently, retainedNodes tracks function-local variables and labels.
To support function-local import, types and static variables (which are globals
in LLVM IR), subsequent patches use the same field. So this patch makes
preliminary refactoring of the code tracking local entities to apply future
functional changes lucidly and cleanly.

No functional changes intended.

[jmmartinez]

llvm/lib/IR/DIBuilder.cpp

783

I'll be tempted to move the call to getSubprogramNodesTrackingVector(Scope) into createLocalVariable. It seems that every time createLocalVariable is called, getSubprogramNodesTrackingVector(Scope) is passed as value for PreservedNodes.

(I've just started reviewing so I may be missing some other modifications)

[krisb]

@jmmartinez thank you for looking at this!

llvm/lib/IR/DIBuilder.cpp

783

That's right, but the problem is in the fact that createLocalVariable() is static while getSubprogramNodesTrackingVector() is a DIBuilder's member. To move the call inside createLocalVariable() we need either to make it a member of DIBuilder or to pass DIBuilder object to it. None of these options looks much better to me, honestly.

[jmmartinez]

llvm/lib/IR/DIBuilder.cpp

783

You're right. I didn't realize createLocalVariable was not a member of DIBuilder.

Ok for me to keep it as it .

[krisb]

Friendly ping.

[aprantl]

llvm/include/llvm/IR/DIBuilder.h

75–78

Do you need a writeable copy, or could you get by with returning an ArrayRef<TrackingMDNodeRef>?

[krisb]

llvm/include/llvm/IR/DIBuilder.h

75–78

It should return smth suitable to emplace new elements, so, right, we need a writable reference here.

[jmmartinez]

Sorry for the delay, I was out-of-office. Patches 1 to 4 look good to me. If nobody raises concerns this week I'll mark them as accepted.

I'm currently reviewing patch 5.

[krisb]

Rebase.

[dzhidzhoev]

@krisb is out of office for some time, so she asked to retake and wrap up this patchset.

[dzhidzhoev]

Rebased.

[dblaikie]

Looks like this series got approved - is it waiting on any particular feedback, or do you need someone to commit it for you?

[dzhidzhoev]

In D143984#4357517, @dblaikie wrote:

Looks like this series got approved - is it waiting on any particular feedback, or do you need someone to commit it for you?

I had been waiting for approval for the change of https://reviews.llvm.org/D144008. How would you suggest landing these commits - individually or all at once?

[dblaikie]

In D143984#4358115, @dzhidzhoev wrote:

In D143984#4357517, @dblaikie wrote:

Looks like this series got approved - is it waiting on any particular feedback, or do you need someone to commit it for you?

I had been waiting for approval for the change of https://reviews.llvm.org/D144008. How would you suggest landing these commits - individually or all at once?

Ah, fair enough.

It's a tough balance - committing them all at once can make it difficult to see which specific change is the cause of a regression/buildbot failure/etc. But committing them too far apart can cause large chunks of breakage/harder to revert, etc. So... probably close-ish together, let the bots give you some results before pushing the next patch, maybe?

[dzhidzhoev]

In D143984#4362988, @dblaikie wrote:

In D143984#4358115, @dzhidzhoev wrote:

In D143984#4357517, @dblaikie wrote:

Looks like this series got approved - is it waiting on any particular feedback, or do you need someone to commit it for you?

I had been waiting for approval for the change of https://reviews.llvm.org/D144008. How would you suggest landing these commits - individually or all at once?

Ah, fair enough.

It's a tough balance - committing them all at once can make it difficult to see which specific change is the cause of a regression/buildbot failure/etc. But committing them too far apart can cause large chunks of breakage/harder to revert, etc. So... probably close-ish together, let the bots give you some results before pushing the next patch, maybe?

Agreed.

[dzhidzhoev]

Rebased.

[steakhal]

I think this patch broke two build bots, and now the CI is red.
https://lab.llvm.org/buildbot/#/builders/16/builds/49282
https://lab.llvm.org/buildbot/#/builders/109/builds/65904

Could you please have a look?

[dzhidzhoev]

In D143984#4405875, @steakhal wrote:

I think this patch broke two build bots, and now the CI is red.
https://lab.llvm.org/buildbot/#/builders/16/builds/49282
https://lab.llvm.org/buildbot/#/builders/109/builds/65904

Could you please have a look?

Thank you! Reverted.
Does anyone know if there is a possibility to subscribe to buildbot emails related to a specific commit if you're not the author of the commit?

[steakhal]

In D143984#4405961, @dzhidzhoev wrote:

In D143984#4405875, @steakhal wrote:

I think this patch broke two build bots, and now the CI is red.
https://lab.llvm.org/buildbot/#/builders/16/builds/49282
https://lab.llvm.org/buildbot/#/builders/109/builds/65904

Could you please have a look?

Thank you! Reverted.
Does anyone know if there is a possibility to subscribe to buildbot emails related to a specific commit if you're not the author of the commit?

To me, it would make sense if its a shared responsibility, or at least the person pushing the commit to see if it needs to be reverted. IDK of any guidelines though.

[dblaikie]

When we migrated to github we gained higher fidelity for attributing authors (we can now commit on someone's behalf but have the commit author properly recorded as the real author - we couldn't do that back on Subversion) - but I think it's meant that fail-mail from bots goes to that author, and not to the committer. Ideally it'd go to both - so, as it stands now, being a commiter-but-not-author is tricky, because you don't get the feedback on the commit you made. You might watch buildbots manually, or at least bhe responsive to other people complaining on the reviews like this.

[dzhidzhoev]

Updated DebugInfo test of OCaml binding.

In D143984#4409015, @dblaikie wrote:

When we migrated to github we gained higher fidelity for attributing authors (we can now commit on someone's behalf but have the commit author properly recorded as the real author - we couldn't do that back on Subversion) - but I think it's meant that fail-mail from bots goes to that author, and not to the committer. Ideally it'd go to both - so, as it stands now, being a commiter-but-not-author is tricky, because you don't get the feedback on the commit you made. You might watch buildbots manually, or at least bhe responsive to other people complaining on the reviews like this.

Thanks! We agreed to use Authored-by attribute.

clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c

 // REQUIRES: x86-registered-target
 // RUN: %clang -target x86_64 -g -S -O2 -emit-llvm -o - %s | FileCheck %s
 
 #define __tag1 __attribute__((btf_decl_tag("tag1")))
 #define __tag2 __attribute__((btf_decl_tag("tag2")))
 
 struct t1 {
   int a;
 };
 
 extern int __tag1 __tag2 foo(struct t1 *);
 int foo2(struct t1 *arg) {
   return foo(arg);
 }
 
-// CHECK: ![[#]] = !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[#]], flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: ![[#]], annotations: ![[ANNOT:[0-9]+]])
+// CHECK: ![[#]] = !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], line: [[#]], type: ![[#]], flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, annotations: ![[ANNOT:[0-9]+]])
 // CHECK: ![[ANNOT]] = !{![[TAG1:[0-9]+]], ![[TAG2:[0-9]+]]}
 // CHECK: ![[TAG1]] = !{!"btf_decl_tag", !"tag1"}
 // CHECK: ![[TAG2]] = !{!"btf_decl_tag", !"tag2"}

clang/test/CodeGenCXX/aix-static-init-debug-info.cpp

 // CHECK: }
 
 // CHECK: define internal void @_GLOBAL__D_a() [[ATTR:#[0-9]+]] !dbg ![[DBGVAR27:[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @__finalize_v(), !dbg ![[DBGVAR28:[0-9]+]]
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: "__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: "__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR19]] = !DILocation(line: 14, column: 3, scope: ![[DBGVAR16]])
 // CHECK: ![[DBGVAR19b]] = !DILocation(line: 0, scope: ![[DBGVAR16]])
-// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR21b]] = !DILocation(line: 0, scope: ![[DBGVAR20]])
 // CHECK: ![[DBGVAR21]] = !DILocation(line: 14, column: 3, scope: ![[DBGVAR20]])
-// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR24]] = !DILocation(line: 14, column: 3, scope: ![[DBGVAR22]])
-// CHECK: ![[DBGVAR25]] = distinct !DISubprogram(linkageName: "_GLOBAL__sub_I__", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR25]] = distinct !DISubprogram(linkageName: "_GLOBAL__sub_I__", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR26]] = !DILocation(line: 0, scope: ![[DBGVAR25]])
-// CHECK: ![[DBGVAR27]] = distinct !DISubprogram(linkageName: "_GLOBAL__D_a", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR27]] = distinct !DISubprogram(linkageName: "_GLOBAL__D_a", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR28]] = !DILocation(line: 0, scope: ![[DBGVAR27]])

clang/test/CodeGenCXX/debug-info-cxx1y.cpp

 // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only -std=c++14 -emit-llvm -debug-info-kind=limited %s -o - | FileCheck %s
 
 // CHECK: imports: [[IMPS:![0-9]*]]
 
 // CHECK: [[IMPS]] = !{[[IMP:![0-9]*]]}
 // CHECK: [[IMP]] = !DIImportedEntity(
 // CHECK-SAME: entity: [[F3:![0-9]*]]
 // CHECK: [[F3]] = distinct !DISubprogram(name: "f3"
 // CHECK-SAME:          type: [[SUBROUTINE_TYPE:![0-9]*]]
 // CHECK: [[SUBROUTINE_TYPE]] = !DISubroutineType(types: [[TYPE_LIST:![0-9]*]])
 // CHECK: [[TYPE_LIST]] = !{[[INT:![0-9]*]]}
 // CHECK: [[INT]] = !DIBasicType(name: "int"
 
-// CHECK: [[EMPTY:![0-9]*]] = !{}
 // CHECK: [[FOO:![0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo",
-// CHECK-SAME:             elements: [[EMPTY]]
+// CHECK-SAME:             elements: [[EMPTY:![0-9]*]]
+// CHECK: [[EMPTY]] = !{}
 
 // FIXME: The context of this definition should be the CU/file scope, not the class.
 // CHECK: !DISubprogram(name: "func", {{.*}} scope: [[FOO]]
 // CHECK-SAME:          type: [[SUBROUTINE_TYPE]]
 // CHECK-SAME:          DISPFlagDefinition
 // CHECK-SAME:          declaration: [[FUNC_DECL:![0-9]*]]
 // CHECK: [[FUNC_DECL]] = !DISubprogram(name: "func",
 // CHECK-SAME:                          scope: [[FOO]]

clang/test/CodeGenCXX/debug-info-template.cpp

 } // namespace EmptyInnerPack
 
 namespace RawFuncQual {
 struct t1; // use this to ensure the type parameter doesn't shift due to other test cases in this file
 template<typename T1, typename T2, typename T3, typename T4>
 void f1() { }
 template void f1<t1 () volatile, t1 () const volatile, t1 () &, t1 () &&>();
 // CHECK: !DISubprogram(name: "f1<RawFuncQual::t1 () volatile, RawFuncQual::t1 () const volatile, RawFuncQual::t1 () &, RawFuncQual::t1 () &&>", 
-// CHECK-SAME: templateParams: ![[RAW_FUNC_QUAL_ARGS:[0-9]*]],
+// CHECK-SAME: templateParams: ![[RAW_FUNC_QUAL_ARGS:[0-9]*]]
 
 // CHECK: ![[RAW_FUNC_QUAL_ARGS]] = !{![[RAW_FUNC_QUAL_T1:[0-9]*]], ![[RAW_FUNC_QUAL_T2:[0-9]*]], ![[RAW_FUNC_QUAL_T3:[0-9]*]], ![[RAW_FUNC_QUAL_T4:[0-9]*]]}
 // CHECK: ![[RAW_FUNC_QUAL_T1]] = !DITemplateTypeParameter(name: "T1", type: ![[RAW_FUNC_QUAL_VOL:[0-9]*]])
 // CHECK: ![[RAW_FUNC_QUAL_VOL]] = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: ![[RAW_FUNC_QUAL_TYPE:[0-9]*]])
 // CHECK: ![[RAW_FUNC_QUAL_TYPE]] = !DISubroutineType(types: ![[RAW_FUNC_QUAL_LIST:[0-9]*]]
 // CHECK: ![[RAW_FUNC_QUAL_LIST]] = !{![[RAW_FUNC_QUAL_STRUCT:[0-9]*]]}
 // CHECK: ![[RAW_FUNC_QUAL_STRUCT]] = !DICompositeType(tag: DW_TAG_structure_type, name: "t1"
 // CHECK: ![[RAW_FUNC_QUAL_T2]] = !DITemplateTypeParameter(name: "T2", type: ![[RAW_FUNC_QUAL_CNST:[0-9]*]])

 namespace TemplateTemplateParamInlineNamespace {
 inline namespace inl {
   template<typename>
   struct t1 { };
 } // namespace inl
 template<template<typename> class> void f1() { }
 template void f1<t1>();
 // CHECK: !DISubprogram(name: "f1<TemplateTemplateParamInlineNamespace::inl::t1>",
-// CHECK-SAME: templateParams: ![[TEMP_TEMP_INL_ARGS:[0-9]*]],
+// CHECK-SAME: templateParams: ![[TEMP_TEMP_INL_ARGS:[0-9]*]]
 // CHECK: ![[TEMP_TEMP_INL_ARGS]] = !{![[TEMP_TEMP_INL_ARGS_T:[0-9]*]]}
 // CHECK: ![[TEMP_TEMP_INL_ARGS_T]] = !DITemplateValueParameter(tag: DW_TAG_GNU_template_template_param, value: !"TemplateTemplateParamInlineNamespace::inl::t1")
 } // namespace TemplateTemplateParamInlineNamespace
 
 namespace NoPreferredNames {
 template <typename T> struct t1;
 using t1i = t1<int>;
 template <typename T>
 struct __attribute__((__preferred_name__(t1i))) t1 {};
 template <typename T>
 void f1() {}
 template void f1<t1<int>>();
 // CHECK: !DISubprogram(name: "f1<NoPreferredNames::t1<int> >",
 
 } // namespace NoPreferredNames

clang/test/CodeGenObjC/debug-info-category.m

 - (id)add:(Foo *)addend {
   return [self integer:[self integer] + [addend integer]];
 }
 @end
 
 // CHECK: ![[STRUCT:.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Foo"
 
 // Verify "not a definition" by showing spFlags doesn't have DISPFlagDefinition.
-// DWARF5: !DISubprogram(name: "-[Foo integer]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
-// DWARF5: !DISubprogram(name: "-[Foo integer:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
-// DWARF5: !DISubprogram(name: "+[Foo(Bar) zero:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
-// DWARF5: !DISubprogram(name: "-[Foo(Bar) add:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
+// DWARF5: !DISubprogram(name: "-[Foo integer]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit)
+// DWARF5: !DISubprogram(name: "-[Foo integer:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit)
+// DWARF5: !DISubprogram(name: "+[Foo(Bar) zero:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit)
+// DWARF5: !DISubprogram(name: "-[Foo(Bar) add:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit)
 
 // DWARF4-NOT: !DISubprogram(name: "-[Foo integer]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
 // DWARF4-NOT: !DISubprogram(name: "-[Foo integer:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
 // DWARF4-NOT: !DISubprogram(name: "+[Foo(Bar) zero:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
 // DWARF4-NOT: !DISubprogram(name: "-[Foo(Bar) add:]", scope: ![[STRUCT]], {{.*}} spFlags: DISPFlagLocalToUnit,
 
 // CHECK: = distinct !DISubprogram(name: "-[Foo integer]"{{.*}} DISPFlagDefinition
 // CHECK: = distinct !DISubprogram(name: "-[Foo integer:]"{{.*}} DISPFlagDefinition
 // CHECK: = distinct !DISubprogram(name: "+[Foo(Bar) zero:]"{{.*}} DISPFlagDefinition
 // CHECK: = distinct !DISubprogram(name: "-[Foo(Bar) add:]"{{.*}} DISPFlagDefinition

llvm/include/llvm/IR/DIBuilder.h

class DIBuilder {
     /// Metadata all of type DIMacroNode.
     /// DIMacroNode's with nullptr parent are DICompileUnit direct children.
     MapVector<MDNode *, SetVector<Metadata *>> AllMacrosPerParent;
 
     /// Track nodes that may be unresolved.
     SmallVector<TrackingMDNodeRef, 4> UnresolvedNodes;
     bool AllowUnresolvedNodes;
 
-    /// Each subprogram's preserved local variables.
+    /// Each subprogram's preserved local variables and labels.
     ///
     /// Do not use a std::vector.  Some versions of libc++ apparently copy
     /// instead of move on grow operations, and TrackingMDRef is expensive to
     /// copy.
-    DenseMap<MDNode *, SmallVector<TrackingMDNodeRef, 1>> PreservedVariables;
+    DenseMap<DISubprogram *, SmallVector<TrackingMDNodeRef, 4>>
+        SubprogramTrackedNodes;
 
-    /// Each subprogram's preserved labels.
-    DenseMap<MDNode *, SmallVector<TrackingMDNodeRef, 1>> PreservedLabels;
+    SmallVectorImpl<TrackingMDNodeRef> &
+    getSubprogramNodesTrackingVector(const DIScope *S) {
+      return SubprogramTrackedNodes[cast<DILocalScope>(S)->getSubprogram()];
+    }
 
     /// Create a temporary.
     ///
     /// Create an \a temporary node and track it in \a UnresolvedNodes.
     void trackIfUnresolved(MDNode *N);
 
     /// Internal helper for insertDeclare.
     Instruction *insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo,

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   }
   MDString *getRawTargetFuncName() const {
     return getNumOperands() > 12 ? getOperandAs<MDString>(12) : nullptr;
   }
 
   void replaceRawLinkageName(MDString *LinkageName) {
     replaceOperandWith(3, LinkageName);
   }
+  void replaceRetainedNodes(DINodeArray N) {
+    replaceOperandWith(7, N.get());
+  }
 
   /// Check if this subprogram describes the given function.
   ///
   /// FIXME: Should this be looking through bitcasts?
   bool describes(const Function *F) const;
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DISubprogramKind;

llvm/lib/IR/DIBuilder.cpp

void DIBuilder::trackIfUnresolved(MDNode *N) {
   if (N->isResolved())
     return;
 
   assert(AllowUnresolvedNodes && "Cannot handle unresolved nodes");
   UnresolvedNodes.emplace_back(N);
 }
 
 void DIBuilder::finalizeSubprogram(DISubprogram *SP) {
-  MDTuple *Temp = SP->getRetainedNodes().get();
-  if (!Temp || !Temp->isTemporary())
-    return;
-
-  SmallVector<Metadata *, 16> RetainedNodes;
-
-  auto PV = PreservedVariables.find(SP);
-  if (PV != PreservedVariables.end())
-    RetainedNodes.append(PV->second.begin(), PV->second.end());
-
-  auto PL = PreservedLabels.find(SP);
-  if (PL != PreservedLabels.end())
-    RetainedNodes.append(PL->second.begin(), PL->second.end());
-
-  DINodeArray Node = getOrCreateArray(RetainedNodes);
-
-  TempMDTuple(Temp)->replaceAllUsesWith(Node.get());
+  auto PN = SubprogramTrackedNodes.find(SP);
+  if (PN != SubprogramTrackedNodes.end())
+    SP->replaceRetainedNodes(
+        MDTuple::get(VMContext, SmallVector<Metadata *, 16>(PN->second.begin(),
+                                                            PN->second.end())));
 }
 
 void DIBuilder::finalize() {
   if (!CUNode) {
     assert(!AllowUnresolvedNodes &&
            "creating type nodes without a CU is not supported");
     return;
   }
return DIGlobalVariable::getTemporary(
              LineNumber, Ty, IsLocalToUnit, false,
              cast_or_null<DIDerivedType>(Decl), TemplateParams, AlignInBits,
              nullptr)
       .release();
 }
 
 static DILocalVariable *createLocalVariable(
     LLVMContext &VMContext,
-    DenseMap<MDNode *, SmallVector<TrackingMDNodeRef, 1>> &PreservedVariables,
-    DIScope *Scope, StringRef Name, unsigned ArgNo, DIFile *File,
+    SmallVectorImpl<TrackingMDNodeRef> &PreservedNodes,
+    DIScope *Context, StringRef Name, unsigned ArgNo, DIFile *File,
     unsigned LineNo, DIType *Ty, bool AlwaysPreserve, DINode::DIFlags Flags,
     uint32_t AlignInBits, DINodeArray Annotations = nullptr) {
-  // FIXME: Why getNonCompileUnitScope()?
-  // FIXME: Why is "!Context" okay here?
   // FIXME: Why doesn't this check for a subprogram or lexical block (AFAICT
   // the only valid scopes)?
-  DIScope *Context = getNonCompileUnitScope(Scope);
-
-  auto *Node = DILocalVariable::get(
-      VMContext, cast_or_null<DILocalScope>(Context), Name, File, LineNo, Ty,
+  auto *Scope = cast<DILocalScope>(Context);
+  auto *Node = DILocalVariable::get(VMContext, Scope, Name, File, LineNo, Ty,
                                     ArgNo, Flags, AlignInBits, Annotations);
   if (AlwaysPreserve) {
     // The optimizer may remove local variables. If there is an interest
     // to preserve variable info in such situation then stash it in a
     // named mdnode.
-    DISubprogram *Fn = getDISubprogram(Scope);
-    assert(Fn && "Missing subprogram for local variable");
-    PreservedVariables[Fn].emplace_back(Node);
+    PreservedNodes.emplace_back(Node);
   }
   return Node;
 }
 
 DILocalVariable *DIBuilder::createAutoVariable(DIScope *Scope, StringRef Name,
                                                DIFile *File, unsigned LineNo,
                                                DIType *Ty, bool AlwaysPreserve,
                                                DINode::DIFlags Flags,
                                                uint32_t AlignInBits) {
-  return createLocalVariable(VMContext, PreservedVariables, Scope, Name,
-                             /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve,
-                             Flags, AlignInBits);
+  assert(Scope && isa<DILocalScope>(Scope) &&
+         "Unexpected scope for a local variable.");
+  return createLocalVariable(
+      VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name,
+      /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve, Flags, AlignInBits);
 }
 
 DILocalVariable *DIBuilder::createParameterVariable(
     DIScope *Scope, StringRef Name, unsigned ArgNo, DIFile *File,
     unsigned LineNo, DIType *Ty, bool AlwaysPreserve, DINode::DIFlags Flags,
     DINodeArray Annotations) {
   assert(ArgNo && "Expected non-zero argument number for parameter");
-  return createLocalVariable(VMContext, PreservedVariables, Scope, Name, ArgNo,
-                             File, LineNo, Ty, AlwaysPreserve, Flags,
-                             /*AlignInBits=*/0, Annotations);
+  assert(Scope && isa<DILocalScope>(Scope) &&
+         "Unexpected scope for a local variable.");
+  return createLocalVariable(
+      VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name, ArgNo,
+      File, LineNo, Ty, AlwaysPreserve, Flags, /*AlignInBits=*/0, Annotations);
 }
 
-DILabel *DIBuilder::createLabel(DIScope *Scope, StringRef Name, DIFile *File,
+DILabel *DIBuilder::createLabel(DIScope *Context, StringRef Name, DIFile *File,
                                  unsigned LineNo, bool AlwaysPreserve) {
-  DIScope *Context = getNonCompileUnitScope(Scope);
-
-  auto *Node = DILabel::get(VMContext, cast_or_null<DILocalScope>(Context),
-                            Name, File, LineNo);
+  auto *Scope = cast<DILocalScope>(Context);
+  auto *Node = DILabel::get(VMContext, Scope, Name, File, LineNo);
 
   if (AlwaysPreserve) {
     /// The optimizer may remove labels. If there is an interest
     /// to preserve label info in such situation then append it to
     /// the list of retained nodes of the DISubprogram.
-    DISubprogram *Fn = getDISubprogram(Scope);
-    assert(Fn && "Missing subprogram for label");
-    PreservedLabels[Fn].emplace_back(Node);
+    getSubprogramNodesTrackingVector(Scope).emplace_back(Node);
   }
   return Node;
 }
 
 DIExpression *DIBuilder::createExpression(ArrayRef<uint64_t> Addr) {
   return DIExpression::get(VMContext, Addr);
 }
 
DISubprogram *DIBuilder::createFunction(
     DINode::DIFlags Flags, DISubprogram::DISPFlags SPFlags,
     DITemplateParameterArray TParams, DISubprogram *Decl,
     DITypeArray ThrownTypes, DINodeArray Annotations,
     StringRef TargetFuncName) {
   bool IsDefinition = SPFlags & DISubprogram::SPFlagDefinition;
   auto *Node = getSubprogram(
       /*IsDistinct=*/IsDefinition, VMContext, getNonCompileUnitScope(Context),
       Name, LinkageName, File, LineNo, Ty, ScopeLine, nullptr, 0, 0, Flags,
-      SPFlags, IsDefinition ? CUNode : nullptr, TParams, Decl,
-      MDTuple::getTemporary(VMContext, std::nullopt).release(), ThrownTypes,
-      Annotations, TargetFuncName);
+      SPFlags, IsDefinition ? CUNode : nullptr, TParams, Decl, nullptr,
+      ThrownTypes, Annotations, TargetFuncName);
 
   if (IsDefinition)
     AllSubprograms.push_back(Node);
   trackIfUnresolved(Node);
   return Node;
 }
 
 DISubprogram *DIBuilder::createTempFunctionFwdDecl(