[DebugInfo][DwarfDebug] Move emission of globals from beginModule() to endModule() (5/7)#184219
Conversation
…() (5/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 This patch moves the emission of global variables from `DwarfDebug::beginModule()` to `DwarfDebug::endModule()`. It has the following effects: 1. The order of debug entities in the resulting DWARF changes. 2. Currently, if a DISubprogram requires emission of both concrete out-of-line and inlined subprogram DIEs, and such a subprogram contains a static local variable, the DIE for the variable is emitted into the concrete out-of-line subprogram DIE. As a result, the variable is not available in debugger when breaking at the inlined function instance. It happens because static locals are emitted in `DwarfDebug::beginModule()`, but abstract DIEs for functions that are not completely inlined away are created only later during `DwarfDebug::endFunctionImpl()` calls. With this patch, DIEs for static local variables of subprograms that have both inlined and the concrete out-of-line instances are placed into abstract subprogram DIEs. They get visible in debugger when breaking at concrete out-of-line and inlined function instances. llvm/test/DebugInfo/Generic/inlined-static-var.ll illustrates that. 3. It will allow to simplify abstract subprogram DIEs creation by reverting llvm#159104 later. This is needed to simplify DWARF emission in a context of proper support of function-local static variables (which are globals in terms of LLVM IR) which comes in the next patch (https://reviews.llvm.org/D144008), making all function-local entities handled in `DwarfDebug::endModuleImpl()`. This change has already been reviewed in https://reviews.llvm.org/D144007, but I’m opening a new PR because it has been a while since it was approved. Authored-by: Kristina Bessonova <[email protected]>
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-backend-amdgpu Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesThis patch moves the emission of global variables from It has the following effects:
This is needed to simplify DWARF emission in a context of proper support of function-local static variables which comes in the next patch (https://reviews.llvm.org/D144008), making all function-local entities handled in This change has already been reviewed in https://reviews.llvm.org/D144007, but I’m opening a new PR because it has been a while since it was approved (the process of the patchset merge was stuck on the previous patch). Authored-by: Kristina Bessonova <[email protected]> Patch is 89.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/184219.diff 44 Files Affected:
diff --git a/lld/test/wasm/debuginfo.test b/lld/test/wasm/debuginfo.test
index 7e6bd51cb35c9..09a998aa3521c 100644
--- a/lld/test/wasm/debuginfo.test
+++ b/lld/test/wasm/debuginfo.test
@@ -44,9 +44,23 @@ CHECK-NEXT: DW_AT_producer ("clang version 7.0.0 (trunk {{.*}})")
CHECK-NEXT: DW_AT_language (DW_LANG_C99)
CHECK-NEXT: DW_AT_name ("hi_foo.c")
+CHECK: DW_TAG_subprogram
+CHECK-NEXT: DW_AT_low_pc
+CHECK-NEXT: DW_AT_high_pc
+CHECK-NEXT: DW_AT_frame_base
+CHECK-NEXT: DW_AT_name ("foo")
+CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
+CHECK-NEXT: DW_AT_decl_line (3)
+
+CHECK: DW_TAG_formal_parameter
+CHECK-NEXT: DW_AT_location (DW_OP_WASM_location 0x0 0x0, DW_OP_stack_value)
+CHECK-NEXT: DW_AT_name ("p")
+CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
+CHECK-NEXT: DW_AT_decl_line (3)
+
CHECK: DW_TAG_variable
CHECK-NEXT: DW_AT_name ("y")
-CHECK-NEXT: DW_AT_type (0x000000ac "int[2]")
+CHECK-NEXT: DW_AT_type (0x000000d4 "int[2]")
CHECK-NEXT: DW_AT_external (true)
CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
CHECK-NEXT: DW_AT_decl_line (1)
@@ -68,23 +82,9 @@ CHECK-NEXT: DW_AT_encoding (DW_ATE_unsigned)
CHECK: DW_TAG_variable
CHECK-NEXT: DW_AT_name ("z")
-CHECK-NEXT: DW_AT_type (0x000000ac "int[2]")
+CHECK-NEXT: DW_AT_type (0x000000d4 "int[2]")
CHECK-NEXT: DW_AT_external (true)
CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
CHECK-NEXT: DW_AT_decl_line (8)
CHECK-NEXT: DW_AT_location (DW_OP_addr 0xffffffff)
-CHECK: DW_TAG_subprogram
-CHECK-NEXT: DW_AT_low_pc
-CHECK-NEXT: DW_AT_high_pc
-CHECK-NEXT: DW_AT_frame_base
-CHECK-NEXT: DW_AT_name ("foo")
-CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
-CHECK-NEXT: DW_AT_decl_line (3)
-
-CHECK: DW_TAG_formal_parameter
-CHECK-NEXT: DW_AT_location (DW_OP_WASM_location 0x0 0x0, DW_OP_stack_value)
-CHECK-NEXT: DW_AT_name ("p")
-CHECK-NEXT: DW_AT_decl_file ("{{.*}}hi_foo.c")
-CHECK-NEXT: DW_AT_decl_line (3)
-
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 042c98fa1c02f..cd91a2a08d536 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -1256,14 +1256,6 @@ void DwarfDebug::beginModule(Module *M) {
assert(NumDebugCUs > 0 && "Asm unexpectedly initialized");
SingleCU = NumDebugCUs == 1;
- DenseMap<DIGlobalVariable *, SmallVector<DwarfCompileUnit::GlobalExpr, 1>>
- GVMap;
- for (const GlobalVariable &Global : M->globals()) {
- SmallVector<DIGlobalVariableExpression *, 1> GVs;
- Global.getDebugInfo(GVs);
- for (auto *GVE : GVs)
- GVMap[GVE->getVariable()].push_back({&Global, GVE->getExpression()});
- }
// Create the symbol that designates the start of the unit's contribution
// to the string offsets table. In a split DWARF scenario, only the skeleton
@@ -1298,24 +1290,6 @@ void DwarfDebug::beginModule(Module *M) {
DwarfCompileUnit &CU = getOrCreateDwarfCompileUnit(CUNode);
- // Global Variables.
- for (auto *GVE : CUNode->getGlobalVariables()) {
- // Don't bother adding DIGlobalVariableExpressions listed in the CU if we
- // already know about the variable and it isn't adding a constant
- // expression.
- auto &GVMapEntry = GVMap[GVE->getVariable()];
- auto *Expr = GVE->getExpression();
- if (!GVMapEntry.size() || (Expr && Expr->isConstant()))
- GVMapEntry.push_back({nullptr, Expr});
- }
-
- DenseSet<DIGlobalVariable *> Processed;
- for (auto *GVE : CUNode->getGlobalVariables()) {
- DIGlobalVariable *GV = GVE->getVariable();
- if (Processed.insert(GV).second)
- CU.getOrCreateGlobalVariableDIE(GV, sortGlobalExprs(GVMap[GV]));
- }
-
for (auto *Ty : CUNode->getEnumTypes()) {
assert(!isa_and_nonnull<DILocalScope>(Ty->getScope()) &&
"Unexpected function-local entity in 'enums' CU field.");
@@ -1513,9 +1487,47 @@ void DwarfDebug::endModule() {
assert(CurFn == nullptr);
assert(CurMI == nullptr);
- for (const auto &P : CUMap) {
- const auto *CUNode = cast<DICompileUnit>(P.first);
- DwarfCompileUnit *CU = &*P.second;
+ const Module *M = MMI->getModule();
+
+ // Collect global variables info.
+ DenseMap<DIGlobalVariable *, SmallVector<DwarfCompileUnit::GlobalExpr, 1>>
+ GVMap;
+ for (const GlobalVariable &Global : M->globals()) {
+ SmallVector<DIGlobalVariableExpression *, 1> GVs;
+ Global.getDebugInfo(GVs);
+ for (auto *GVE : GVs)
+ GVMap[GVE->getVariable()].push_back({&Global, GVE->getExpression()});
+ }
+
+ for (DICompileUnit *CUNode : M->debug_compile_units()) {
+ DwarfCompileUnit *CU = CUMap.lookup(CUNode);
+
+ // If the CU hasn't been emitted yet, create it here unless it is empty.
+ if (!CU) {
+ if (CUNode->getImportedEntities().empty() &&
+ CUNode->getEnumTypes().empty() &&
+ CUNode->getRetainedTypes().empty() &&
+ CUNode->getGlobalVariables().empty() && CUNode->getMacros().empty())
+ continue;
+ CU = &getOrCreateDwarfCompileUnit(CUNode);
+ }
+
+ // Emit Global Variables.
+ for (auto *GVE : CUNode->getGlobalVariables()) {
+ // Don't bother adding DIGlobalVariableExpressions listed in the CU if we
+ // already know about the variable and it isn't adding a constant
+ // expression.
+ auto &GVMapEntry = GVMap[GVE->getVariable()];
+ auto *Expr = GVE->getExpression();
+ if (!GVMapEntry.size() || (Expr && Expr->isConstant()))
+ GVMapEntry.push_back({nullptr, Expr});
+ }
+ DenseSet<DIGlobalVariable *> Processed;
+ for (auto *GVE : CUNode->getGlobalVariables()) {
+ DIGlobalVariable *GV = GVE->getVariable();
+ if (Processed.insert(GV).second)
+ CU->getOrCreateGlobalVariableDIE(GV, sortGlobalExprs(GVMap[GV]));
+ }
// Emit imported entities.
for (auto *IE : CUNode->getImportedEntities()) {
diff --git a/llvm/test/CodeGen/X86/dbg-distringtype-uint.ll b/llvm/test/CodeGen/X86/dbg-distringtype-uint.ll
index 7542c1b8db327..9eb7c9c59ebd4 100644
--- a/llvm/test/CodeGen/X86/dbg-distringtype-uint.ll
+++ b/llvm/test/CodeGen/X86/dbg-distringtype-uint.ll
@@ -1,17 +1,19 @@
; RUN: llc -mtriple=x86_64 -filetype=obj < %s | llvm-dwarfdump -debug-info - | FileCheck %s
-; Ensure that static local variable elemnt is placed in abstract subprogram DIE.
; CHECK: DW_TAG_subprogram
; CHECK-NOT: DW_TAG
; CHECK: DW_AT_inline (DW_INL_inlined)
-; CHECK-EMPTY:
-; CHECK-NEXT: DW_TAG_variable
+; CHECK-NOT: DW_TAG
+; CHECK: [[SYM:[a-z0-9]+]]: DW_TAG_formal_parameter
+; CHECK: DW_AT_name ("esym")
+; CHECK: DW_AT_type ([[TYPE:[a-z0-9]+]] "CHARACTER_1")
+; CHECK-NOT: DW_TAG
+; CHECK: DW_TAG_formal_parameter
+; CHECK-NOT: DW_TAG
+; Ensure that static local variable elemnt is placed in abstract subprogram DIE.
+; CHECK: DW_TAG_variable
; CHECK-NEXT: DW_AT_name ("elemnt")
-; CHECK: [[SYM:[a-z0-9]+]]: DW_TAG_formal_parameter
-; CHECK: DW_AT_name ("esym")
-; CHECK: DW_AT_type ([[TYPE:[a-z0-9]+]] "CHARACTER_1")
-;
; CHECK: DW_TAG_formal_parameter
; CHECK: DW_AT_const_value (7523094288207667809)
; CHECK: DW_AT_abstract_origin ([[SYM]] "esym")
diff --git a/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll b/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
index 399d80c778072..1183202dfadaf 100644
--- a/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
+++ b/llvm/test/DebugInfo/AArch64/DW_AT_APPLE_enum_kind.ll
@@ -15,16 +15,16 @@
; CHECK: .debug_abbrev contents:
-; CHECK: [3] DW_TAG_enumeration_type DW_CHILDREN_yes
+; CHECK: [2] DW_TAG_enumeration_type DW_CHILDREN_yes
; CHECK: DW_AT_APPLE_enum_kind DW_FORM_data1
; CHECK: .debug_info contents:
-; CHECK: DW_TAG_enumeration_type [3]
+; CHECK: DW_TAG_enumeration_type [2]
; CHECK-DAG: DW_AT_name {{.*}} string = "OpenEnum"
; CHECK-DAG: DW_AT_APPLE_enum_kind [DW_FORM_data1] (DW_APPLE_ENUM_KIND_Open)
-; CHECK: DW_TAG_enumeration_type [3]
+; CHECK: DW_TAG_enumeration_type [2]
; CHECK-DAG: DW_AT_name {{.*}} string = "ClosedEnum"
; CHECK-DAG: DW_AT_APPLE_enum_kind [DW_FORM_data1] (DW_APPLE_ENUM_KIND_Closed)
diff --git a/llvm/test/DebugInfo/AMDGPU/variable-locations.ll b/llvm/test/DebugInfo/AMDGPU/variable-locations.ll
index b795ad1b61a9c..ce69180e01539 100644
--- a/llvm/test/DebugInfo/AMDGPU/variable-locations.ll
+++ b/llvm/test/DebugInfo/AMDGPU/variable-locations.ll
@@ -13,22 +13,7 @@
declare void @llvm.dbg.declare(metadata, metadata, metadata)
-; CHECK: {{.*}}DW_TAG_variable
-; CHECK-NEXT: DW_AT_name {{.*}}"GlobA"
-; CHECK-NEXT: DW_AT_type
-; CHECK-NEXT: DW_AT_external
-; CHECK-NEXT: DW_AT_decl_file
-; CHECK-NEXT: DW_AT_decl_line
-; CHECK-NEXT: DW_AT_location [DW_FORM_block1] (DW_OP_addr 0x0)
@GlobA = common addrspace(1) global i32 0, align 4, !dbg !0
-
-; CHECK: {{.*}}DW_TAG_variable
-; CHECK-NEXT: DW_AT_name {{.*}}"GlobB"
-; CHECK-NEXT: DW_AT_type
-; CHECK-NEXT: DW_AT_external
-; CHECK-NEXT: DW_AT_decl_file
-; CHECK-NEXT: DW_AT_decl_line
-; CHECK-NEXT: DW_AT_location [DW_FORM_block1] (DW_OP_addr 0x0)
@GlobB = common addrspace(1) global i32 0, align 4, !dbg !6
; CHECK: {{.*}}DW_TAG_subprogram
@@ -78,12 +63,28 @@ entry:
!llvm.ident = !{!12}
!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+
+; CHECK: {{.*}}DW_TAG_variable
+; CHECK-NEXT: DW_AT_name {{.*}}"GlobA"
+; CHECK-NEXT: DW_AT_type
+; CHECK-NEXT: DW_AT_external
+; CHECK-NEXT: DW_AT_decl_file
+; CHECK-NEXT: DW_AT_decl_line
+; CHECK-NEXT: DW_AT_location [DW_FORM_block1] (DW_OP_addr 0x0)
!1 = distinct !DIGlobalVariable(name: "GlobA", scope: !2, file: !3, line: 1, type: !8, isLocal: false, isDefinition: true)
!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 5.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
!3 = !DIFile(filename: "variable-locations.cl", directory: "/some/random/directory")
!4 = !{}
!5 = !{!0, !6}
!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+
+; CHECK: {{.*}}DW_TAG_variable
+; CHECK-NEXT: DW_AT_name {{.*}}"GlobB"
+; CHECK-NEXT: DW_AT_type
+; CHECK-NEXT: DW_AT_external
+; CHECK-NEXT: DW_AT_decl_file
+; CHECK-NEXT: DW_AT_decl_line
+; CHECK-NEXT: DW_AT_location [DW_FORM_block1] (DW_OP_addr 0x0)
!7 = distinct !DIGlobalVariable(name: "GlobB", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!9 = !{i32 2, i32 0}
diff --git a/llvm/test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll b/llvm/test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll
index 450e38c72ed2e..c9abc29b4bd6d 100644
--- a/llvm/test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll
+++ b/llvm/test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll
@@ -33,9 +33,12 @@
; CHECK: DW_TAG_compile_unit
; CHECK-NOT: DW_TAG
; CHECK: DW_AT_name ("a.cpp")
+; CHECK: DW_TAG_subprogram
+; CHECK-NOT: DW_TAG
; CHECK: DW_AT_name ("func")
source_filename = "test/DebugInfo/Generic/cross-cu-linkonce-distinct.ll"
+target triple = "x86_64-unknown-linux-gnu"
@x = global ptr @_Z4funci, align 8, !dbg !0
@y = global ptr @_Z4funci, align 8, !dbg !7
diff --git a/llvm/test/DebugInfo/Generic/debug-names-linkage-name.ll b/llvm/test/DebugInfo/Generic/debug-names-linkage-name.ll
index 9fa73935cacd5..5f901bf9dba95 100644
--- a/llvm/test/DebugInfo/Generic/debug-names-linkage-name.ll
+++ b/llvm/test/DebugInfo/Generic/debug-names-linkage-name.ll
@@ -11,9 +11,9 @@
; We should have all three linkage names in the .debug_info and .debug_names
; ALL: .debug_info contents:
-; ALL: DW_AT_linkage_name ("_ZN1n1vE")
; ALL: DW_AT_linkage_name ("_Z1fi")
; ALL: DW_AT_linkage_name ("_Z1gi")
+; ALL: DW_AT_linkage_name ("_ZN1n1vE")
; ALL: .debug_names contents:
; ALL: String: {{.*}} "_Z1fi"
; ALL: String: {{.*}} "_Z1gi"
diff --git a/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll b/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll
index 3bbc79bdf03c1..b52d5cc1f18d8 100644
--- a/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll
+++ b/llvm/test/DebugInfo/Generic/dwarf5-debug-info-static-member.ll
@@ -88,8 +88,6 @@ attributes #1 = { nounwind readnone }
!35 = !DILocation(line: 13, column: 3, scope: !26)
; CHECK: .debug_info contents:
-; CHECK: DW_TAG_variable
-; CHECK-NEXT: DW_AT_specification {{.*}} "a"
; CHECK: DW_TAG_structure_type
; CHECK: DW_AT_name {{.*}} "C"
; CHECK: DW_TAG_variable
@@ -105,6 +103,10 @@ attributes #1 = { nounwind readnone }
; CHECK: DW_AT_external
; CHECK: DW_AT_declaration
; CHECK: NULL
+; CHECK: DW_TAG_subprogram
+; CHECK: NULL
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_specification {{.*}} "a"
; CHECK: DW_TAG_variable
; CHECK-NEXT: DW_AT_specification {{.*}} "b"
; CHECK-NEXT: DW_AT_const_value
diff --git a/llvm/test/DebugInfo/Generic/inlined-static-var.ll b/llvm/test/DebugInfo/Generic/inlined-static-var.ll
index 1d24646896d80..60c986fc2b648 100644
--- a/llvm/test/DebugInfo/Generic/inlined-static-var.ll
+++ b/llvm/test/DebugInfo/Generic/inlined-static-var.ll
@@ -14,18 +14,17 @@
; CHECK: DW_TAG_compile_unit
; CHECK: DW_TAG_subprogram
; CHECK: DW_AT_abstract_origin {{.*}} "_Z11not_removedv"
-; TODO: This variable should be emitted in abstract subprogram DIE.
-; CHECK: DW_TAG_variable
-; CHECK: DW_AT_name ("B")
-; CHECK: NULL
-; CHECK: DW_TAG_base_type
; CHECK: DW_TAG_subprogram
; CHECK: DW_AT_name ("removed")
; CHECK: DW_TAG_variable
; CHECK: DW_AT_name ("A")
; CHECK: NULL
+; CHECK: DW_TAG_base_type
; CHECK: DW_TAG_subprogram
; CHECK: DW_AT_name ("not_removed")
+; CHECK: DW_TAG_variable
+; CHECK: DW_AT_name ("B")
+; CHECK: NULL
; CHECK: DW_TAG_subprogram
; CHECK: DW_AT_name ("foo")
; CHECK: DW_TAG_inlined_subroutine
diff --git a/llvm/test/DebugInfo/Generic/namespace.ll b/llvm/test/DebugInfo/Generic/namespace.ll
index da6b156e186c2..1d49ea4b8a000 100644
--- a/llvm/test/DebugInfo/Generic/namespace.ll
+++ b/llvm/test/DebugInfo/Generic/namespace.ll
@@ -13,11 +13,6 @@
; CHECK-NOT: DW_AT_decl_file
; CHECK-NOT: DW_AT_decl_line
-; CHECK: [[I:0x[0-9a-f]*]]:{{ *}}DW_TAG_variable
-; CHECK: DW_AT_name ("i")
-; CHECK: [[VAR_FWD:0x[0-9a-f]*]]:{{ *}}DW_TAG_variable
-; CHECK: DW_AT_name ("var_fwd")
-
; CHECK: [[FOO:0x[0-9a-f]*]]:{{ *}}DW_TAG_structure_type
; CHECK: DW_AT_name ("foo")
; CHECK: DW_AT_declaration
@@ -37,6 +32,11 @@
; CHECK: DW_AT_name ("func_fwd")
; CHECK-NOT: DW_AT_declaration
+; CHECK: [[I:0x[0-9a-f]*]]:{{ *}}DW_TAG_variable
+; CHECK: DW_AT_name ("i")
+; CHECK: [[VAR_FWD:0x[0-9a-f]*]]:{{ *}}DW_TAG_variable
+; CHECK: DW_AT_name ("var_fwd")
+
; CHECK: [[BAZ:0x[0-9a-f]*]]:{{.*}}DW_TAG_typedef
; CHECK: DW_AT_name ("baz")
@@ -56,7 +56,6 @@
; CHECK: DW_TAG_imported_declaration
; CHECK: NULL
-; CHECK: DW_TAG_base_type
; CHECK: DW_TAG_subprogram
; CHECK: DW_TAG_subprogram
@@ -122,6 +121,7 @@
; CHECK: NULL
; CHECK: DW_TAG_subprogram
+; CHECK: DW_TAG_base_type
; CHECK: DW_TAG_imported_module
; CHECK: DW_AT_decl_file ([[F2:.*]])
; CHECK: DW_AT_decl_line (18)
diff --git a/llvm/test/DebugInfo/MSP430/global-var.ll b/llvm/test/DebugInfo/MSP430/global-var.ll
index 1941b33846dc1..d09dfd6d46379 100644
--- a/llvm/test/DebugInfo/MSP430/global-var.ll
+++ b/llvm/test/DebugInfo/MSP430/global-var.ll
@@ -4,7 +4,7 @@
; CHECK: DW_TAG_variable
; CHECK-NEXT: DW_AT_name ("global_var")
-; CHECK-NEXT: DW_AT_type ({{0x[0-9]+}} "char")
+; CHECK-NEXT: DW_AT_type ({{0x[0-9a-f]+}} "char")
; CHECK-NEXT: DW_AT_external (true)
; CHECK-NEXT: DW_AT_decl_file ("/tmp{{[/\\]}}global-var.c")
; CHECK-NEXT: DW_AT_decl_line (1)
diff --git a/llvm/test/DebugInfo/NVPTX/debug-addr-class.ll b/llvm/test/DebugInfo/NVPTX/debug-addr-class.ll
index 8e3e81bcf1517..19f76b941cadd 100644
--- a/llvm/test/DebugInfo/NVPTX/debug-addr-class.ll
+++ b/llvm/test/DebugInfo/NVPTX/debug-addr-class.ll
@@ -100,36 +100,6 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata)
; CHECK-NEXT: .b8 0 // EOM(1)
; CHECK-NEXT: .b8 0 // EOM(2)
; CHECK-NEXT: .b8 2 // Abbreviation Code
-; CHECK-NEXT: .b8 52 // DW_TAG_variable
-; CHECK-NEXT: .b8 0 // DW_CHILDREN_no
-; CHECK-NEXT: .b8 3 // DW_AT_name
-; CHECK-NEXT: .b8 8 // DW_FORM_string
-; CHECK-NEXT: .b8 73 // DW_AT_type
-; CHECK-NEXT: .b8 19 // DW_FORM_ref4
-; CHECK-NEXT: .b8 63 // DW_AT_external
-; CHECK-NEXT: .b8 12 // DW_FORM_flag
-; CHECK-NEXT: .b8 58 // DW_AT_decl_file
-; CHECK-NEXT: .b8 11 // DW_FORM_data1
-; CHECK-NEXT: .b8 59 // DW_AT_decl_line
-; CHECK-NEXT: .b8 11 // DW_FORM_data1
-; CHECK-NEXT: .b8 51 // DW_AT_address_class
-; CHECK-NEXT: .b8 11 // DW_FORM_data1
-; CHECK-NEXT: .b8 2 // DW_AT_location
-; CHECK-NEXT: .b8 10 // DW_FORM_block1
-; CHECK-NEXT: .b8 0 // EOM(1)
-; CHECK-NEXT: .b8 0 // EOM(2)
-; CHECK-NEXT: .b8 3 // Abbreviation Code
-; CHECK-NEXT: .b8 36 // DW_TAG_base_type
-; CHECK-NEXT: .b8 0 // DW_CHILDREN_no
-; CHECK-NEXT: .b8 3 // DW_AT_name
-; CHECK-NEXT: .b8 8 // DW_FORM_string
-; CHECK-NEXT: .b8 62 // DW_AT_encoding
-; CHECK-NEXT: .b8 11 // DW_FORM_data1
-; CHECK-NEXT: .b8 11 // DW_AT_byte_size
-; CHECK-NEXT: .b8 11 // DW_FORM_data1
-; CHECK-NEXT: .b8 0 // EOM(1)
-; CHECK-NEXT: .b8 0 // EOM(2)
-; CHECK-NEXT: .b8 4 // Abbreviation Code
; CHECK-NEXT: .b8 46 // DW_TAG_subprogram
; CHECK-NEXT: .b8 1 // DW_CHILDREN_yes
; CHECK-NEXT: .b8 17 // DW_AT_low_pc
@@ -151,7 +121,7 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata)
; CHECK-NEXT: .b8 12 // DW_FORM_flag
; CHECK-NEXT: .b8 0 // EOM(1)
; CHECK-NEXT: .b8 0 // EOM(2)
-; CHECK-NEXT: .b8 5 // Abbreviation Code
+; CHECK-NEXT: .b8 3 // Abbreviation Code
; CHECK-NEXT: .b8 5 // DW_TAG_formal_parameter
; CHECK-NEXT: .b8 0 // DW_CHILDREN_no
; CHECK-NEXT: .b8 51 // DW_AT_address_class
@@ -168,6 +138,36 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata)
; CHECK-NEXT: .b8 19 // DW_FORM_ref4
; CHECK-NEXT: .b8 0 // EOM(1)
; CHECK-NEXT: .b8 0 ...
[truncated]
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
| ; CHECK: [2] DW_TAG_enumeration_type DW_CHILDREN_yes | ||
| ; CHECK: DW_AT_APPLE_enum_kind DW_FORM_data1 | ||
|
|
||
| ; CHECK: .debug_info contents: | ||
|
|
||
| ; CHECK: DW_TAG_enumeration_type [3] | ||
| ; CHECK: DW_TAG_enumeration_type [2] | ||
| ; CHECK-DAG: DW_AT_name {{.*}} string = "OpenEnum" | ||
| ; CHECK-DAG: DW_AT_APPLE_enum_kind [DW_FORM_data1] (DW_APPLE_ENUM_KIND_Open) | ||
|
|
||
| ; CHECK: DW_TAG_enumeration_type [3] | ||
| ; CHECK: DW_TAG_enumeration_type [2] | ||
| ; CHECK-DAG: DW_AT_name {{.*}} string = "ClosedEnum" |
There was a problem hiding this comment.
Perhaps this could be improved to avoid hardcoding the abbreviation number with a match reference instead? Or maybe the abbreviation doesn't need to be tested at all - might just be an old test from back when the dump format was different. (could check the history)
(though I realize this change churns a lot of tests and improving those tests might not be worth it... )
There was a problem hiding this comment.
No, this was added last year. I think the form used to encode DW_AT_APPLE_enum_kind is checked intentionally.
Added FileCheck variable for abbreviation number.
| DwarfCompileUnit *CU = CUMap.lookup(CUNode); | ||
|
|
||
| // If the CU hasn't been emitted yet, create it here unless it is empty. | ||
| if (!CU) { | ||
| if (CUNode->getImportedEntities().empty() && | ||
| CUNode->getEnumTypes().empty() && | ||
| CUNode->getRetainedTypes().empty() && | ||
| CUNode->getGlobalVariables().empty() && CUNode->getMacros().empty()) | ||
| continue; | ||
| CU = &getOrCreateDwarfCompileUnit(CUNode); | ||
| } |
There was a problem hiding this comment.
Wouldn't it have already been created under the right conditions in the previous loop? (so we could do the lookup, then if !CU continue, without checking the empties again?)
There was a problem hiding this comment.
Ah, I agree that we can skip duplicate emptiness check. However, CUMap.lookup() only is not enough here to support split dwarf mode.
I've separated DwarfDebug::getDwarfCompileUnit() from DwarfDebug::getOrCreateDwarfCompileUnit(), used it here, and removed the check for the empties.
dwblaikie
left a comment
There was a problem hiding this comment.
Got any size comparison? Might be useful just as a heuristic that this shouldn't've changed anything (I wouldn't expect the sizes to be bit-identical, reshuffling things may change ULEB encoding lengths, etc, but shuold be really close, I think?)?
|
Built clang on Linux in RelWithDebInfo mode. Comparison results for bin/clang and bin/llc Also, built clang for macOS in RelWithDebInfo, and got more interesting result. Whereas the total size of debug info of clang binary has increased: After looking at some object files where the increase is the highest, I think that we get this increase because:
For example, before the patch, in the file whereas after the patch, they are emitted at higher indices: Grep over debug_info section dump tells that "operator++" is used 300+ times in this file. So, with the patch, subprogram DIEs for operator++ need at least 300 bytes more only for "operator++"s in debug_info section: To check this reasoning, I tried to build clang with I think this measurement supports my assumption about why we get debug_info size increase for clang binary. But currently I don't have specific ideas how to improve the situation, as DwarfStringPool doesn't allow to directly control which indices in debug_str_offsets strings are assigned with. |
dwblaikie
left a comment
There was a problem hiding this comment.
Thanks for the size analysis - that seems within the noise and the investigation seems to point to understandable changes and not some accidental increase in debug info emission (or omission/loss) due to this change.
|
Thank you! |
Target triple line was added to this test in https://reviews.llvm.org/D144007, but it's needless.
…t.ll Fixes buildbot failures after llvm#184219. Target triple line was added to this test in https://reviews.llvm.org/D144007, but it's needless.
…t.ll (#184685) Fixes buildbot failures after #184219. Target triple line was added to this test in https://reviews.llvm.org/D144007, but it's needless.
…o-static-member.ll Fixes failure on clang-ppc64le-rhel buildbot (https://lab.llvm.org/buildbot/#/builders/145/builds/13080) after llvm#184219. On ppc64le, children are not produced for DW_TAG_subprogram "main" in this test. Therefore, dwarfdump doesn't print NULL after this tag. On other platforms (AArch64/X86), DW_TAG_subprogram has DW_TAG_variable "instance_C", which should not be matched by the check lines. Loosened the check lines (turned CHECK-NEXT into CHECK) to make them work for all mentioned platforms.
…o-static-member.ll (#184694) Fixes failure on clang-ppc64le-rhel buildbot (https://lab.llvm.org/buildbot/#/builders/145/builds/13080) after #184219. On ppc64le, children are not produced for DW_TAG_subprogram "main" in this test. Therefore, dwarfdump doesn't print NULL after this tag. On other platforms (AArch64/X86), DW_TAG_subprogram has DW_TAG_variable "instance_C", which should not be matched by the check lines. Loosened the check lines (turned CHECK-NEXT into CHECK) to make them work for all mentioned platforms.
…o endModule() (5/7) (llvm#184219) 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 This patch moves the emission of global variables from `DwarfDebug::beginModule()` to `DwarfDebug::endModule()`. It has the following effects: 1. The order of debug entities in the resulting DWARF changes. 2. Currently, if a DISubprogram requires emission of both concrete out-of-line and inlined subprogram DIEs, and such a subprogram contains a static local variable, the DIE for the variable is emitted into the concrete out-of-line subprogram DIE. As a result, the variable is not available in debugger when breaking at the inlined function instance. It happens because static locals are emitted in `DwarfDebug::beginModule()`, but abstract DIEs for functions that are not completely inlined away are created only later during `DwarfDebug::endFunctionImpl()` calls. With this patch, DIEs for static local variables of subprograms that have both inlined and the concrete out-of-line instances are placed into abstract subprogram DIEs. They become visible in debugger when breaking at concrete out-of-line and inlined function instances. `llvm/test/DebugInfo/Generic/inlined-static-var.ll` illustrates that. 3. It will allow to simplify abstract subprogram DIEs creation by reverting llvm#159104 later. This is needed to simplify DWARF emission in a context of proper support of function-local static variables which comes in the next patch (https://reviews.llvm.org/D144008), making all function-local entities handled in `DwarfDebug::endModuleImpl()`. Authored-by: Kristina Bessonova <[email protected]> Co-authored-by: David Blaikie <[email protected]> Co-authored-by: Vladislav Dzhidzhoev <[email protected]>
…t.ll (llvm#184685) Fixes buildbot failures after llvm#184219. Target triple line was added to this test in https://reviews.llvm.org/D144007, but it's needless.
…o-static-member.ll (llvm#184694) Fixes failure on clang-ppc64le-rhel buildbot (https://lab.llvm.org/buildbot/#/builders/145/builds/13080) after llvm#184219. On ppc64le, children are not produced for DW_TAG_subprogram "main" in this test. Therefore, dwarfdump doesn't print NULL after this tag. On other platforms (AArch64/X86), DW_TAG_subprogram has DW_TAG_variable "instance_C", which should not be matched by the check lines. Loosened the check lines (turned CHECK-NEXT into CHECK) to make them work for all mentioned platforms.
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
This patch moves the emission of global variables from
DwarfDebug::beginModule()toDwarfDebug::endModule().It has the following effects:
The order of debug entities in the resulting DWARF changes.
Currently, if a DISubprogram requires emission of both concrete out-of-line and inlined subprogram DIEs, and such a subprogram contains a static local variable, the DIE for the variable is emitted into the concrete out-of-line subprogram DIE. As a result, the variable is not available in debugger when breaking at the inlined function instance.
It happens because static locals are emitted in
DwarfDebug::beginModule(), but abstract DIEs for functions that are not completely inlined away are created only later duringDwarfDebug::endFunctionImpl()calls.With this patch, DIEs for static local variables of subprograms that have both inlined and the concrete out-of-line instances are placed into abstract subprogram DIEs. They become visible in debugger when breaking at concrete out-of-line and inlined function instances.
llvm/test/DebugInfo/Generic/inlined-static-var.llillustrates that.It will allow to simplify abstract subprogram DIEs creation by reverting [DebugInfo][DwarfDebug] Separate creation and population of abstract subprogram DIEs #159104 later.
This is needed to simplify DWARF emission in a context of proper support of function-local static variables which comes in the next patch (https://reviews.llvm.org/D144008), making all function-local entities handled in
DwarfDebug::endModuleImpl().This change has already been reviewed in https://reviews.llvm.org/D144007, but I’m opening a new PR because it has been a while since it was approved (the process of the patchset merge was stuck on the previous patch).
Authored-by: Kristina Bessonova [email protected]