Skip to content

Commit

Permalink
[DebugMetadata] Simplify handling subprogram's retainedNodes field. N…
Browse files Browse the repository at this point in the history 
…FCI (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

Authored-by: Kristina Bessonova <kbessonova@accesssoftek.com>
  • Loading branch information
@dzhidzhoev
dzhidzhoev committed Jun 12, 2023
1 parent 11e5a0d commit ed506dd
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 66 deletions.
2 changes: 1 addition & 1 deletion clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
View file
Expand Up @@ -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"}
10 changes: 5 additions & 5 deletions clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
View file
Expand Up @@ -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]])
4 changes: 2 additions & 2 deletions clang/test/CodeGenCXX/debug-info-cxx1y.cpp
View file
Expand Up @@ -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]]
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CodeGenCXX/debug-info-template.cpp
View file
Expand Up @@ -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]*]])
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CodeGenObjC/debug-info-category.m
View file
Expand Up @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions llvm/include/llvm/IR/DIBuilder.h
View file
Expand Up @@ -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.
///
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/IR/DebugInfoMetadata.h
View file
Expand Up @@ -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.
///
Expand Down
75 changes: 28 additions & 47 deletions llvm/lib/IR/DIBuilder.cpp
View file
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
Expand All @@ -795,35 +777,35 @@ 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(
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,
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;
}
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Bindings/OCaml/debuginfo.ml
View file
Expand Up @@ -142,7 +142,7 @@ let test_get_function m dibuilder file_di m_di =
~flags:flags_zero ~is_optimized:false
in
stdout_metadata f_di;
(* CHECK: [[SBPRG_PTR:<0x[0-9a-f]*>]] = distinct !DISubprogram(name: "tfun", linkageName: "tfun", scope: [[MODULE_PTR]], file: [[FILE_PTR]], line: 10, type: [[SBRTNTY_PTR]], scopeLine: 10, spFlags: DISPFlagDefinition, unit: [[CMPUNIT_PTR]], retainedNodes: {{<0x[0-9a-f]*>}})
(* CHECK: [[SBPRG_PTR:<0x[0-9a-f]*>]] = distinct !DISubprogram(name: "tfun", linkageName: "tfun", scope: [[MODULE_PTR]], file: [[FILE_PTR]], line: 10, type: [[SBRTNTY_PTR]], scopeLine: 10, spFlags: DISPFlagDefinition, unit: [[CMPUNIT_PTR]])
*)
Llvm_debuginfo.set_subprogram f f_di;
( match Llvm_debuginfo.get_subprogram f with
Expand Down

0 comments on commit ed506dd

Please sign in to comment.