krisb created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
krisb edited the summary of this revision.
krisb added reviewers: dblaikie, jmmartinez.
krisb added projects: LLVM, debug-info.
krisb published this revision for review.
Herald added a project: clang.
Herald added subscribers: llvm-commits, cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143984
Files:
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
Index: llvm/lib/IR/DIBuilder.cpp
===================================================================
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -57,23 +57,11 @@
}
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() {
@@ -772,26 +760,20 @@
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;
}
@@ -801,9 +783,11 @@
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(
@@ -811,25 +795,23 @@
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;
}
@@ -856,9 +838,8 @@
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);
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===================================================================
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2104,6 +2104,9 @@
void replaceRawLinkageName(MDString *LinkageName) {
replaceOperandWith(3, LinkageName);
}
+ void replaceRetainedNodes(DINodeArray N) {
+ replaceOperandWith(7, N.get());
+ }
/// Check if this subprogram describes the given function.
///
Index: llvm/include/llvm/IR/DIBuilder.h
===================================================================
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -65,15 +65,18 @@
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.
///
Index: clang/test/CodeGenObjC/debug-info-category.m
===================================================================
--- clang/test/CodeGenObjC/debug-info-category.m
+++ clang/test/CodeGenObjC/debug-info-category.m
@@ -37,10 +37,10 @@
// 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,
Index: clang/test/CodeGenCXX/debug-info-template.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-template.cpp
+++ clang/test/CodeGenCXX/debug-info-template.cpp
@@ -222,7 +222,7 @@
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 @@
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
Index: clang/test/CodeGenCXX/debug-info-cxx1y.cpp
===================================================================
--- clang/test/CodeGenCXX/debug-info-cxx1y.cpp
+++ 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]]
Index: clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
===================================================================
--- clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
+++ clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -52,15 +52,15 @@
// 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]])
Index: clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
===================================================================
--- clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
+++ clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
@@ -13,7 +13,7 @@
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"}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
