Skip to content

Conversation

@dzhidzhoev
Copy link
Member

@dzhidzhoev dzhidzhoev commented Oct 24, 2025

This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix.

The last merge attempt was #75385.
The issue with it was investigated in #75385 (comment).
The problem happens when

  1. Several modules are being linked.
  2. There are several DISubprograms that initially belong to different modules but represent the same source code function (for example, a function included from the same source code file).
  3. Some of such DISubprograms survive IR linking. It may happen if one of them is inlined somewhere or if the functions that have these DISubprograms attached have internal linkage.
  4. Each of these DISubprograms has a local type that corresponds to the same source code type. These types are initially from different modules, but have the same ODR identifier.

If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists.
Further compilation of such modules causes crashes.

To tackle that,

  • previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function),
  • for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

With this approach, clang builds without crashes in FullLTO (which is not the case for #75385).

Additionally:

  • a check is added to Verifier to report about local types located in a wrong retainedNodes list,
  • DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date.

Commit #75385 and the new changes are separate commits to simplify review process.

@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2025

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang-static-analyzer-1
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix.

The last merge attempt was #75385.
The issue with it was investigated in #75385 (comment).
If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists.
Further compilation of such modules causes crashes.

To tackle that,

  • previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function),
  • for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

With this approach, clang builds without crashes in FullLTO (which is not the case for #75385).

Additionally:

  • a check is added to Verifier to report about local types located in a wrong retainedNodes list,
  • DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date.

Commit #75385 and the new changes are separate commits to simplify review process.


Patch is 112.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165032.diff

40 Files Affected:

  • (modified) clang/test/Analysis/analyzeOneFunction.cpp (+2-2)
  • (modified) clang/test/DebugInfo/CXX/access.cpp (+1-1)
  • (modified) clang/test/DebugInfo/CXX/anon-union-vars.cpp (+6-6)
  • (modified) clang/test/DebugInfo/CXX/codeview-unnamed.cpp (+62-48)
  • (modified) clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp (+2-2)
  • (modified) clang/test/DebugInfo/CXX/lambda-capture-packs.cpp (+7-8)
  • (modified) clang/test/DebugInfo/CXX/lambda-this.cpp (+2-2)
  • (modified) clang/test/DebugInfo/Generic/codeview-unnamed.c (+8-8)
  • (modified) clang/test/DebugInfo/Generic/unused-types.c (+9-7)
  • (modified) clang/test/DebugInfo/Generic/unused-types.cpp (+8-6)
  • (modified) llvm/include/llvm/AsmParser/LLParser.h (+4)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-3)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+15)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+7)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+65-33)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (+42-22)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (+8-8)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+10-3)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+48-18)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+2)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+39)
  • (modified) llvm/lib/IR/Verifier.cpp (+13-3)
  • (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+15-2)
  • (added) llvm/test/Bitcode/local-type-scope.ll (+48)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll (+41-27)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll.bc ()
  • (modified) llvm/test/CodeGen/ARM/call-graph-section-assembly.ll (+2-2)
  • (modified) llvm/test/CodeGen/X86/call-graph-section-assembly.ll (+2-2)
  • (added) llvm/test/DebugInfo/Generic/inlined-local-type.ll (+128)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-retained-types.ll (+55)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-types.ll (+425)
  • (modified) llvm/test/DebugInfo/Generic/verifier-invalid-disubprogram.ll (+1-1)
  • (added) llvm/test/DebugInfo/Inputs/cleanup-retained-nodes.ll (+17)
  • (added) llvm/test/DebugInfo/X86/cleanup-retained-nodes.ll (+46)
  • (added) llvm/test/DebugInfo/X86/llparser-cleanup-retained-nodes.ll (+37)
  • (added) llvm/test/DebugInfo/X86/local-type-as-template-parameter.ll (+161)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import2.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import3.ll ()
  • (modified) llvm/unittests/Transforms/Utils/CloningTest.cpp (+105)
diff --git a/clang/test/Analysis/analyzeOneFunction.cpp b/clang/test/Analysis/analyzeOneFunction.cpp
index 3a362dfd9a08c..e4c96eb35f06b 100644
--- a/clang/test/Analysis/analyzeOneFunction.cpp
+++ b/clang/test/Analysis/analyzeOneFunction.cpp
@@ -5,9 +5,9 @@
 // RUN:   -analyze-function="c:@S@Window@F@overloaded#I#"
 
 // RUN: %clang_extdef_map %s | FileCheck %s
-// CHECK:      27:c:@S@Window@F@overloaded#I#
+// CHECK:      27:c:@S@Window@F@overloaded#d#
 // CHECK-NEXT: 27:c:@S@Window@F@overloaded#C#
-// CHECK-NEXT: 27:c:@S@Window@F@overloaded#d#
+// CHECK-NEXT: 27:c:@S@Window@F@overloaded#I#
 
 void clang_analyzer_warnIfReached();
 
diff --git a/clang/test/DebugInfo/CXX/access.cpp b/clang/test/DebugInfo/CXX/access.cpp
index 9f2c044843d0f..7c0bf8ccb0384 100644
--- a/clang/test/DebugInfo/CXX/access.cpp
+++ b/clang/test/DebugInfo/CXX/access.cpp
@@ -18,9 +18,9 @@ class B : public A {
   static int public_static;
 
 protected:
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+3]],{{.*}} flags: DIFlagProtected)
   // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_typedef",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   typedef int prot_typedef;
-  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   using prot_using = prot_typedef;
   prot_using prot_member;
 
diff --git a/clang/test/DebugInfo/CXX/anon-union-vars.cpp b/clang/test/DebugInfo/CXX/anon-union-vars.cpp
index 3aca4e199ab8d..27e6c6850e013 100644
--- a/clang/test/DebugInfo/CXX/anon-union-vars.cpp
+++ b/clang/test/DebugInfo/CXX/anon-union-vars.cpp
@@ -51,13 +51,13 @@ void instantiate(int x) {
 // CHECK: !DIGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "result", {{.*}} isLocal: false, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "value", {{.*}} isLocal: false, isDefinition: true
-// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(
-// CHECK-NOT: name:
-// CHECK: type: ![[UNION:[0-9]+]]
-// CHECK: ![[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type,
+// CHECK: ![[UNION:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_union_type,
 // CHECK-NOT: name:
 // CHECK: elements
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]],
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]],
+// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(
+// CHECK-NOT: name:
+// CHECK: type: ![[UNION]]
diff --git a/clang/test/DebugInfo/CXX/codeview-unnamed.cpp b/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
index 30815bda020ea..b625a80313c38 100644
--- a/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
+++ b/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
@@ -4,6 +4,60 @@
 
 int main(int argc, char* argv[], char* arge[]) {
   //
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-one>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-two>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-SAME:     name: "named"
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "named"
+  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      )
+
+  //
+  // LINUX:      [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_class_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_class_type
+  // MSVC-SAME:      name: "<lambda_0>"
+  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      )
+
+
   // In CodeView, the LF_MFUNCTION entry for "bar()" refers to the forward
   // reference of the unnamed struct. Visual Studio requires a unique
   // identifier to match the LF_STRUCTURE forward reference to the definition.
@@ -11,21 +65,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct { void bar() {} } one;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-one>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
   // MSVC-SAME:      )
 
 
