From 45c78f137fe886eb934dbb905236dc0b821730f0 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Tue, 15 Oct 2024 20:26:33 +0100 Subject: [PATCH 1/4] [flang][debug] Don't generate debug info for compiler-generated variables. Flang generates many globals to handle derived types. There was a check in debug info to filter them based on the information that their names start with '.'. This changed since PR#104859 where 'X' is being used instead of '.'. This PR fixes this issue by also adding 'X' in that list. As user variables gets lower cased by the NameUniquer, there is no risk that those will be filtered out. I added a test for that to be sure. --- .../lib/Optimizer/Transforms/AddDebugInfo.cpp | 11 +++++++---- flang/test/Integration/debug-extra-global.f90 | 14 ++++++++++++++ flang/test/Transforms/debug-derived-type-1.fir | 10 +++++----- flang/test/Transforms/debug-extra-global.fir | 18 ++++++++++++++++++ 4 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 flang/test/Integration/debug-extra-global.f90 create mode 100644 flang/test/Transforms/debug-extra-global.fir diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp index 400a8648dd7e0..f31c86cf67863 100644 --- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp +++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp @@ -211,10 +211,13 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp, if (result.first != fir::NameUniquer::NameKind::VARIABLE) return; - // Discard entries that describe a derived type. Usually start with '.c.', - // '.dt.' or '.n.'. It would be better if result of the deconstruct had a flag - // for such values so that we dont have to look at string values. - if (!result.second.name.empty() && result.second.name[0] == '.') + // Discard entries that describe a derived type. They start with either '.' or + // 'X'. We filter both of them out. Note that NameUniquer makes the name lower + // case so user variables should be safe. It would be better if result of the + // deconstruct had a flag for such values so that we dont have to look at + // string values. + if (!result.second.name.empty() && + (result.second.name[0] == '.' || result.second.name[0] == 'X')) return; unsigned line = getLineFromLoc(globalOp.getLoc()); diff --git a/flang/test/Integration/debug-extra-global.f90 b/flang/test/Integration/debug-extra-global.f90 new file mode 100644 index 0000000000000..c0ad2e306386d --- /dev/null +++ b/flang/test/Integration/debug-extra-global.f90 @@ -0,0 +1,14 @@ +! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s + +program test + type t1 + integer :: XcX + integer :: xdtx + end type + type(t1) :: var + var%XcX = 2 + var%xdtx = 3 +end + +! Test that there is no debug info for compiler generated globals. +! CHECK-NOT: DIGlobalVariable diff --git a/flang/test/Transforms/debug-derived-type-1.fir b/flang/test/Transforms/debug-derived-type-1.fir index cfbd361a91e72..b1c6076c9c212 100644 --- a/flang/test/Transforms/debug-derived-type-1.fir +++ b/flang/test/Transforms/debug-derived-type-1.fir @@ -8,15 +8,15 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry, d %0 = fir.zero_bits !fir.type<_QMm_employeeTt_employee{t_person:!fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}>,hired_date:!fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}>,monthly_salary:f32}> fir.has_value %0 : !fir.type<_QMm_employeeTt_employee{t_person:!fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}>,hired_date:!fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}>,monthly_salary:f32}> } loc(#loc5) - fir.global @_QMt1Evar : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> { - %0 = fir.zero_bits !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> - fir.has_value %0 : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> + fir.global @_QMt1Evar : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> { + %0 = fir.zero_bits !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> + fir.has_value %0 : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> } loc(#loc6) fir.global @_QMtest_1Exyz : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> { %0 = fir.zero_bits !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> fir.has_value %0 : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> } loc(#loc12) - fir.type_info @_QMt1Tt_t1 noinit nodestroy nofinal : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> loc(#loc7) + fir.type_info @_QMt1Tt_t1 noinit nodestroy nofinal : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> loc(#loc7) fir.type_info @_QMm_employeeTt_address noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_address{house_number:i32}> loc(#loc1) fir.type_info @_QMm_employeeTt_person noinit nodestroy nofinal extends !fir.type<_QMm_employeeTt_address{house_number:i32}> : !fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}> loc(#loc2) fir.type_info @_QMm_employeeTt_date noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}> loc(#loc3) @@ -68,7 +68,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry, d // CHECK-DAG: #[[ELME3:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[EMP:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_employee"{{.*}}line = 46, scope = #[[MOD]], sizeInBits = 320, elements = #[[ELME1]], #[[ELME2]], #[[ELME3]]> -// CHECK-DAG: #[[ELM1:.*]] = #llvm.di_derived_type +// CHECK-DAG: #[[ELM1:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM2:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM3:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM4:.*]] = #llvm.di_derived_type diff --git a/flang/test/Transforms/debug-extra-global.fir b/flang/test/Transforms/debug-extra-global.fir new file mode 100644 index 0000000000000..d3bc22ad0c59b --- /dev/null +++ b/flang/test/Transforms/debug-extra-global.fir @@ -0,0 +1,18 @@ +// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s + +module attributes {dlti.dl_spec = #dlti.dl_spec<>} { + fir.global linkonce_odr @_QFEXnXxcx constant target : !fir.char<1,3> { + %0 = fir.string_lit "xcx"(3) : !fir.char<1,3> + fir.has_value %0 : !fir.char<1,3> + } loc(#loc1) + fir.global linkonce_odr @_QFEXnXxdtx constant target : !fir.char<1,4> { + %0 = fir.string_lit "xdtx"(4) : !fir.char<1,4> + fir.has_value %0 : !fir.char<1,4> + } loc(#loc1) +} +#loc1 = loc("derived.f90":24:1) + +// Test that no di_global_variable gets created for these compile generated +// globals. + +// CHECK-NOT: #di_global_variable From 3217ca693aa752de81395ee5f09e1078971d6809 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Tue, 15 Oct 2024 21:30:46 +0100 Subject: [PATCH 2/4] Correct a problem in the test. Test now check a global variable while previously I checked a derived class member. --- flang/test/Integration/debug-extra-global-2.f90 | 8 ++++++++ flang/test/Transforms/debug-derived-type-1.fir | 10 +++++----- 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 flang/test/Integration/debug-extra-global-2.f90 diff --git a/flang/test/Integration/debug-extra-global-2.f90 b/flang/test/Integration/debug-extra-global-2.f90 new file mode 100644 index 0000000000000..20a3605e4f49a --- /dev/null +++ b/flang/test/Integration/debug-extra-global-2.f90 @@ -0,0 +1,8 @@ +! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s + +module m +integer XcX +end + + +CHECK: !DIGlobalVariable(name: "xcx", linkageName: "_QMmExcx"{{.*}}) diff --git a/flang/test/Transforms/debug-derived-type-1.fir b/flang/test/Transforms/debug-derived-type-1.fir index b1c6076c9c212..cfbd361a91e72 100644 --- a/flang/test/Transforms/debug-derived-type-1.fir +++ b/flang/test/Transforms/debug-derived-type-1.fir @@ -8,15 +8,15 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry, d %0 = fir.zero_bits !fir.type<_QMm_employeeTt_employee{t_person:!fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}>,hired_date:!fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}>,monthly_salary:f32}> fir.has_value %0 : !fir.type<_QMm_employeeTt_employee{t_person:!fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}>,hired_date:!fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}>,monthly_salary:f32}> } loc(#loc5) - fir.global @_QMt1Evar : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> { - %0 = fir.zero_bits !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> - fir.has_value %0 : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> + fir.global @_QMt1Evar : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> { + %0 = fir.zero_bits !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> + fir.has_value %0 : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> } loc(#loc6) fir.global @_QMtest_1Exyz : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> { %0 = fir.zero_bits !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> fir.has_value %0 : !fir.type<_QMtest_1Tsometype{m_array:!fir.array<3xi32>,m_vt1:!fir.box,height:f32}>>>,v2:i32,m_alloc:!fir.box>>,v3:i32,m_first:!fir.box>>,v4:i32,m_p1:!fir.box>,v5:i32,m_p2:!fir.box>,v6:i32,m_p3:!fir.box>>,v7:i32}> } loc(#loc12) - fir.type_info @_QMt1Tt_t1 noinit nodestroy nofinal : !fir.type<_QMt1Tt_t1{XcX:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> loc(#loc7) + fir.type_info @_QMt1Tt_t1 noinit nodestroy nofinal : !fir.type<_QMt1Tt_t1{age:i32,points:!fir.array<3xcomplex>,cond:!fir.logical<1>,name:!fir.char<1,20>,ratio:f64}> loc(#loc7) fir.type_info @_QMm_employeeTt_address noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_address{house_number:i32}> loc(#loc1) fir.type_info @_QMm_employeeTt_person noinit nodestroy nofinal extends !fir.type<_QMm_employeeTt_address{house_number:i32}> : !fir.type<_QMm_employeeTt_person{t_address:!fir.type<_QMm_employeeTt_address{house_number:i32}>,name:!fir.char<1,20>}> loc(#loc2) fir.type_info @_QMm_employeeTt_date noinit nodestroy nofinal : !fir.type<_QMm_employeeTt_date{year:i32,month:i32,day:i32}> loc(#loc3) @@ -68,7 +68,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry, d // CHECK-DAG: #[[ELME3:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[EMP:.*]] = #llvm.di_composite_type<{{.*}}tag = DW_TAG_structure_type, name = "t_employee"{{.*}}line = 46, scope = #[[MOD]], sizeInBits = 320, elements = #[[ELME1]], #[[ELME2]], #[[ELME3]]> -// CHECK-DAG: #[[ELM1:.*]] = #llvm.di_derived_type +// CHECK-DAG: #[[ELM1:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM2:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM3:.*]] = #llvm.di_derived_type // CHECK-DAG: #[[ELM4:.*]] = #llvm.di_derived_type From 3122f825bf8829fa8cefbefddaca2005b652d6bb Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Tue, 15 Oct 2024 21:33:05 +0100 Subject: [PATCH 3/4] Add a comment. --- flang/test/Integration/debug-extra-global-2.f90 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/test/Integration/debug-extra-global-2.f90 b/flang/test/Integration/debug-extra-global-2.f90 index 20a3605e4f49a..59135747f5f97 100644 --- a/flang/test/Integration/debug-extra-global-2.f90 +++ b/flang/test/Integration/debug-extra-global-2.f90 @@ -4,5 +4,5 @@ module m integer XcX end - +! Test that global starting with 'X' don't get filtered. CHECK: !DIGlobalVariable(name: "xcx", linkageName: "_QMmExcx"{{.*}}) From f22f41e5492560901574bd2af86376cade8fb678 Mon Sep 17 00:00:00 2001 From: Abid Qadeer Date: Wed, 16 Oct 2024 17:51:27 +0100 Subject: [PATCH 4/4] Handle review comments. Added isSpecialSymbol to check for symbols which don't require debug info. --- flang/include/flang/Optimizer/Support/InternalNames.h | 4 ++++ flang/lib/Optimizer/Support/InternalNames.cpp | 4 ++++ flang/lib/Optimizer/Transforms/AddDebugInfo.cpp | 8 +------- flang/test/Integration/debug-extra-global-2.f90 | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/flang/include/flang/Optimizer/Support/InternalNames.h b/flang/include/flang/Optimizer/Support/InternalNames.h index 67ab36cf8da7f..41f2cb9842dc7 100644 --- a/flang/include/flang/Optimizer/Support/InternalNames.h +++ b/flang/include/flang/Optimizer/Support/InternalNames.h @@ -184,6 +184,10 @@ struct NameUniquer { static std::string replaceSpecialSymbols(const std::string &name); + /// Returns true if the passed name denotes a special symbol (e.g. global + /// symbol generated for derived type description). + static bool isSpecialSymbol(llvm::StringRef name); + private: static std::string intAsString(std::int64_t i); static std::string doKind(std::int64_t kind); diff --git a/flang/lib/Optimizer/Support/InternalNames.cpp b/flang/lib/Optimizer/Support/InternalNames.cpp index 58a5da5de7972..011021c9f0350 100644 --- a/flang/lib/Optimizer/Support/InternalNames.cpp +++ b/flang/lib/Optimizer/Support/InternalNames.cpp @@ -411,3 +411,7 @@ fir::NameUniquer::dropTypeConversionMarkers(llvm::StringRef mangledTypeName) { std::string fir::NameUniquer::replaceSpecialSymbols(const std::string &name) { return std::regex_replace(name, std::regex{"\\."}, "X"); } + +bool fir::NameUniquer::isSpecialSymbol(llvm::StringRef name) { + return !name.empty() && (name[0] == '.' || name[0] == 'X'); +} diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp index f31c86cf67863..3a437c7a0f013 100644 --- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp +++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp @@ -211,13 +211,7 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp, if (result.first != fir::NameUniquer::NameKind::VARIABLE) return; - // Discard entries that describe a derived type. They start with either '.' or - // 'X'. We filter both of them out. Note that NameUniquer makes the name lower - // case so user variables should be safe. It would be better if result of the - // deconstruct had a flag for such values so that we dont have to look at - // string values. - if (!result.second.name.empty() && - (result.second.name[0] == '.' || result.second.name[0] == 'X')) + if (fir::NameUniquer::isSpecialSymbol(result.second.name)) return; unsigned line = getLineFromLoc(globalOp.getLoc()); diff --git a/flang/test/Integration/debug-extra-global-2.f90 b/flang/test/Integration/debug-extra-global-2.f90 index 59135747f5f97..59cb4b66def50 100644 --- a/flang/test/Integration/debug-extra-global-2.f90 +++ b/flang/test/Integration/debug-extra-global-2.f90 @@ -1,8 +1,8 @@ ! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s module m -integer XcX + integer XcX end ! Test that global starting with 'X' don't get filtered. -CHECK: !DIGlobalVariable(name: "xcx", linkageName: "_QMmExcx"{{.*}}) +! CHECK: !DIGlobalVariable(name: "xcx", linkageName: "_QMmExcx"{{.*}})