Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On Tue, Feb 09, 2021 at 08:55:26PM +0100, Jakub Jelinek wrote: > On Tue, Feb 09, 2021 at 02:40:12PM -0500, Jason Merrill wrote: > > For GCC 11, I think let's fix the regression with your (Jakub) earlier > > patch, maybe only for DIEs with DW_AT_const_value. > > Thanks. > Following works too, so I'll test it tonight. The patch fixed not just the expected -FAIL: g++.dg/debug/dwarf2/constexpr-var-1.C scan-assembler-times DW_AT_const_expr 2 but also -FAIL: libstdc++-prettyprinters/80276.cc whatis p4 -FAIL: libstdc++-prettyprinters/80276.cc whatis p4 -FAIL: libstdc++-prettyprinters/libfundts.cc print as -FAIL: libstdc++-prettyprinters/libfundts.cc print as -FAIL: libstdc++-prettyprinters/libfundts.cc print os -FAIL: libstdc++-prettyprinters/libfundts.cc print os Jakub
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On Tue, Feb 09, 2021 at 02:40:12PM -0500, Jason Merrill wrote: > For GCC 11, I think let's fix the regression with your (Jakub) earlier > patch, maybe only for DIEs with DW_AT_const_value. Thanks. Following works too, so I'll test it tonight. 2021-02-09 Jakub Jelinek PR debug/98755 * dwarf2out.c (prune_unused_types_walk): Mark DW_TAG_variable DIEs at class scope for DWARF5+. --- gcc/dwarf2out.c.jj 2021-01-27 10:05:35.313942040 +0100 +++ gcc/dwarf2out.c 2021-02-09 20:51:42.907922550 +0100 @@ -29475,6 +29475,16 @@ prune_unused_types_walk (dw_die_ref die) if (die->die_perennial_p) break; + /* For static data members, the declaration in the class is supposed +to have DW_TAG_member tag in DWARF{3,4} but DW_TAG_variable in +DWARF5. DW_TAG_member will be marked, so mark even such +DW_TAG_variables in DWARF5, as long as it has DW_AT_const_value +attribute. */ + if (dwarf_version >= 5 + && class_scope_p (die->die_parent) + && get_AT (die, DW_AT_const_value)) + break; + /* premark_used_variables marks external variables --- don't mark them here. But function-local externals are always considered used. */ Jakub
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On 9/1/20 2:46 PM, Jason Merrill wrote: On 8/25/20 5:19 AM, Mark Wielaard wrote: On Mon, 2020-08-24 at 17:38 -0400, Jason Merrill wrote: This looks incorrect to me, that is a workaround for a real GCC bug. Shouldn't we instead do something like (untested) following patch? I mean, for DWARF < 5 the static data members were using DW_TAG_member, which has been always marked by the function, so IMHO we should also always mark the DW_TAG_variables at the class scope that replaced those. The earlier behavior seems like an accident, that happened because we always need to emit information about non-static data members. I don't think we should take it as guidance. Maybe the reason they got emitted this way was an accident on the GCC side. But I don't think it is an accident on the GDB side. At least looking at the various gdb testcases, the intention is to show a struct/class type as defined to the user, which includes both the static and non-static data members of a class. That would make sense. So, GDB prefers no pruning of members... In this case one reason we don't emit debug info is because (before C++17) there's no definition of 'b'. After C++17 the in-class declaration of 'b' is a definition, but we don't have to give it a symbol, so there's still nothing for the debug info to describe. But don't we describe other parts of a type that don't have a symbol as part of the debug info? It seems that currently we describe unused/undefined functions, but not unused nested types/typedefs. On 8/25/20 8:41 AM, Jakub Jelinek wrote: On Mon, Aug 24, 2020 at 05:38:28PM -0400, Jason Merrill via Gcc-patches wrote: This issue doesn't seem specific to class members; it also affects namespace-scope C++17 inline variables: inline const int var = 42; int main() { return var; } Compiling this testcase with -g doesn't emit any debug info for 'var' even though it's used. Should we assume that if a variable with DW_AT_const_value is TREE_USED, we need to write out debug info for it? I guess it is a question of how exactly the (default on) -feliminate-unused-debug-symbols should behave with different kind of entities. One thing are the non-inline static data members without const/constexpr or without initializer in the class. Those need a definition if they are ever used, so it is probably fine to only emit them in the class in the TU where they are defined. But note that e.g. with -fdebug-types-section we still force them to be output in class and do no pruning (and the pruning actually makes dwz less efficient unless dwz is tought to not only merge the DIEs with the same children and attributes, but also reconstruct more complete DIEs out of several less complete ones for the same class). Right, this gets at Mark's point above. How much pruning do we want to do of class bodies? We currently do some, but how much benefit does that actually give us? Is it worth the cost? ...and our deduplication mechanisms prefer no pruning of members. Another case is non-inline static const data member with initializer, do we track non-odr uses e.g. during constant evaluation and if so, should that result in the static data member appearing? Because if the static const data member with initializer is never odr used, it doesn't have to be ever defined and so it might never appear in the debug info. Another case are inline vars, shall we treat as being used just that they were used in some constant expression, or do only odr uses count? If the goal of debug info is to be able to evaluate the same expressions that appear in the source, constant uses need to count, too. I wonder how we could associate the uses with their context so pruning works properly. For GCC 11, I think let's fix the regression with your (Jakub) earlier patch, maybe only for DIEs with DW_AT_const_value. For GCC 12, maybe we want to stop pruning any class members by default. Jason
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On 8/25/20 5:19 AM, Mark Wielaard wrote: On Mon, 2020-08-24 at 17:38 -0400, Jason Merrill wrote: This looks incorrect to me, that is a workaround for a real GCC bug. Shouldn't we instead do something like (untested) following patch? I mean, for DWARF < 5 the static data members were using DW_TAG_member, which has been always marked by the function, so IMHO we should also always mark the DW_TAG_variables at the class scope that replaced those. The earlier behavior seems like an accident, that happened because we always need to emit information about non-static data members. I don't think we should take it as guidance. Maybe the reason they got emitted this way was an accident on the GCC side. But I don't think it is an accident on the GDB side. At least looking at the various gdb testcases, the intention is to show a struct/class type as defined to the user, which includes both the static and non-static data members of a class. That would make sense. In this case one reason we don't emit debug info is because (before C++17) there's no definition of 'b'. After C++17 the in-class declaration of 'b' is a definition, but we don't have to give it a symbol, so there's still nothing for the debug info to describe. But don't we describe other parts of a type that don't have a symbol as part of the debug info? It seems that currently we describe unused/undefined functions, but not unused nested types/typedefs. On 8/25/20 8:41 AM, Jakub Jelinek wrote: On Mon, Aug 24, 2020 at 05:38:28PM -0400, Jason Merrill via Gcc-patches wrote: This issue doesn't seem specific to class members; it also affects namespace-scope C++17 inline variables: inline const int var = 42; int main() { return var; } Compiling this testcase with -g doesn't emit any debug info for 'var' even though it's used. Should we assume that if a variable with DW_AT_const_value is TREE_USED, we need to write out debug info for it? I guess it is a question of how exactly the (default on) -feliminate-unused-debug-symbols should behave with different kind of entities. One thing are the non-inline static data members without const/constexpr or without initializer in the class. Those need a definition if they are ever used, so it is probably fine to only emit them in the class in the TU where they are defined. But note that e.g. with -fdebug-types-section we still force them to be output in class and do no pruning (and the pruning actually makes dwz less efficient unless dwz is tought to not only merge the DIEs with the same children and attributes, but also reconstruct more complete DIEs out of several less complete ones for the same class). Right, this gets at Mark's point above. How much pruning do we want to do of class bodies? We currently do some, but how much benefit does that actually give us? Is it worth the cost? Another case is non-inline static const data member with initializer, do we track non-odr uses e.g. during constant evaluation and if so, should that result in the static data member appearing? Because if the static const data member with initializer is never odr used, it doesn't have to be ever defined and so it might never appear in the debug info. Another case are inline vars, shall we treat as being used just that they were used in some constant expression, or do only odr uses count? If the goal of debug info is to be able to evaluate the same expressions that appear in the source, constant uses need to count, too. I wonder how we could associate the uses with their context so pruning works properly. Jason
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On Mon, Aug 24, 2020 at 05:38:28PM -0400, Jason Merrill via Gcc-patches wrote: > > Shouldn't we instead do something like (untested) following patch? > > I mean, for DWARF < 5 the static data members were using DW_TAG_member, > > which has been always marked by the function, so IMHO we should also always > > mark the DW_TAG_variables at the class scope that replaced those. > > The earlier behavior seems like an accident, that happened because we always > need to emit information about non-static data members. I don't think we > should take it as guidance. > > In this case one reason we don't emit debug info is because (before C++17) > there's no definition of 'b'. After C++17 the in-class declaration of 'b' > is a definition, but we don't have to give it a symbol, so there's still > nothing for the debug info to describe. > > This issue doesn't seem specific to class members; it also affects > namespace-scope C++17 inline variables: > > inline const int var = 42; > int main() { return var; } > > Compiling this testcase with -g doesn't emit any debug info for 'var' even > though it's used. > > Should we assume that if a variable with DW_AT_const_value is TREE_USED, we > need to write out debug info for it? I guess it is a question of how exactly the (default on) -feliminate-unused-debug-symbols should behave with different kind of entities. One thing are the non-inline static data members without const/constexpr or without initializer in the class. Those need a definition if they are ever used, so it is probably fine to only emit them in the class in the TU where they are defined. But note that e.g. with -fdebug-types-section we still force them to be output in class and do no pruning (and the pruning actually makes dwz less efficient unless dwz is tought to not only merge the DIEs with the same children and attributes, but also reconstruct more complete DIEs out of several less complete ones for the same class). Another case is non-inline static const data member with initializer, do we track non-odr uses e.g. during constant evaluation and if so, should that result in the static data member appearing? Because if the static const data member with initializer is never odr used, it doesn't have to be ever defined and so it might never appear in the debug info. Another case are inline vars, shall we treat as being used just that they were used in some constant expression, or do only odr uses count? And, shall we make the DWARF-3/4 DW_TAG_member in class behave the same as DW_TAG_variable in DWARF-5? Jakub
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
Hi, On Mon, 2020-08-24 at 17:38 -0400, Jason Merrill wrote: > > This looks incorrect to me, that is a workaround for a real GCC bug. > > > > Shouldn't we instead do something like (untested) following patch? > > I mean, for DWARF < 5 the static data members were using DW_TAG_member, > > which has been always marked by the function, so IMHO we should also always > > mark the DW_TAG_variables at the class scope that replaced those. > > The earlier behavior seems like an accident, that happened because we > always need to emit information about non-static data members. I don't > think we should take it as guidance. Maybe the reason they got emitted this way was an accident on the GCC side. But I don't think it is an accident on the GDB side. At least looking at the various gdb testcases, the intention is to show a struct/class type as defined to the user, which includes both the static and non-static data members of a class. > In this case one reason we don't emit debug info is because (before > C++17) there's no definition of 'b'. After C++17 the in-class > declaration of 'b' is a definition, but we don't have to give it a > symbol, so there's still nothing for the debug info to describe. But don't we describe other parts of a type that don't have a symbol as part of the debug info? > This issue doesn't seem specific to class members; it also affects > namespace-scope C++17 inline variables: > > inline const int var = 42; > int main() { return var; } > > Compiling this testcase with -g doesn't emit any debug info for 'var' > even though it's used. That is new in GCC11, don't have GCC10 here to compare against, but GCC9 produces: DWARF section [27] '.debug_info' at offset 0x30b5: [Offset] Compilation unit at offset 0: Version: 5, Abbreviation section offset: 0, Address size: 8, Offset size: 4 Unit type: compile (1) [ c] compile_unit abbrev: 1 producer (strp) "GNU C++17 9.3.1 20200408 (Red Hat 9.3.1-2) -mtune=generic -march=x86-64 -gdwarf-5 -O2 -std=gnu++17" language (data1) C_plus_plus_14 (33) name (strp) "p.cc" comp_dir (strp) "/tmp" ranges (sec_offset) range list [ c] low_pc (addr) 00 stmt_list(sec_offset) 0 [2a]variable abbrev: 2 name (string) "var" decl_file(data1) p.cc (1) decl_line(data1) 1 decl_column (data1) 18 type (ref4) [3f] external (flag_present) yes inline (data1) declared_inlined (3) const_value (data1) 42 [38]base_typeabbrev: 3 byte_size(data1) 4 encoding (data1) signed (5) name (string) "int" [3f]const_type abbrev: 4 type (ref4) [38] [44]subprogram abbrev: 5 external (flag_present) yes name (strp) "main" decl_file(data1) p.cc (1) decl_line(data1) 2 decl_column (data1) 5 type (ref4) [38] low_pc (addr) 0x00401050 high_pc (data8) 6 (0x00401056 <_start>) frame_base (exprloc) [ 0] call_frame_cfa call_all_calls (flag_present) yes > Should we assume that if a variable with DW_AT_const_value is TREE_USED, > we need to write out debug info for it? I would say yes. If it is used a user might want to look up its value. Cheers, Mark
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On 8/24/20 1:40 PM, Jakub Jelinek wrote: On Mon, Aug 24, 2020 at 02:56:55PM +0200, Mark Wielaard wrote: In DWARF5 class variables (static data members) are represented with a DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't optimized away in the constexpr-var-1.C testcase so we can still match (2) const_expr in the the assembly output. Note that the same issue causes some failures in the gdb testsuite for static data members when we enable DWARF5 by default: https://sourceware.org/bugzilla/show_bug.cgi?id=26525 --- gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C index 19062e29fd59..c6ad3f645379 100644 --- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C +++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C @@ -7,3 +7,4 @@ struct S { static constexpr int b = 6; } s; +const int = s.b; This looks incorrect to me, that is a workaround for a real GCC bug. Shouldn't we instead do something like (untested) following patch? I mean, for DWARF < 5 the static data members were using DW_TAG_member, which has been always marked by the function, so IMHO we should also always mark the DW_TAG_variables at the class scope that replaced those. The earlier behavior seems like an accident, that happened because we always need to emit information about non-static data members. I don't think we should take it as guidance. In this case one reason we don't emit debug info is because (before C++17) there's no definition of 'b'. After C++17 the in-class declaration of 'b' is a definition, but we don't have to give it a symbol, so there's still nothing for the debug info to describe. This issue doesn't seem specific to class members; it also affects namespace-scope C++17 inline variables: inline const int var = 42; int main() { return var; } Compiling this testcase with -g doesn't emit any debug info for 'var' even though it's used. Should we assume that if a variable with DW_AT_const_value is TREE_USED, we need to write out debug info for it? Jason 2020-08-24 Jakub Jelinek * dwarf2out.c (prune_unused_types_walk): Mark DW_TAG_variable DIEs at class scope for DWARF5+. --- gcc/dwarf2out.c.jj 2020-07-28 15:39:09.883757946 +0200 +++ gcc/dwarf2out.c 2020-08-24 19:33:16.503961786 +0200 @@ -29392,6 +29392,13 @@ prune_unused_types_walk (dw_die_ref die) if (die->die_perennial_p) break; + /* For static data members, the declaration in the class is supposed +to have DW_TAG_member tag in DWARF{3,4} but DW_TAG_variable in +DWARF5. DW_TAG_member will be marked, so mark even such +DW_TAG_variables in DWARF5. */ + if (dwarf_version >= 5 && class_scope_p (die->die_parent)) + break; + /* premark_used_variables marks external variables --- don't mark them here. But function-local externals are always considered used. */ Jakub
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
>> This looks incorrect to me, that is a workaround for a real GCC bug. Mark> I was discussing this after the BoF with Tom Tromey (CCed) and he also Mark> thought gdb could/should actually support the DWARF5 representation, Mark> but because the DW_TAG_variable was removed because the static data Mark> member wasn't referenced in the gdb testcases. I updated the gdb bug with some findings: https://sourceware.org/bugzilla/show_bug.cgi?id=26525 Mark> This looks really good, and it makes all the FAILs in the gdb bug Mark> report PASS (when build with -gdwarf-5 as default). I thought this might be the case; thank you for trying it out. Tom
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
Hi Jakub, On Mon, Aug 24, 2020 at 07:40:51PM +0200, Jakub Jelinek wrote: > On Mon, Aug 24, 2020 at 02:56:55PM +0200, Mark Wielaard wrote: > > In DWARF5 class variables (static data members) are represented with a > > DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't > > optimized away in the constexpr-var-1.C testcase so we can still match (2) > > const_expr in the the assembly output. > > > > Note that the same issue causes some failures in the gdb testsuite > > for static data members when we enable DWARF5 by default: > > https://sourceware.org/bugzilla/show_bug.cgi?id=26525 > > --- > > gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > > b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > > index 19062e29fd59..c6ad3f645379 100644 > > --- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > > +++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > > @@ -7,3 +7,4 @@ struct S > > { > >static constexpr int b = 6; > > } s; > > +const int = s.b; > > This looks incorrect to me, that is a workaround for a real GCC bug. I was discussing this after the BoF with Tom Tromey (CCed) and he also thought gdb could/should actually support the DWARF5 representation, but because the DW_TAG_variable was removed because the static data member wasn't referenced in the gdb testcases. > Shouldn't we instead do something like (untested) following patch? > I mean, for DWARF < 5 the static data members were using DW_TAG_member, > which has been always marked by the function, so IMHO we should also always > mark the DW_TAG_variables at the class scope that replaced those. > > 2020-08-24 Jakub Jelinek > > * dwarf2out.c (prune_unused_types_walk): Mark DW_TAG_variable DIEs > at class scope for DWARF5+. This looks really good, and it makes all the FAILs in the gdb bug report PASS (when build with -gdwarf-5 as default). > --- gcc/dwarf2out.c.jj2020-07-28 15:39:09.883757946 +0200 > +++ gcc/dwarf2out.c 2020-08-24 19:33:16.503961786 +0200 > @@ -29392,6 +29392,13 @@ prune_unused_types_walk (dw_die_ref die) > if (die->die_perennial_p) > break; > > + /* For static data members, the declaration in the class is supposed > + to have DW_TAG_member tag in DWARF{3,4} but DW_TAG_variable in > + DWARF5. DW_TAG_member will be marked, so mark even such > + DW_TAG_variables in DWARF5. */ > + if (dwarf_version >= 5 && class_scope_p (die->die_parent)) > + break; > + > /* premark_used_variables marks external variables --- don't mark >them here. But function-local externals are always considered >used. */ Thanks, Mark
Re: [PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
On Mon, Aug 24, 2020 at 02:56:55PM +0200, Mark Wielaard wrote: > In DWARF5 class variables (static data members) are represented with a > DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't > optimized away in the constexpr-var-1.C testcase so we can still match (2) > const_expr in the the assembly output. > > Note that the same issue causes some failures in the gdb testsuite > for static data members when we enable DWARF5 by default: > https://sourceware.org/bugzilla/show_bug.cgi?id=26525 > --- > gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > index 19062e29fd59..c6ad3f645379 100644 > --- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > +++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C > @@ -7,3 +7,4 @@ struct S > { >static constexpr int b = 6; > } s; > +const int = s.b; This looks incorrect to me, that is a workaround for a real GCC bug. Shouldn't we instead do something like (untested) following patch? I mean, for DWARF < 5 the static data members were using DW_TAG_member, which has been always marked by the function, so IMHO we should also always mark the DW_TAG_variables at the class scope that replaced those. 2020-08-24 Jakub Jelinek * dwarf2out.c (prune_unused_types_walk): Mark DW_TAG_variable DIEs at class scope for DWARF5+. --- gcc/dwarf2out.c.jj 2020-07-28 15:39:09.883757946 +0200 +++ gcc/dwarf2out.c 2020-08-24 19:33:16.503961786 +0200 @@ -29392,6 +29392,13 @@ prune_unused_types_walk (dw_die_ref die) if (die->die_perennial_p) break; + /* For static data members, the declaration in the class is supposed +to have DW_TAG_member tag in DWARF{3,4} but DW_TAG_variable in +DWARF5. DW_TAG_member will be marked, so mark even such +DW_TAG_variables in DWARF5. */ + if (dwarf_version >= 5 && class_scope_p (die->die_parent)) + break; + /* premark_used_variables marks external variables --- don't mark them here. But function-local externals are always considered used. */ Jakub
[PATCH 2/5] Make sure that static data member constexpr isn't optimized away in test.
In DWARF5 class variables (static data members) are represented with a DW_TAG_variable instead of a DW_TAG_member. Make sure the variable isn't optimized away in the constexpr-var-1.C testcase so we can still match (2) const_expr in the the assembly output. Note that the same issue causes some failures in the gdb testsuite for static data members when we enable DWARF5 by default: https://sourceware.org/bugzilla/show_bug.cgi?id=26525 --- gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C index 19062e29fd59..c6ad3f645379 100644 --- a/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C +++ b/gcc/testsuite/g++.dg/debug/dwarf2/constexpr-var-1.C @@ -7,3 +7,4 @@ struct S { static constexpr int b = 6; } s; +const int = s.b; -- 2.18.4