@@ -37,21 +81,11 @@ int main(int argc, char* argv[], char* arge[]) {
   int decltype(two)::*ptr2unnamed = &decltype(two)::bar;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // LINUX-SAME:     type: [[TYPE_OF_TWO:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_TWO]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_TWO]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // MSVC-SAME:      type: [[TYPE_OF_TWO:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_TWO]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-two>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_TWO]]
   // MSVC-SAME:      )
 
 
@@ -62,21 +96,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct named { int bar; int named::* p2mem; } three = { 42, &named::bar };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // LINUX-SAME:     type: [[TYPE_OF_THREE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_THREE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-SAME:     name: "named"
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_THREE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // MSVC-SAME:      type: [[TYPE_OF_THREE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_THREE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "named"
-  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_THREE]]
   // MSVC-SAME:      )
 
 
@@ -88,21 +112,11 @@ int main(int argc, char* argv[], char* arge[]) {
   auto four = [argc](int i) -> int { return argc == i ? 1 : 0; };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // LINUX-SAME:     type: [[TYPE_OF_FOUR:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_FOUR]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_class_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_FOUR]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // MSVC-SAME:      type: [[TYPE_OF_FOUR:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_FOUR]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_class_type
-  // MSVC-SAME:      name: "<lambda_0>"
-  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_FOUR]]
   // MSVC-SAME:      )
 
   return 0;
diff --git a/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp b/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
index 6b9c9a243decd..122e4aa62ea7d 100644
--- a/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
+++ b/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
@@ -51,9 +51,9 @@ void test() {
   // CHECK-SAME:                              name: "<lambda_2_1>",
   c.lambda_params();
 
-  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1:[0-9]+]],
-  // CHECK: ![[LAMBDA1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK: ![[LAMBDA1:[0-9]+]] = !DICompositeType(tag: DW_TAG_class_type,
   // CHECK-SAME:                            name: "<lambda_1>",
   // CHECK-SAME:                            flags: DIFlagFwdDecl
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1]],
   c.lambda2();
 }
diff --git a/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp b/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
index 021b85d4ce3a4..609a71c6ec015 100644
--- a/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
+++ b/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
@@ -2,14 +2,6 @@
 // RUN:   -debug-info-kind=standalone -std=c++26 %s -o - | FileCheck %s
 
 
-// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
-// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
-// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
-// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
-// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
-// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
-// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
-
 template<typename... Args>
 auto capture_pack(Args... args) {
   return [args..., ...params = args] {
@@ -17,6 +9,7 @@ auto capture_pack(Args... args) {
   }();
 }
 
+// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK1]]
 // CHECK-SAME:                           elements: ![[PACK1_ELEMS:[0-9]+]]
 // CHECK-DAG:  ![[PACK1_ELEMS]] = !{![[PACK1_ARGS:[0-9]+]], ![[PACK1_PARAMS:[0-9]+]]}
@@ -24,6 +17,7 @@ auto capture_pack(Args... args) {
 // CHECK-DAG:  ![[PACK1_PARAMS]] = !DIDerivedType(tag: DW_TAG_member, name: "params"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK2]]
 // CHECK-SAME:                           elements: ![[PACK2_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK2_ELEMS]] = !{![[PACK2_ARGS:[0-9]+]]
@@ -42,6 +36,7 @@ auto capture_pack_and_locals(int x, Args... args) {
   }();
 }
 
+// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK3]]
 // CHECK-SAME:                           elements: ![[PACK3_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK3_ELEMS]] = !{![[PACK3_ARGS:[0-9]+]]
@@ -55,6 +50,7 @@ auto capture_pack_and_locals(int x, Args... args) {
 // CHECK-DAG:  ![[PACK3_W]] = !DIDerivedType(tag: DW_TAG_member, name: "w"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK4]]
 // CHECK-SAME:                           elements: ![[PACK4_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK4_ELEMS]] = !{![[PACK4_ARGS:[0-9]+]]
@@ -90,6 +86,7 @@ struct Foo {
   int w = 10;
 } f;
 
+// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK5]]
 // CHECK-SAME:                           elements: ![[PACK5a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK5a_ELEMS]] = !{![[PACK5a_THIS:[0-9]+]]
@@ -120,6 +117,7 @@ struct Foo {
 // CHECK-DAG:  ![[PACK5c_THIS]] = !DIDerivedType(tag: DW_TAG_member, name: "this"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK6]]
 // CHECK-SAME:                           elements: ![[PACK6a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK6a_ELEMS]] = !{![[PACK6a_THIS:[0-9]+]]
@@ -168,6 +166,7 @@ auto capture_binding_and_param_pack(Args... args) {
   }();
 }
 
+// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
 // CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK7]]
 // CHECK-SAME:                           elements: ![[PACK7_ELEMS:[0-9]+]]
diff --git a/clang/test/DebugInfo/CXX/lambda-this.cpp b/clang/test/DebugInfo/CXX/lambda-this.cpp
index 019d09c48f858..1df210953ac2e 100644
--- a/clang/test/DebugInfo/CXX/lambda-this.cpp
+++ b/clang/test/DebugInfo/CXX/lambda-this.cpp
@@ -13,10 +13,10 @@ void D::d(int x) {
 }
 
 // CHECK: ![[D:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "D",
-// CHECK: ![[POINTER:.*]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "this",
 // CHECK-SAME:           line: 11
-// CHECK-SAME:           baseType: ![[POINTER]]
+// CHECK-SAME:           baseType: ![[POINTER:[0-9]+]]
 // CHECK-SAME:           size: 64
 // CHECK-NOT:            offset: 0
 // CHECK-SAME:           ){{$}}
+// CHECK: ![[POINTER]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
diff --git a/clang/test/DebugInfo/Generic/codeview-unnamed.c b/clang/test/DebugInfo/Generic/codeview-unnamed.c
index 0df6e1a0419bb..7b88f53a92e42 100644
--- a/clang/test/DebugInfo/Generic/codeview-unnamed.c
+++ b/clang/test/DebugInfo/Generic/codeview-unnamed.c
@@ -8,23 +8,23 @@ int main(int argc, char* argv[], char* arge[]) {
   //
   struct { int bar; } one = {42};
   //
-  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
   // LINUX-SAME:     tag: DW_TAG_structure_type
   // LINUX-NOT:      name:
   // LINUX-NOT:      identifier:
   // LINUX-SAME:     )
+  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
+  // LINUX-SAME:     )
   //
-  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
   // MSVC-SAME:      tag: DW_TAG_structure_type
   // MSVC-NOT:       name:
   // MSVC-NOT:       identifier:
   // MSVC-SAME:      )
+  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
+  // MSVC-SAME:      )
 
   return 0;
 }
diff --git a/clang/test/DebugInfo/Generic/unused-types.c b/clang/test/DebugInfo/Generic/unused-types.c
index 3e9f7b07658e3..31d608d92a06b 100644
--- a/clang/test/DebugInfo/Generic/unused-types.c
+++ b/clang/test/DebugInfo/Generic/unused-types.c
@@ -18,13 +18,15 @@ void quux(void) {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAR"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], [[TYPE6:![0-9]+]], {{![0-9]+}}, [[TYPE7:![0-9]+]], [[TYPE2]], [[TYPE8:![0-9]+]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
-// CHECK: [[TYPE7]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], [[TYPE4:![0-9]+]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE5:![0-9]+]], [[TYPE6:![0-9]+]], [[TYPE8:![0-9]+]]}
+// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[TYPE6]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: [[TYPE7:![0-9]+]] = !DIEnumerator(name: "Z"
 // CHECK: [[TYPE8]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "w"
 
 // Check that debug info is not emitted for the typedef, struct, enum, and
diff --git a/clang/test/DebugInfo/Generic/unused-types.cpp b/clang/test/DebugInfo/Generic/unused-types.cpp
index 023cac159faa4..5b01c6dbb3941 100644
--- a/clang/test/DebugInfo/Generic/unused-types.cpp
+++ b/clang/test/DebugInfo/Generic/unused-types.cpp
@@ -13,12 +13,14 @@ void quux() {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAZ"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], {{![0-9]+}}, [[TYPE6:![0-9]+]], [[TYPE2]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]]}
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y", scope: [[SP]]
+// CHECK: [[TYPE5]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: [[SP]]
+// CHECK: [[TYPE6:![0-9]+]] = !DIEnumerator(name: "Z"
 
 // NODBG-NOT: !DI{{CompositeType|Enumerator|DerivedType}}
 
diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h
index 9eb31d7e0a451..ae568f8ead039 100644
--- a/llvm/include/llvm/AsmParser/LLParser.h
+++ b/llvm/include/llvm/AsmParser/LLParser.h
@@ -181,6 +181,10 @@ namespace llvm {
     /// Keeps track of so...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2025

@llvm/pr-subscribers-backend-arm

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix.

The last merge attempt was #75385.
The issue with it was investigated in #75385 (comment).
If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists.
Further compilation of such modules causes crashes.

To tackle that,

  • previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function),
  • for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

With this approach, clang builds without crashes in FullLTO (which is not the case for #75385).

Additionally:

  • a check is added to Verifier to report about local types located in a wrong retainedNodes list,
  • DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date.

Commit #75385 and the new changes are separate commits to simplify review process.


Patch is 112.61 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165032.diff

40 Files Affected:

  • (modified) clang/test/Analysis/analyzeOneFunction.cpp (+2-2)
  • (modified) clang/test/DebugInfo/CXX/access.cpp (+1-1)
  • (modified) clang/test/DebugInfo/CXX/anon-union-vars.cpp (+6-6)
  • (modified) clang/test/DebugInfo/CXX/codeview-unnamed.cpp (+62-48)
  • (modified) clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp (+2-2)
  • (modified) clang/test/DebugInfo/CXX/lambda-capture-packs.cpp (+7-8)
  • (modified) clang/test/DebugInfo/CXX/lambda-this.cpp (+2-2)
  • (modified) clang/test/DebugInfo/Generic/codeview-unnamed.c (+8-8)
  • (modified) clang/test/DebugInfo/Generic/unused-types.c (+9-7)
  • (modified) clang/test/DebugInfo/Generic/unused-types.cpp (+8-6)
  • (modified) llvm/include/llvm/AsmParser/LLParser.h (+4)
  • (modified) llvm/include/llvm/IR/DIBuilder.h (+3-3)
  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+15)
  • (modified) llvm/lib/AsmParser/LLParser.cpp (+7)
  • (modified) llvm/lib/Bitcode/Reader/MetadataLoader.cpp (+65-33)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (+42-22)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h (+8-8)
  • (modified) llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (+10-3)
  • (modified) llvm/lib/IR/DIBuilder.cpp (+48-18)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+2)
  • (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+39)
  • (modified) llvm/lib/IR/Verifier.cpp (+13-3)
  • (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+15-2)
  • (added) llvm/test/Bitcode/local-type-scope.ll (+48)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll (+41-27)
  • (modified) llvm/test/Bitcode/upgrade-cu-locals.ll.bc ()
  • (modified) llvm/test/CodeGen/ARM/call-graph-section-assembly.ll (+2-2)
  • (modified) llvm/test/CodeGen/X86/call-graph-section-assembly.ll (+2-2)
  • (added) llvm/test/DebugInfo/Generic/inlined-local-type.ll (+128)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-retained-types.ll (+55)
  • (added) llvm/test/DebugInfo/Generic/lexical-block-types.ll (+425)
  • (modified) llvm/test/DebugInfo/Generic/verifier-invalid-disubprogram.ll (+1-1)
  • (added) llvm/test/DebugInfo/Inputs/cleanup-retained-nodes.ll (+17)
  • (added) llvm/test/DebugInfo/X86/cleanup-retained-nodes.ll (+46)
  • (added) llvm/test/DebugInfo/X86/llparser-cleanup-retained-nodes.ll (+37)
  • (added) llvm/test/DebugInfo/X86/local-type-as-template-parameter.ll (+161)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import2.ll (-1)
  • (renamed) llvm/test/DebugInfo/X86/split-dwarf-local-import3.ll ()
  • (modified) llvm/unittests/Transforms/Utils/CloningTest.cpp (+105)
diff --git a/clang/test/Analysis/analyzeOneFunction.cpp b/clang/test/Analysis/analyzeOneFunction.cpp
index 3a362dfd9a08c..e4c96eb35f06b 100644
--- a/clang/test/Analysis/analyzeOneFunction.cpp
+++ b/clang/test/Analysis/analyzeOneFunction.cpp
@@ -5,9 +5,9 @@
 // RUN:   -analyze-function="c:@S@Window@F@overloaded#I#"
 
 // RUN: %clang_extdef_map %s | FileCheck %s
-// CHECK:      27:c:@S@Window@F@overloaded#I#
+// CHECK:      27:c:@S@Window@F@overloaded#d#
 // CHECK-NEXT: 27:c:@S@Window@F@overloaded#C#
-// CHECK-NEXT: 27:c:@S@Window@F@overloaded#d#
+// CHECK-NEXT: 27:c:@S@Window@F@overloaded#I#
 
 void clang_analyzer_warnIfReached();
 
diff --git a/clang/test/DebugInfo/CXX/access.cpp b/clang/test/DebugInfo/CXX/access.cpp
index 9f2c044843d0f..7c0bf8ccb0384 100644
--- a/clang/test/DebugInfo/CXX/access.cpp
+++ b/clang/test/DebugInfo/CXX/access.cpp
@@ -18,9 +18,9 @@ class B : public A {
   static int public_static;
 
 protected:
+  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+3]],{{.*}} flags: DIFlagProtected)
   // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_typedef",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   typedef int prot_typedef;
-  // CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+1]],{{.*}} flags: DIFlagProtected)
   using prot_using = prot_typedef;
   prot_using prot_member;
 
diff --git a/clang/test/DebugInfo/CXX/anon-union-vars.cpp b/clang/test/DebugInfo/CXX/anon-union-vars.cpp
index 3aca4e199ab8d..27e6c6850e013 100644
--- a/clang/test/DebugInfo/CXX/anon-union-vars.cpp
+++ b/clang/test/DebugInfo/CXX/anon-union-vars.cpp
@@ -51,13 +51,13 @@ void instantiate(int x) {
 // CHECK: !DIGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "result", {{.*}} isLocal: false, isDefinition: true
 // CHECK: !DIGlobalVariable(name: "value", {{.*}} isLocal: false, isDefinition: true
-// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
-// CHECK: !DILocalVariable(
-// CHECK-NOT: name:
-// CHECK: type: ![[UNION:[0-9]+]]
-// CHECK: ![[UNION]] = distinct !DICompositeType(tag: DW_TAG_union_type,
+// CHECK: ![[UNION:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_union_type,
 // CHECK-NOT: name:
 // CHECK: elements
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]],
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]],
+// CHECK: !DILocalVariable(name: "i", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
+// CHECK: !DILocalVariable(
+// CHECK-NOT: name:
+// CHECK: type: ![[UNION]]
diff --git a/clang/test/DebugInfo/CXX/codeview-unnamed.cpp b/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
index 30815bda020ea..b625a80313c38 100644
--- a/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
+++ b/clang/test/DebugInfo/CXX/codeview-unnamed.cpp
@@ -4,6 +4,60 @@
 
 int main(int argc, char* argv[], char* arge[]) {
   //
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-one>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_TWO:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "<unnamed-type-two>"
+  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      )
+
+
+  //
+  // LINUX:      [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_structure_type
+  // LINUX-SAME:     name: "named"
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_THREE:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_structure_type
+  // MSVC-SAME:      name: "named"
+  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      )
+
+  //
+  // LINUX:      [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType(
+  // LINUX-SAME:     tag: DW_TAG_class_type
+  // LINUX-NOT:      name:
+  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     )
+  //
+  // MSVC:       [[TYPE_OF_FOUR:![0-9]+]] = distinct !DICompositeType
+  // MSVC-SAME:      tag: DW_TAG_class_type
+  // MSVC-SAME:      name: "<lambda_0>"
+  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      )
+
+
   // In CodeView, the LF_MFUNCTION entry for "bar()" refers to the forward
   // reference of the unnamed struct. Visual Studio requires a unique
   // identifier to match the LF_STRUCTURE forward reference to the definition.
