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

Vladislav Dzhidzhoev via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 7 07:43:33 PDT 2023


Author: Kristina Bessonova
Date: 2023-06-07T16:43:12+02:00
New Revision: 4418434c6de7a861e241ba2448ea4a12080cf08f

URL: https://github.com/llvm/llvm-project/commit/4418434c6de7a861e241ba2448ea4a12080cf08f
DIFF: https://github.com/llvm/llvm-project/commit/4418434c6de7a861e241ba2448ea4a12080cf08f.diff

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

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.

Differential Revision: https://reviews.llvm.org/D143984

Added: 
    

Modified: 
    clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
    clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
    clang/test/CodeGenCXX/debug-info-cxx1y.cpp
    clang/test/CodeGenCXX/debug-info-template.cpp
    clang/test/CodeGenObjC/debug-info-category.m
    llvm/include/llvm/IR/DIBuilder.h
    llvm/include/llvm/IR/DebugInfoMetadata.h
    llvm/lib/IR/DIBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c b/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
index e3bfd71a56aad..5bbb05e6b8098 100644
--- a/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
+++ b/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
@@ -13,7 +13,7 @@ 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"}

diff  --git a/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
index a35ca27e10d2a..6453e4379ae9a 100644
--- a/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
+++ b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -52,15 +52,15 @@ X v;
 // 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]])

diff  --git a/clang/test/CodeGenCXX/debug-info-cxx1y.cpp b/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
index 6ec55626033d6..42c801ad6530c 100644
--- a/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
+++ b/clang/test/CodeGenCXX/debug-info-cxx1y.cpp
@@ -11,9 +11,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]]

diff  --git a/clang/test/CodeGenCXX/debug-info-template.cpp b/clang/test/CodeGenCXX/debug-info-template.cpp
index cf45524769785..a5a17c932604c 100644
--- a/clang/test/CodeGenCXX/debug-info-template.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template.cpp
@@ -222,7 +222,7 @@ 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]*]])
@@ -254,7 +254,7 @@ inline 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

diff  --git a/clang/test/CodeGenObjC/debug-info-category.m b/clang/test/CodeGenObjC/debug-info-category.m
index 5ff2365722f39..4cd71f25938f9 100644
--- a/clang/test/CodeGenObjC/debug-info-category.m
+++ b/clang/test/CodeGenObjC/debug-info-category.m
@@ -37,10 +37,10 @@ - (id)add:(Foo *)addend {
 // 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,

diff  --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index 7bcbc78a7fe18..90ba6a7d1110c 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -64,15 +64,18 @@ namespace llvm {
     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.
     ///

diff  --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 325592421ab9d..defd1d5c2a1ea 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1856,6 +1856,9 @@ class DISubprogram : public DILocalScope {
   void replaceRawLinkageName(MDString *LinkageName) {
     replaceOperandWith(3, LinkageName);
   }
+  void replaceRetainedNodes(DINodeArray N) {
+    replaceOperandWith(7, N.get());
+  }
 
   /// Check if this subprogram describes the given function.
   ///

diff  --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index 1428cebfd6dac..4fa61b87c379f 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -52,23 +52,11 @@ void DIBuilder::trackIfUnresolved(MDNode *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() {
@@ -766,26 +754,20 @@ DIGlobalVariable *DIBuilder::createTempGlobalVariableFwdDecl(
 
 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,
-      ArgNo, Flags, AlignInBits, Annotations);
+  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;
 }
@@ -795,9 +777,11 @@ DILocalVariable *DIBuilder::createAutoVariable(DIScope *Scope, StringRef Name,
                                                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(
@@ -805,25 +789,23 @@ DILocalVariable *DIBuilder::createParameterVariable(
     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,
-                                unsigned LineNo, bool AlwaysPreserve) {
-  DIScope *Context = getNonCompileUnitScope(Scope);
-
-  auto *Node = DILabel::get(VMContext, cast_or_null<DILocalScope>(Context),
-                            Name, File, LineNo);
+DILabel *DIBuilder::createLabel(DIScope *Context, StringRef Name, DIFile *File,
+                                 unsigned LineNo, bool AlwaysPreserve) {
+  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;
 }
@@ -850,9 +832,8 @@ DISubprogram *DIBuilder::createFunction(
   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);


        


More information about the cfe-commits mailing list