@@ -11,21 +65,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct { void bar() {} } one;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-one>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-one>@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
   // MSVC-SAME:      )
 
 
@@ -37,21 +81,11 @@ int main(int argc, char* argv[], char* arge[]) {
   int decltype(two)::*ptr2unnamed = &decltype(two)::bar;
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // LINUX-SAME:     type: [[TYPE_OF_TWO:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_TWO]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_TWO]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "two"
-  // MSVC-SAME:      type: [[TYPE_OF_TWO:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_TWO]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "<unnamed-type-two>"
-  // MSVC-SAME:      identifier: ".?AU<unnamed-type-two>@?2??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_TWO]]
   // MSVC-SAME:      )
 
 
@@ -62,21 +96,11 @@ int main(int argc, char* argv[], char* arge[]) {
   struct named { int bar; int named::* p2mem; } three = { 42, &named::bar };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // LINUX-SAME:     type: [[TYPE_OF_THREE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_THREE]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_structure_type
-  // LINUX-SAME:     name: "named"
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_THREE]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "three"
-  // MSVC-SAME:      type: [[TYPE_OF_THREE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_THREE]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_structure_type
-  // MSVC-SAME:      name: "named"
-  // MSVC-SAME:      identifier: ".?AUnamed@?1??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_THREE]]
   // MSVC-SAME:      )
 
 
@@ -88,21 +112,11 @@ int main(int argc, char* argv[], char* arge[]) {
   auto four = [argc](int i) -> int { return argc == i ? 1 : 0; };
   //
   // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // LINUX-SAME:     type: [[TYPE_OF_FOUR:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_FOUR]] = distinct !DICompositeType(
-  // LINUX-SAME:     tag: DW_TAG_class_type
-  // LINUX-NOT:      name:
-  // LINUX-NOT:      identifier:
+  // LINUX-SAME:     type: [[TYPE_OF_FOUR]]
   // LINUX-SAME:     )
   //
   // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "four"
-  // MSVC-SAME:      type: [[TYPE_OF_FOUR:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_FOUR]] = distinct !DICompositeType
-  // MSVC-SAME:      tag: DW_TAG_class_type
-  // MSVC-SAME:      name: "<lambda_0>"
-  // MSVC-SAME:      identifier: ".?AV<lambda_0>@?0??main@@9@"
+  // MSVC-SAME:      type: [[TYPE_OF_FOUR]]
   // MSVC-SAME:      )
 
   return 0;
diff --git a/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp b/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
index 6b9c9a243decd..122e4aa62ea7d 100644
--- a/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
+++ b/clang/test/DebugInfo/CXX/gline-tables-only-codeview.cpp
@@ -51,9 +51,9 @@ void test() {
   // CHECK-SAME:                              name: "<lambda_2_1>",
   c.lambda_params();
 
-  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1:[0-9]+]],
-  // CHECK: ![[LAMBDA1]] = !DICompositeType(tag: DW_TAG_class_type,
+  // CHECK: ![[LAMBDA1:[0-9]+]] = !DICompositeType(tag: DW_TAG_class_type,
   // CHECK-SAME:                            name: "<lambda_1>",
   // CHECK-SAME:                            flags: DIFlagFwdDecl
+  // CHECK: !DISubprogram(name: "operator()", scope: ![[LAMBDA1]],
   c.lambda2();
 }
diff --git a/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp b/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
index 021b85d4ce3a4..609a71c6ec015 100644
--- a/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
+++ b/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
@@ -2,14 +2,6 @@
 // RUN:   -debug-info-kind=standalone -std=c++26 %s -o - | FileCheck %s
 
 
-// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
-// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
-// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
-// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
-// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
-// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
-// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
-
 template<typename... Args>
 auto capture_pack(Args... args) {
   return [args..., ...params = args] {
@@ -17,6 +9,7 @@ auto capture_pack(Args... args) {
   }();
 }
 
+// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK1]]
 // CHECK-SAME:                           elements: ![[PACK1_ELEMS:[0-9]+]]
 // CHECK-DAG:  ![[PACK1_ELEMS]] = !{![[PACK1_ARGS:[0-9]+]], ![[PACK1_PARAMS:[0-9]+]]}
@@ -24,6 +17,7 @@ auto capture_pack(Args... args) {
 // CHECK-DAG:  ![[PACK1_PARAMS]] = !DIDerivedType(tag: DW_TAG_member, name: "params"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK2]]
 // CHECK-SAME:                           elements: ![[PACK2_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK2_ELEMS]] = !{![[PACK2_ARGS:[0-9]+]]
@@ -42,6 +36,7 @@ auto capture_pack_and_locals(int x, Args... args) {
   }();
 }
 
+// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK3]]
 // CHECK-SAME:                           elements: ![[PACK3_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK3_ELEMS]] = !{![[PACK3_ARGS:[0-9]+]]
@@ -55,6 +50,7 @@ auto capture_pack_and_locals(int x, Args... args) {
 // CHECK-DAG:  ![[PACK3_W]] = !DIDerivedType(tag: DW_TAG_member, name: "w"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK4]]
 // CHECK-SAME:                           elements: ![[PACK4_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK4_ELEMS]] = !{![[PACK4_ARGS:[0-9]+]]
@@ -90,6 +86,7 @@ struct Foo {
   int w = 10;
 } f;
 
+// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK5]]
 // CHECK-SAME:                           elements: ![[PACK5a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK5a_ELEMS]] = !{![[PACK5a_THIS:[0-9]+]]
@@ -120,6 +117,7 @@ struct Foo {
 // CHECK-DAG:  ![[PACK5c_THIS]] = !DIDerivedType(tag: DW_TAG_member, name: "this"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK6]]
 // CHECK-SAME:                           elements: ![[PACK6a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK6a_ELEMS]] = !{![[PACK6a_THIS:[0-9]+]]
@@ -168,6 +166,7 @@ auto capture_binding_and_param_pack(Args... args) {
   }();
 }
 
+// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
 // CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK7]]
 // CHECK-SAME:                           elements: ![[PACK7_ELEMS:[0-9]+]]
diff --git a/clang/test/DebugInfo/CXX/lambda-this.cpp b/clang/test/DebugInfo/CXX/lambda-this.cpp
index 019d09c48f858..1df210953ac2e 100644
--- a/clang/test/DebugInfo/CXX/lambda-this.cpp
+++ b/clang/test/DebugInfo/CXX/lambda-this.cpp
@@ -13,10 +13,10 @@ void D::d(int x) {
 }
 
 // CHECK: ![[D:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "D",
-// CHECK: ![[POINTER:.*]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "this",
 // CHECK-SAME:           line: 11
-// CHECK-SAME:           baseType: ![[POINTER]]
+// CHECK-SAME:           baseType: ![[POINTER:[0-9]+]]
 // CHECK-SAME:           size: 64
 // CHECK-NOT:            offset: 0
 // CHECK-SAME:           ){{$}}
+// CHECK: ![[POINTER]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[D]], size: 64)
diff --git a/clang/test/DebugInfo/Generic/codeview-unnamed.c b/clang/test/DebugInfo/Generic/codeview-unnamed.c
index 0df6e1a0419bb..7b88f53a92e42 100644
--- a/clang/test/DebugInfo/Generic/codeview-unnamed.c
+++ b/clang/test/DebugInfo/Generic/codeview-unnamed.c
@@ -8,23 +8,23 @@ int main(int argc, char* argv[], char* arge[]) {
   //
   struct { int bar; } one = {42};
   //
-  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // LINUX-SAME:     type: [[TYPE_OF_ONE:![0-9]+]]
-  // LINUX-SAME:     )
-  // LINUX:      [[TYPE_OF_ONE]] = distinct !DICompositeType(
+  // LINUX:      [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType(
   // LINUX-SAME:     tag: DW_TAG_structure_type
   // LINUX-NOT:      name:
   // LINUX-NOT:      identifier:
   // LINUX-SAME:     )
+  // LINUX:      !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // LINUX-SAME:     type: [[TYPE_OF_ONE]]
+  // LINUX-SAME:     )
   //
-  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
-  // MSVC-SAME:      type: [[TYPE_OF_ONE:![0-9]+]]
-  // MSVC-SAME:      )
-  // MSVC:       [[TYPE_OF_ONE]] = distinct !DICompositeType
+  // MSVC:       [[TYPE_OF_ONE:![0-9]+]] = distinct !DICompositeType
   // MSVC-SAME:      tag: DW_TAG_structure_type
   // MSVC-NOT:       name:
   // MSVC-NOT:       identifier:
   // MSVC-SAME:      )
+  // MSVC:       !{{[0-9]+}} = !DILocalVariable(name: "one"
+  // MSVC-SAME:      type: [[TYPE_OF_ONE]]
+  // MSVC-SAME:      )
 
   return 0;
 }
diff --git a/clang/test/DebugInfo/Generic/unused-types.c b/clang/test/DebugInfo/Generic/unused-types.c
index 3e9f7b07658e3..31d608d92a06b 100644
--- a/clang/test/DebugInfo/Generic/unused-types.c
+++ b/clang/test/DebugInfo/Generic/unused-types.c
@@ -18,13 +18,15 @@ void quux(void) {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAR"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], [[TYPE6:![0-9]+]], {{![0-9]+}}, [[TYPE7:![0-9]+]], [[TYPE2]], [[TYPE8:![0-9]+]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
-// CHECK: [[TYPE7]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], [[TYPE4:![0-9]+]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE5:![0-9]+]], [[TYPE6:![0-9]+]], [[TYPE8:![0-9]+]]}
+// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y"
+// CHECK: [[TYPE6]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: [[TYPE7:![0-9]+]] = !DIEnumerator(name: "Z"
 // CHECK: [[TYPE8]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: "w"
 
 // Check that debug info is not emitted for the typedef, struct, enum, and
diff --git a/clang/test/DebugInfo/Generic/unused-types.cpp b/clang/test/DebugInfo/Generic/unused-types.cpp
index 023cac159faa4..5b01c6dbb3941 100644
--- a/clang/test/DebugInfo/Generic/unused-types.cpp
+++ b/clang/test/DebugInfo/Generic/unused-types.cpp
@@ -13,12 +13,14 @@ void quux() {
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAZ"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], {{![0-9]+}}, [[TYPE6:![0-9]+]], [[TYPE2]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]]}
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y", scope: [[SP]]
+// CHECK: [[TYPE5]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: [[SP]]
+// CHECK: [[TYPE6:![0-9]+]] = !DIEnumerator(name: "Z"
 
 // NODBG-NOT: !DI{{CompositeType|Enumerator|DerivedType}}
 
diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h
index 9eb31d7e0a451..ae568f8ead039 100644
--- a/llvm/include/llvm/AsmParser/LLParser.h
+++ b/llvm/include/llvm/AsmParser/LLParser.h
@@ -181,6 +181,10 @@ namespace llvm {
     /// Keeps track of so...
[truncated]

@github-actions
Copy link

github-actions bot commented Oct 24, 2025

✅ With the latest revision this PR passed the undef deprecator.

@dzhidzhoev
Copy link
Member Author

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
The following files introduce new uses of undef:

  • llvm/test/DebugInfo/X86/split-dwarf-local-import.ll

Seems to be a false alarm, triggered due to the file being moved.

@dzhidzhoev dzhidzhoev force-pushed the debuginfo/rfc-krisb/4/base branch 2 times, most recently from bfee696 to e97af0a Compare October 27, 2025 12:18
@dzhidzhoev
Copy link
Member Author

Fixed failing checks.

@dzhidzhoev
Copy link
Member Author

Gentle ping

dzhidzhoev added a commit to dzhidzhoev/llvm-project that referenced this pull request Nov 6, 2025
These checks ensure that retained nodes of a DISubprogram belong
to the subprogram.

This was helpful to find issues with https://reviews.llvm.org/D144004,
llvm#75385,
llvm#165032.

Also, interface for accessing DISubprogram's retained nodes is slightly
refactored. `DISubprogram::visitRetainedNodes` and
`DISubprogram::forEachRetainedNode` are added to avoid repeating
checks like
```
if (const auto *LV = dyn_cast<DILocalVariable>(N))
  ...
else if (const auto *L = dyn_cast<DILabel>(N))
  ...
else if (const auto *IE = dyn_cast<DIImportedEntity>(N))
  ...
```

Tests with wrongly scoped retained nodes are fixed.
@dzhidzhoev dzhidzhoev force-pushed the debuginfo/rfc-krisb/4/base branch from e97af0a to 99884a4 Compare November 11, 2025 15:27
@github-actions
Copy link

github-actions bot commented Nov 11, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@dzhidzhoev dzhidzhoev force-pushed the debuginfo/rfc-krisb/4/base branch from 99884a4 to e10adfd Compare November 11, 2025 16:19
@dwblaikie
Copy link
Collaborator

Could you update the description to include an explanation of why the same uniqued type might end up in two distinct DISubprograms under LTO? I guess/assume/vaguely recall it's probably due to internalizing those functions? (like they're inline functions and in one module one version gets internalized (so it can be specialized there - change the parameters, etc - say it has a dead/unused parameter, so it gets internalized and the parameter is removed) and in another module it gets internalized for similar reasons (maybe different enough they don't then get merged together because they don't end up identical))

for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

So we'd end up with the two distinct versions of the DISubprogram, each emitted into their own CUs, but one will reference the type in another? Seems sort of fair/doesn't seem fundamentally wrong...

a check is added to Verifier to report about local types located in a wrong retainedNodes list,

Ah, OK, because now we won't do that - a local variable in one DISubprogram might reference a local type in another DISubprogram - but the type will only exist/be retained in one DISubprogram, the one it references as its own scope. Good good. Awkward, but good.

@dzhidzhoev
Copy link
Member Author

for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

So we'd end up with the two distinct versions of the DISubprogram, each emitted into their own CUs, but one will reference the type in another? Seems sort of fair/doesn't seem fundamentally wrong...

a check is added to Verifier to report about local types located in a wrong retainedNodes list,

Ah, OK, because now we won't do that - a local variable in one DISubprogram might reference a local type in another DISubprogram - but the type will only exist/be retained in one DISubprogram, the one it references as its own scope. Good good. Awkward, but good.

Yes, that's what this fix is trying to achieve.

Could you update the description to include an explanation of why the same uniqued type might end up in two distinct DISubprograms under LTO? I guess/assume/vaguely recall it's probably due to internalizing those functions? (like they're inline functions and in one module one version gets internalized (so it can be specialized there - change the parameters, etc - say it has a dead/unused parameter, so it gets internalized and the parameter is removed) and in another module it gets internalized for similar reasons (maybe different enough they don't then get merged together because they don't end up identical))

The problem happens when

  1. Several modules are being linked.
  2. There are several DISubprograms that initially belong to different modules but represent the same source code function (for example, a function included from the same source code file).
  3. Some of such DISubprograms survive IR linking. It may happen if one of them is inlined somewhere or if the functions that have these DISubprograms attached have internal linkage.
  4. Each of these DISubprograms has a local type that corresponds to the same source code type. These types are initially from different modules, but have the same ODR identifier.

Added that to the PR description.

@dzhidzhoev
Copy link
Member Author

like they're inline functions and in one module one version gets internalized (so it can be specialized there - change the parameters, etc - say it has a dead/unused parameter, so it gets internalized and the parameter is removed) and in another module it gets internalized for similar reasons (maybe different enough they don't then get merged together because they don't end up identical))

We only merge declaration DISubprograms. Definition DISubprograms are not merged together in general, if I get it right.

@dwblaikie
Copy link
Collaborator

for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

So we'd end up with the two distinct versions of the DISubprogram, each emitted into their own CUs, but one will reference the type in another? Seems sort of fair/doesn't seem fundamentally wrong...

a check is added to Verifier to report about local types located in a wrong retainedNodes list,

Ah, OK, because now we won't do that - a local variable in one DISubprogram might reference a local type in another DISubprogram - but the type will only exist/be retained in one DISubprogram, the one it references as its own scope. Good good. Awkward, but good.

Yes, that's what this fix is trying to achieve.

As always, I really appreciate your patience with this challenging project - I'm sorry but I do find myself going around in circles (& consequently possibly steering you around in circles) as we try to address one problem and then the next.

It's really challenging to try to keep the whole history of different directions attempted, why they didn't work, etc...

But I guess this "we specialized the function and now we have two versions of the same function" is always going to be weird. It's a bit unfortunate if we don't keep a copy of the type in each copy of the function (name lookup will be wrong (naming the type in the function copy that doesn't keep the type definition won't find it - or worse, could find some outer-scope entity by the same name) - and name lookup is one of the reasons to do this work in the first place - to get local types and static variables in the right scopes), but perhaps that's an acceptable tradeoff?

@adrian-prantl @JDevlieghere @jmorse - you folks have any thoughts on this work? It's been/continues to be quite a slog with some awkward tradeoffs along the way, I think.

@jmorse
Copy link
Member

jmorse commented Nov 13, 2025

I'll probably only get around to reading the code tomorrow or Friday; my perspective though is that all the difficulty is coming from the ORDUniquing facility, and the fact that it's installed at a lower level within LLVM than the debug-info metadata itself. Hence, types get de-duplicated in unexpected ways that can't be controlled, and we have to work around it in numerous places.

I haven't put any though into whether we can instead get rid of / adjust ORDUniquing to not bite us here. It's been my assumption that people use the facility, and thus it's probably necessary for peoples builds.

In terms of how useful these changes are, there are a fair few instances in game codebases where function-local types appear, and our debugger team have raised this as a limitation in LLVM in the past. It's certainly something we'd like to be fixed; I don't think anyone anticipated the level of effort involved to achieve that though (thank you @dzhidzhoev !).

@dzhidzhoev
Copy link
Member Author

for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links.

So we'd end up with the two distinct versions of the DISubprogram, each emitted into their own CUs, but one will reference the type in another? Seems sort of fair/doesn't seem fundamentally wrong...

a check is added to Verifier to report about local types located in a wrong retainedNodes list,

Ah, OK, because now we won't do that - a local variable in one DISubprogram might reference a local type in another DISubprogram - but the type will only exist/be retained in one DISubprogram, the one it references as its own scope. Good good. Awkward, but good.

Yes, that's what this fix is trying to achieve.

As always, I really appreciate your patience with this challenging project - I'm sorry but I do find myself going around in circles (& consequently possibly steering you around in circles) as we try to address one problem and then the next.

It's really challenging to try to keep the whole history of different directions attempted, why they didn't work, etc...

Yes, I understand, I'm also getting lost in that sometimes. I think it was also due to my lack of understanding how dwarf emission actually works, which contributed to making incomplete/incorrect fixes to the original patch.
I've written a message to the original RFC with the history of attempts, and the problem I've encountered. Hope it makes navigation easier.

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

But I guess this "we specialized the function and now we have two versions of the same function" is always going to be weird. It's a bit unfortunate if we don't keep a copy of the type in each copy of the function (name lookup will be wrong (naming the type in the function copy that doesn't keep the type definition won't find it - or worse, could find some outer-scope entity by the same name) - and name lookup is one of the reasons to do this work in the first place - to get local types and static variables in the right scopes), but perhaps that's an acceptable tradeoff?

At least, we should get more precise information in non-LTO/non-heavily-inlined-code case.

Offtopic: I think it's an interesting idea to try to deduplicate inlined-out DISubprograms in a merged module after LTO. I'm checking my implementation of that, but I think it will be hard to implement it at the level of MetadataLoader, where debug type ODR uniquing happens, since we don't have full information about LLVM IR module inside it. I'm leaning towards making a separate pass for it, which will be closer to machine code emission, to leverage the fact that some extra inlining may happen during LTO after all modules are loaded and optimizations are done.

@dzhidzhoev
Copy link
Member Author

I haven't put any though into whether we can instead get rid of / adjust ORDUniquing to not bite us here. It's been my assumption that people use the facility, and thus it's probably necessary for peoples builds.

AFAIK, ODRUniquing's goal is to reduce code size/link time for LTO builds. To my understanding, it should not affect correctness if we have several local types with the same identifier (or if we identifiers for some of them during module loading). But it seems to me, as for now, that merging semantically identical but distinct DISubprograms, or emitting one abstract DW_TAG_subprogram for subprograms with the same linkageName inlined in different modules by any other way, is better approach. However, as it mostly affects LTO, it looks like an LTO-related task that should be separate from the task of improving local entities scoping in general case.

}

/// Remove types that do not belong to the subprogram's scope from
/// retainedNodes list.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would help to comment here under what circumstances such a situation might arise.

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description of the the problem in the PR makes sense to me, but it would help future maintainers to also explain this better in the comments for the new APIs in the header files.

/// When DebugTypeODRUniquing is enabled, after multiple modules are loaded,
/// some subprograms (that are from different compilation units, usually)
/// may have references to the same local type in their retainedNodes lists.
///
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For someone reading the API documentation it would help to answer the following questions in this document:

  • why may two subprograms refer to the same local type?
  • why is that a problem?
  • what exactly does the "cleanup" look like?
  • does the cleanup degrade the debug info quality/accuracy in any way?

@dzhidzhoev
Copy link
Member Author

Thank you! Added more detailed comments.

@github-actions
Copy link

github-actions bot commented Nov 17, 2025

🐧 Linux x64 Test Results

  • 193630 tests passed
  • 6242 tests skipped

✅ The build succeeded and all tests passed.

jmorse and others added 4 commits November 19, 2025 17:27
…lock scopes (4/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

Similar to imported declarations, the patch tracks function-local types in
DISubprogram's 'retainedNodes' field. DwarfDebug is adjusted in accordance with
the aforementioned metadata change and provided a support of function-local
types scoped within a lexical block.

The patch assumes that DICompileUnit's 'enums field' no longer tracks local
types and DwarfDebug would assert if any locally-scoped types get placed there.

Reviewed By: jmmartinez
Authored-by: Kristina Bessonova <kbessonova@accesssoftek.com>
Differential Revision: https://reviews.llvm.org/D144006

(cherry picked from commit 2953d4a)
Signed-off-by: Jeremy Morse <jeremy.morse@sony.com>

Conflicts:
	llvm/lib/IR/DIBuilder.cpp
	llvm/lib/IR/DebugInfo.cpp
        llvm/lib/Transforms/Utils/CloneFunction.cpp
…exical block scopes (4/7)"

This is an attempt to merge https://reviews.llvm.org/D144006 with LTO
fix.

The last merge attempt was llvm#75385.
The issue with it was investigated in
llvm#75385 (comment).
If the same (in the sense of ODR identifier/ODR uniquing rules) local type
is present in two modules, and these modules are linked together, the type
gets uniqued. A DIType, that happens to be loaded first, survives linking,
and the references on other types with the same ODR identifier from the modules
loaded later are replaced with the references on the DIType loaded first.
Since defintion subprograms, in scope of which these types are located,
are not deduplicated, the linker output may contain multiple
DISubprogram's having the same (uniqued) type in their retainedNodes
lists.
Further compilation of such modules causes crashes.

To tackle that,
* previous solution to handle LTO linking with local types in
retainedNodes is removed (cloneLocalTypes() function),
* for each loaded distinct (definition) DISubprogram, its retainedNodes
list is scanned after loading, and DITypes with a scope of another subprogram
are removed. If something from a Function corresponding to the
DISubprogram references uniqued type, we rely on cross-CU links.

With this approach, clang builds without crashes in FullLTO (which is not
the case for llvm#75385).

Additionally:
* a check is added to Verifier to report about local types located
in a wrong retainedNodes list,
* DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006
has gotten slightly out-of-date.

Commit llvm#75385 and the new changes
are placed in separate commits to simplify review process.
…e cloned

CGVTables: hide unresolved local types from ValueMapper
@dzhidzhoev dzhidzhoev force-pushed the debuginfo/rfc-krisb/4/base branch from 75ca316 to c028e0d Compare November 21, 2025 13:24
@dzhidzhoev
Copy link
Member Author

Fixed a couple of extra issues with this patch, which I've found:

One problem is related to function cloning.
Many derived types, such as pointer types or qualified types, do not have scope information, although they can refer to local types.
If a function is cloned in CloneFunction, the function's local types are also cloned, but derived types are not remapped to refer to cloned local types, since CloneFunction (IdentityMDPredicate used by it, to be precise) treats scopeless derived types as non-local and prevents ValueMapper from remapping them. This may lead to a situation where local types are cloned, but pointer types from the cloned function still refer to the local types from the original subprogram rather than those from the cloned subprogram.
To solve that, I've updated CloneFunction's IdentityMDPredicate so that it ignores scopeless derived types. Thus, ValueMapper will remap such types if they refer to (cloned) local types.

Another issue relates to CodeGenFunction::GenerateVarArgsThunk in clang (for which changes were recently made in PR #167758).
It calls CloneFunction for an IR that is not finalized. Particularly, some DIType nodes may still be temporary when CloneFunction is called. ValueMapper can't handle temporary nodes. On the mainline, it's not a problem, since type nodes are never remapped (ValueMapper is prevented from remapping them by IdentityMDPredicate). However, local types and derived types are cloned/remapped with this patch, and ValueMapper may encounter temporary nodes (examples are given in clang/test/CodeGenCXX/tmp-md-nodes3.cpp).
CodeGenFunction::GenerateVarArgsThunk tackles a similar problem with DILocalVariables by calling MDNode::resolve() on them. It does not actually resolve any dependencies on temporaries, but forcefully marks DILocalVariables as resolved. I doubt the same trick would be good for local types, as it might prevent them from being correctly finalized later.
To solve this, for each DILocalVariable encountered in a base function, its DIType is mapped to itself. Adding the DIType to VMap prevents ValueMapper from inspecting the node. This should not affect debug information produced by CodeGenFunction::GenerateVarArgsThunk, compared to the current mainline, where local types are not remapped at all.

@llvmbot llvmbot added the clang:codegen IR generation bugs: mangling, exceptions, etc. label Nov 21, 2025
@dzhidzhoev
Copy link
Member Author

I've tried to build Chromium on ThinLTO with this PR, and encountered another problem.

With the previous version of the patch, we trigger DISubprograms retainedNodes cleanup only when loading function-level metadata block or module-level metadata block. Under some conditions, DISubprograms that are loaded lazily can slip through ThinLTO function import process without being cleaned up (if lazy loading is not followed by parsing any function-level or module-level metadata blocks). This behavior can be reproduced with llvm/test/Transforms/FunctionImport/funcimport-debug-retained-nodes.ll.

I've made necessary changes to MetadataLoader that should guarantee that every DISubprogram returned to clients of the class is cleaned up.

With this change, I managed to build Chromium in regular and ThinLTO modes without crashes (it seems that Chromium doesn't provide out of the box configurations for FullLTO build, unfortunately).

@dzhidzhoev
Copy link
Member Author

Gentle ping

Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some shallow comments; I'm still getting my head around the major differences here and haven't looked at the tests, but see:

  • We're directly un-doing the effect of ODRUniqing by filtering for vulnerable retainedNodes lists on bitcode/IR load,
  • Types that have been ODRUnique'd get cross-scope and possibly cross-CU references to them,

Which seems like a proportionate solution: ORDUniquing is saving memory in this very direct and unstructured way, and we have to work around it. Which sucks, but it's the price of the design decision.

Is there any protection against a part of LLVM duplicating a type and silently getting an ODRUnique'd type back, which then leads to confusion later -- or is that what the createIdentityMDPredicate code is protecting against? I wonder whether this could lead to a situation where creating type metadata is vulnerable to accidents (however if it is, we should protect against it with lots of assertions, instead of trying to design around it).

The potential expansion of the types being cloned due to changing createIdentityMDPredicate might affect compile-time performance; we can look at that in due course though.

Thanks for sticking with this, it's been a rollercoaster, hopefully this will cover all the complications seen to date!

Comment on lines +129 to +130
const_iterator begin() { return MetadataPtrs.begin(); }
const_iterator end() { return MetadataPtrs.end(); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the methods const too?

Comment on lines +455 to +456
/// retainedNodes of these subprograms should be cleaned up from incorrectly
/// scoped local types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO to avoid confusing future readers, we need to signal something about the fact that it's LLVM/ORDUniquing that's causing things to be invalid, not the input. Possibly "See the comment on \ref DISubprogram::cleanupRetainedNodes"?

NewEnums.push_back(Op);

// Find DISubprogram corresponding to each entity.
std::map<DISubprogram *, SmallVector<Metadata *>> SPToEntities;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that std::map is usually discouraged due to allocating for each new element; I think we can get away with this as it's on the autoupgrade path, which isn't going to be fast anyway.

Comment on lines +558 to +560
SetVector<Metadata *> MetadataToRemove;
// Collect imported entities to be moved.
if (CU->getRawImportedEntities())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give an overview of the algorithm being used in this function, as I can't piece it together: we've got the collection of metadata-to-remove from the CU, and the NewImports NewEnums collections are the remainder of retainedNodes elements that won't be moved to DISubprograms? If so, I think the term "New" is giving me slight confusion, could we use the term "remaining" or "KeepInCU" or something like that?

Comment on lines +758 to +759
void resolveLoadedMetadata(PlaceholderQueue &Placeholders,
bool IsModuleLevelDIUpgrade) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see this overload is to assist two call-sites that already have a bool? IMO it would be better to sink the selection of DebugInfoUpgradeMode into :MetadataLoaderImpl::parseMetadata rather than rely on this overload, on the principle of "explicit is better than implicit".

Comment on lines +775 to +779
// FIXME: We may have a concrete DIE for this scope already created.
// This may happen when we emit local variables for an abstract tree of
// an inlined function: if a local variable has a templated type with
// a function-local type as a template parameter. See PR55680 for details
// (see also llvm/test/DebugInfo/Generic/local-type-as-template-parameter.ll).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear what's to be fixed here: the concrete DIE isn't supposed to exist already but we accept it temporarily, or we should intend on creating a duplicate, or placing it somewhere else?

Copy link
Collaborator

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few thoughts - thanks @jmorse for taking a look at this too! (it's been a long term project and I'm more distant from LLVM over the course of it & getting a bit change-blind/struggling to keep up with the state of things)

Comment on lines +138 to +139
// Some types may be still unresolved. As they can't be cloned, keep
// references to the types from the base subprogram.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"some" feels concerningly vague - could you be more specific?

Comment on lines +140 to +141
// FIXME: As a result, variables of cloned subprogram may refer to local types
// from base subprogram. In such case, type locality information is damaged.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXME is a bit unclear about what should be fixed, and explains more about the problem?

Perhaps the explanation should go above, and the FIXME should be more specific about what should be fixed?

Comment on lines +549 to +550
/// Move function-local enums from DICompileUnit's enums
/// to DISubprogram's retainedNodes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably go in a separate standalone patch?

@@ -703,13 +727,41 @@ class MetadataLoader::MetadataLoaderImpl {
return Error::success();
}

void upgradeDebugInfo(bool ModuleLevel) {
/// Specifies which kind of debug info upgrade should be performed.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there could be some more details in this comment about why there are different kinds of debug info upgrade?

Copy link
Member

@jmorse jmorse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at the tests; only thing I've missed now is the unittests (out of time today).

funcimport-debug-retained-nodes.ll is enjoyably thorough -- I wonder whether it's so specific that it'll easily lose coverage of what it's testing though. Sometimes we use -debug output to ensure we're taking the correct codepath, would it be feasible to check that to ensure we're hitting the lazy-metadata-loading path? (This is usually discouraged, but given how hard it is to debug this area, IMO it's worth doing to avoid future quiet loss of coverage)

Comment on lines +9 to +10
// This test checks that the varargs thunk is correctly created if types of
// DILocalVariables of the base function are unresolved at the cloning time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate on the test criteria -- as I understand it this test is to avoid a crash in a tricky situation where types have no full definition? If so, please say that explicitly, and what kind of DILocalVariable information should be produced (currently none / incorrect information).

(I'm thinking ahead to if/when this test fails and someone has to determine the intention behind it).

static int public_static;

protected:
// CHECK-DAG: !DIDerivedType(tag: DW_TAG_typedef, name: "prot_using",{{.*}} line: [[@LINE+3]],{{.*}} flags: DIFlagProtected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm interested in why this is failing / necessary -- the '-DAG' suffix should mean the check lines are re-orderable anyway, and as far as I read the diff you're only changing the CHECK-line position and updating the relative-line-num calculation accordingly?

// CHECK: !DILocalVariable(name: "c", {{.*}}, flags: DIFlagArtificial
// CHECK: !DILocalVariable(
// CHECK-NOT: name:
// CHECK: type: ![[UNION]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be CHECK-SAME, to ensure that we're on the same line as the nameless DILocalVariable? Otherwise this (while unlikely) could match an unrelated later type.

Comment on lines +1 to +3
; RUN: llvm-as --disable-verify < %s -o %t0
; RUN: opt --passes=verify %t0 -o /dev/null
; RUN: llvm-dis %t0 -o - | FileCheck %s --implicit-check-not=DICompositeType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good test; but isn't the retainedNodes field going to be normalised by the first llvm-as, meaning that opt and llvm-dis only see a normalised input? If so, perhaps a round-trip test is sufficient, and something else is needed to exercise the metadata-loader for bitcode?

; CHECK: distinct !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "type_global_in_another_module")
; CHECK: [[EMPTY:![0-9]+]] = !{}
; CHECK: [[BAR1:![0-9]+]] = distinct !DISubprogram(name: "bar", {{.*}}, retainedNodes: [[RN_BAR1:![0-9]+]])
; CHECK: [[RN_BAR1]] = !{[[T1:![0-9]+]], [[T1:![0-9]+]], [[T1:![0-9]+]], [[T2:![0-9]+]]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each of these T1 variables capture, can the second and third just be [[T1]]? The fact that the same node appears multiple times is presumably something intended to be checked, to ensure type_global_in_another_module doesn't appear here.

; CHECK: DW_TAG_subprogram
; CHECK: DW_AT_abstract_origin {{.*}} "_Z3foov"

; FIXME: 'struct B' should be in the abstract tree below, not here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could I request more indentation, or ';' characters at the start, to make this standout: otherwise it's the same width as CHECK: and could easily be mistaken for a check line.

; CHECK: DW_AT_abstract_origin {{.*}} "objA"
; CHECK: NULL

%struct.B = type { i32 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could be misreading, but shouldn't we have some type information for "A" above, or otherwise something for the objA variable to refer to?

Comment on lines +54 to +57
; CHECK: ![[EMPTY:[0-9]+]] = !{}
; CHECK: !DISubprogram(name: "inlined_out_clone", {{.*}}, retainedNodes: ![[EMPTY]]
; CHECK: !DICompositeType({{.*}}, identifier: "local_type"
; CHECK: !DISubprogram(name: "inlined_out_clone", {{.*}}, retainedNodes: ![[EMPTY]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't say I completely follow the premise but -- shouldn't we also check that the DICompositeType is in the correct scope?

Comment on lines +44 to +50
; RUN: opt --bitcode-mdindex-threshold=0 -module-summary %s -o %t.bc
; RUN: opt --bitcode-mdindex-threshold=0 -module-summary %p/Inputs/funcimport-debug-retained-nodes.ll -o %t2.bc
; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
; RUN: llvm-lto --thinlto-action=run %t.bc %t2.bc --thinlto-save-temps=%t3 2>&1 | FileCheck --allow-empty --check-prefix=LTO %s
; RUN: llvm-dis %t30.3.imported.bc -o - | FileCheck %s \
; RUN: --implicit-check-not='DISubprogram(name: "inlined_out_clone"' \
; RUN: --implicit-check-not='DICompositeType({{.*}}, identifier: "local_type"'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanation above; these RUN lines should be above it though to meet the LLVM convention.

Comment on lines +12 to +13
; Ensure that function-local types have the correct subprogram parent even if
; those subprograms are inlined.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we specify which is the correct subprogram parent for future readers; the abstract subprogram, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:ARM backend:X86 clang:codegen IR generation bugs: mangling, exceptions, etc. clang:static analyzer clang Clang issues not falling into any other category debuginfo llvm:codegen llvm:ir llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants