| Message ID | 3b197eef-db36-7df9-417e-e17be0616b6c@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška <mliska@suse.cz> wrote: > May I ask Jim to test the patch? > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. I started an aarch64 bootstrap to test. My fast machine is busy with work tasks, so I have to use a slower machine, and hence this will take some time. Jim
On Fri, Jul 14, 2017 at 12:59 PM, Jim Wilson <jim.wilson@linaro.org> wrote: > On Fri, Jul 14, 2017 at 1:35 AM, Martin Liška <mliska@suse.cz> wrote: >> May I ask Jim to test the patch? >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > I started an aarch64 bootstrap to test. My fast machine is busy with > work tasks, so I have to use a slower machine, and hence this will > take some time. The aarch64-linux bootstrap with the patch succeeds. I also did a make check, and see no regressions. Jim
PING^1 On 07/14/2017 10:35 AM, Martin Liška wrote: > On 05/01/2017 09:13 PM, Jason Merrill wrote: >> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>> Hello. >>>>> >>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>> to survive regression tests. That's reason why I'm resending that. >>>>> >>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>> >>>>> Ready to be installed? >>>>> Martin >>>> >>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>> From: marxin <mliska@suse.cz> >>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>> symbol entry. >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>> Martin Liska <mliska@suse.cz> >>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>> >>>>> PR c++/64266 >>>>> PR c++/70353 >>>>> PR bootstrap/70422 >>>>> Core issue 1962 >>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>> >>>> If we don't emit those into the debug info, will the debugger be >>>> able to handle __FUNCTION__ etc. properly? >>> >>> No, debugger with the patch can't handled these. Similar to how clang >>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>> >>>> Admittedly, right now we emit it into debug info only if those decls >>>> are actually used, say on: >>>> const char * foo () { return __FUNCTION__; } >>>> const char * bar () { return ""; } >>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>> has to have some handling of it anyway. But while in functions >>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>> to synthetize those and thus they will be always pointer-equal, >>>> if there are some uses of it and for other uses the debugger would >>>> synthetize it, there is the possibility that the debugger synthetized >>>> string will not be the same object as actually used in the function. >>> >>> You're right, currently one has to use a special function to be able to >>> print it in debugger. I believe we've already discussed that, according >>> to spec, the strings don't have to point to a same string. >>> >>> Suggestions what we should do with the patch? >> >> We need to emit debug information for these variables. From Jim's >> description in 70422 it seems that the problem is that the reference >> to the string from the debug information is breaking >> function_mergeable_rodata_prefix, which relies on >> current_function_decl. It seems to me that its callers should pass >> along their decl parameter so that f_m_r_p can use the decl's >> DECL_CONTEXT rather than rely on current_function_decl being set >> properly> >> Jason >> > > Ok, after some time I returned back to it. I followed your advises and > changed the function function_mergeable_rodata_prefix. Apart from a small > rebase was needed. > > May I ask Jim to test the patch? > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Martin >
PING^2 On 07/27/2017 02:56 PM, Martin Liška wrote: > PING^1 > > On 07/14/2017 10:35 AM, Martin Liška wrote: >> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>> Hello. >>>>>> >>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>> >>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>> >>>>>> Ready to be installed? >>>>>> Martin >>>>> >>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>> From: marxin <mliska@suse.cz> >>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>> symbol entry. >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>> Martin Liska <mliska@suse.cz> >>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>> >>>>>> PR c++/64266 >>>>>> PR c++/70353 >>>>>> PR bootstrap/70422 >>>>>> Core issue 1962 >>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>> >>>>> If we don't emit those into the debug info, will the debugger be >>>>> able to handle __FUNCTION__ etc. properly? >>>> >>>> No, debugger with the patch can't handled these. Similar to how clang >>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>> >>>>> Admittedly, right now we emit it into debug info only if those decls >>>>> are actually used, say on: >>>>> const char * foo () { return __FUNCTION__; } >>>>> const char * bar () { return ""; } >>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>> has to have some handling of it anyway. But while in functions >>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>> to synthetize those and thus they will be always pointer-equal, >>>>> if there are some uses of it and for other uses the debugger would >>>>> synthetize it, there is the possibility that the debugger synthetized >>>>> string will not be the same object as actually used in the function. >>>> >>>> You're right, currently one has to use a special function to be able to >>>> print it in debugger. I believe we've already discussed that, according >>>> to spec, the strings don't have to point to a same string. >>>> >>>> Suggestions what we should do with the patch? >>> >>> We need to emit debug information for these variables. From Jim's >>> description in 70422 it seems that the problem is that the reference >>> to the string from the debug information is breaking >>> function_mergeable_rodata_prefix, which relies on >>> current_function_decl. It seems to me that its callers should pass >>> along their decl parameter so that f_m_r_p can use the decl's >>> DECL_CONTEXT rather than rely on current_function_decl being set >>> properly> >>> Jason >>> >> >> Ok, after some time I returned back to it. I followed your advises and >> changed the function function_mergeable_rodata_prefix. Apart from a small >> rebase was needed. >> >> May I ask Jim to test the patch? >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Martin >> >
On 07/14/2017 01:35 AM, Martin Liška wrote: > On 05/01/2017 09:13 PM, Jason Merrill wrote: >> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>> Hello. >>>>> >>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>> to survive regression tests. That's reason why I'm resending that. >>>>> >>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>> >>>>> Ready to be installed? >>>>> Martin >>>> >>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>> From: marxin <mliska@suse.cz> >>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>> symbol entry. >>>>> >>>>> gcc/cp/ChangeLog: >>>>> >>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>> Martin Liska <mliska@suse.cz> >>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>> >>>>> PR c++/64266 >>>>> PR c++/70353 >>>>> PR bootstrap/70422 >>>>> Core issue 1962 >>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>> >>>> If we don't emit those into the debug info, will the debugger be >>>> able to handle __FUNCTION__ etc. properly? >>> >>> No, debugger with the patch can't handled these. Similar to how clang >>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>> >>>> Admittedly, right now we emit it into debug info only if those decls >>>> are actually used, say on: >>>> const char * foo () { return __FUNCTION__; } >>>> const char * bar () { return ""; } >>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>> has to have some handling of it anyway. But while in functions >>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>> to synthetize those and thus they will be always pointer-equal, >>>> if there are some uses of it and for other uses the debugger would >>>> synthetize it, there is the possibility that the debugger synthetized >>>> string will not be the same object as actually used in the function. >>> >>> You're right, currently one has to use a special function to be able to >>> print it in debugger. I believe we've already discussed that, according >>> to spec, the strings don't have to point to a same string. >>> >>> Suggestions what we should do with the patch? >> >> We need to emit debug information for these variables. From Jim's >> description in 70422 it seems that the problem is that the reference >> to the string from the debug information is breaking >> function_mergeable_rodata_prefix, which relies on >> current_function_decl. It seems to me that its callers should pass >> along their decl parameter so that f_m_r_p can use the decl's >> DECL_CONTEXT rather than rely on current_function_decl being set >> properly> >> Jason >> > > Ok, after some time I returned back to it. I followed your advises and > changed the function function_mergeable_rodata_prefix. Apart from a small > rebase was needed. > > May I ask Jim to test the patch? > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > + DECL_IGNORED_P (decl) = 1; As I said before, we do need to emit debug information for these variables, so this is wrong. > - section *s = targetm.asm_out.function_rodata_section (current_function_decl); > + tree decl = current_function_decl; > + if (decl && DECL_CONTEXT (decl) > + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) > + decl = DECL_CONTEXT (decl); I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. Jason
On 08/10/2017 09:43 PM, Jason Merrill wrote: > On 07/14/2017 01:35 AM, Martin Liška wrote: >> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>> Hello. >>>>>> >>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>> >>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>> >>>>>> Ready to be installed? >>>>>> Martin >>>>> >>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>> From: marxin <mliska@suse.cz> >>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>> symbol entry. >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>> Martin Liska <mliska@suse.cz> >>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>> >>>>>> PR c++/64266 >>>>>> PR c++/70353 >>>>>> PR bootstrap/70422 >>>>>> Core issue 1962 >>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>> >>>>> If we don't emit those into the debug info, will the debugger be >>>>> able to handle __FUNCTION__ etc. properly? >>>> >>>> No, debugger with the patch can't handled these. Similar to how clang >>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>> >>>>> Admittedly, right now we emit it into debug info only if those decls >>>>> are actually used, say on: >>>>> const char * foo () { return __FUNCTION__; } >>>>> const char * bar () { return ""; } >>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>> has to have some handling of it anyway. But while in functions >>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>> to synthetize those and thus they will be always pointer-equal, >>>>> if there are some uses of it and for other uses the debugger would >>>>> synthetize it, there is the possibility that the debugger synthetized >>>>> string will not be the same object as actually used in the function. >>>> >>>> You're right, currently one has to use a special function to be able to >>>> print it in debugger. I believe we've already discussed that, according >>>> to spec, the strings don't have to point to a same string. >>>> >>>> Suggestions what we should do with the patch? >>> >>> We need to emit debug information for these variables. From Jim's >>> description in 70422 it seems that the problem is that the reference >>> to the string from the debug information is breaking >>> function_mergeable_rodata_prefix, which relies on >>> current_function_decl. It seems to me that its callers should pass >>> along their decl parameter so that f_m_r_p can use the decl's >>> DECL_CONTEXT rather than rely on current_function_decl being set >>> properly> >>> Jason >>> >> >> Ok, after some time I returned back to it. I followed your advises and >> changed the function function_mergeable_rodata_prefix. Apart from a small >> rebase was needed. >> >> May I ask Jim to test the patch? >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > >> + DECL_IGNORED_P (decl) = 1; > > As I said before, we do need to emit debug information for these variables, so this is wrong. Hello. Sorry for overlooking that. > >> - section *s = targetm.asm_out.function_rodata_section (current_function_decl); >> + tree decl = current_function_decl; >> + if (decl && DECL_CONTEXT (decl) >> + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >> + decl = DECL_CONTEXT (decl); > > I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time. Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section is called with STRING_CST (which is VALUE_EXPR of created fname_decl): #0 mergeable_string_section (decl=0x2aaaac2bf1a0, align=64, flags=0) at ../../gcc/varasm.c:796 #1 0x0000000001594ce3 in default_elf_select_section (decl=0x2aaaac2bf1a0, reloc=0, align=64) at ../../gcc/varasm.c:6641 #2 0x000000000158b649 in get_constant_section (exp=0x2aaaac2bf1a0, align=64) at ../../gcc/varasm.c:3284 #3 0x000000000158bb31 in build_constant_desc (exp=0x2aaaac2bf1a0) at ../../gcc/varasm.c:3352 #4 0x000000000158bebc in output_constant_def (exp=0x2aaaac2bf1a0, defer=0) at ../../gcc/varasm.c:3415 #5 0x0000000000d68264 in expand_expr_constant (exp=0x2aaaac2bf1a0, defer=0, modifier=EXPAND_NORMAL) at ../../gcc/expr.c:7701 #6 0x0000000000d682e2 in expand_expr_addr_expr_1 (exp=0x2aaaac2bf1a0, target=0x0, tmode=..., modifier=EXPAND_NORMAL, as=0 '\000') at ../../gcc/expr.c:7728 #7 0x0000000000d690c7 in expand_expr_addr_expr (exp=0x2aaaac2bff00, target=0x0, tmode=E_DImode, modifier=EXPAND_NORMAL) at ../../gcc/expr.c:7919 #8 0x0000000000d76d42 in expand_expr_real_1 (exp=0x2aaaac2bff00, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:11084 #9 0x0000000000d69543 in expand_expr_real (exp=0x2aaaac2bff00, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:8087 #10 0x0000000000be75fe in expand_normal (exp=0x2aaaac2bff00) at ../../gcc/expr.h:282 #11 0x0000000000be9626 in precompute_register_parameters (num_actuals=2, args=0x26c1550, reg_parm_seen=0x7fffffffc73c) at ../../gcc/calls.c:958 #12 0x0000000000bf2770 in expand_call (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, ignore=1) at ../../gcc/calls.c:3677 #13 0x0000000000bd667d in expand_builtin (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, subtarget=0x0, mode=E_VOIDmode, ignore=1) at ../../gcc/builtins.c:7591 #14 0x0000000000d75c07 in expand_expr_real_1 (exp=0x2aaaac2d6040, target=0x0, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:10858 #15 0x0000000000d69543 in expand_expr_real (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, tmode=E_VOIDmode, modifier=EXPAND_NORMAL, alt_rtl=0x0, inner_reference_p=false) at ../../gcc/expr.c:8087 #16 0x0000000000c02f4f in expand_expr (exp=0x2aaaac2d6040, target=0x2aaaac0a1480, mode=E_VOIDmode, modifier=EXPAND_NORMAL) at ../../gcc/expr.h:276 #17 0x0000000000c0b144 in expand_call_stmt (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:2666 #18 0x0000000000c0e3a6 in expand_gimple_stmt_1 (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:3585 #19 0x0000000000c0ea9c in expand_gimple_stmt (stmt=0x2aaaac2d02d0) at ../../gcc/cfgexpand.c:3751 #20 0x0000000000c1665e in expand_gimple_basic_block (bb=0x2aaaac0be2d8, disable_tail_calls=false) at ../../gcc/cfgexpand.c:5750 (gdb) p debug_tree(decl) <string_cst 0x2aaaac2bf1a0 type <array_type 0x2aaaac2c4348 type <integer_type 0x2aaaac0c15e8 char readonly unsigned string-flag type_6 QI size <integer_cst 0x2aaaac099d80 constant 8> unit-size <integer_cst 0x2aaaac099d98 constant 1> align:8 warn_if_not_align:0 symtab:-1408509840 alias-set -1 canonical-type 0x2aaaac0c15e8 precision:8 min <integer_cst 0x2aaaac099de0 0> max <integer_cst 0x2aaaac099dc8 255> pointer_to_this <pointer_type 0x2aaaac0c1690>> SI size <integer_cst 0x2aaaac099ed0 constant 32> unit-size <integer_cst 0x2aaaac099ee8 constant 4> align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c4348 domain <integer_type 0x2aaaac2c41f8 type <integer_type 0x2aaaac0b0000 sizetype> type_6 DI size <integer_cst 0x2aaaac099c90 constant 64> unit-size <integer_cst 0x2aaaac099ca8 constant 8> align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c41f8 precision:64 min <integer_cst 0x2aaaac099cc0 0> max <integer_cst 0x2aaaac2bd888 3>> pointer_to_this <pointer_type 0x2aaaac2c43f0>> constant "foo\000"> And idea how to resolve this? Thank you, Martin > > Jason
On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote: > On 08/10/2017 09:43 PM, Jason Merrill wrote: >> On 07/14/2017 01:35 AM, Martin Liška wrote: >>> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>>> Hello. >>>>>>> >>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>>> >>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>>> >>>>>>> Ready to be installed? >>>>>>> Martin >>>>>> >>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>>> From: marxin <mliska@suse.cz> >>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>>> symbol entry. >>>>>>> >>>>>>> gcc/cp/ChangeLog: >>>>>>> >>>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>>> Martin Liska <mliska@suse.cz> >>>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>>> >>>>>>> PR c++/64266 >>>>>>> PR c++/70353 >>>>>>> PR bootstrap/70422 >>>>>>> Core issue 1962 >>>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>>> >>>>>> If we don't emit those into the debug info, will the debugger be >>>>>> able to handle __FUNCTION__ etc. properly? >>>>> >>>>> No, debugger with the patch can't handled these. Similar to how clang >>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>>> >>>>>> Admittedly, right now we emit it into debug info only if those decls >>>>>> are actually used, say on: >>>>>> const char * foo () { return __FUNCTION__; } >>>>>> const char * bar () { return ""; } >>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>>> has to have some handling of it anyway. But while in functions >>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>>> to synthetize those and thus they will be always pointer-equal, >>>>>> if there are some uses of it and for other uses the debugger would >>>>>> synthetize it, there is the possibility that the debugger synthetized >>>>>> string will not be the same object as actually used in the function. >>>>> >>>>> You're right, currently one has to use a special function to be able to >>>>> print it in debugger. I believe we've already discussed that, according >>>>> to spec, the strings don't have to point to a same string. >>>>> >>>>> Suggestions what we should do with the patch? >>>> >>>> We need to emit debug information for these variables. From Jim's >>>> description in 70422 it seems that the problem is that the reference >>>> to the string from the debug information is breaking >>>> function_mergeable_rodata_prefix, which relies on >>>> current_function_decl. It seems to me that its callers should pass >>>> along their decl parameter so that f_m_r_p can use the decl's >>>> DECL_CONTEXT rather than rely on current_function_decl being set >>>> properly> >>>> Jason >>>> >>> >>> Ok, after some time I returned back to it. I followed your advises and >>> changed the function function_mergeable_rodata_prefix. Apart from a small >>> rebase was needed. >>> >>> May I ask Jim to test the patch? >>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >>> + DECL_IGNORED_P (decl) = 1; >> >> As I said before, we do need to emit debug information for these variables, so this is wrong. > > Hello. > > Sorry for overlooking that. > >> >>> - section *s = targetm.asm_out.function_rodata_section (current_function_decl); >>> + tree decl = current_function_decl; >>> + if (decl && DECL_CONTEXT (decl) >>> + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >>> + decl = DECL_CONTEXT (decl); >> >> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. > > I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time. > Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section > is called with STRING_CST (which is VALUE_EXPR of created fname_decl): [...] > And idea how to resolve this? Well, when we're being called from the expander, current_function_decl is fine. Hmm, why would current_function_decl be wrong in dwarf2out? Can we fix that? Why does your change to function_mergeable_rodata_prefix make any difference? Jason
On 10/24/2017 10:24 PM, Jason Merrill wrote: > On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote: >> On 08/10/2017 09:43 PM, Jason Merrill wrote: >>> On 07/14/2017 01:35 AM, Martin Liška wrote: >>>> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>>>> Hello. >>>>>>>> >>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>>>> >>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>>>> >>>>>>>> Ready to be installed? >>>>>>>> Martin >>>>>>> >>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>>>> From: marxin <mliska@suse.cz> >>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>>>> symbol entry. >>>>>>>> >>>>>>>> gcc/cp/ChangeLog: >>>>>>>> >>>>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>>>> Martin Liska <mliska@suse.cz> >>>>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>>>> >>>>>>>> PR c++/64266 >>>>>>>> PR c++/70353 >>>>>>>> PR bootstrap/70422 >>>>>>>> Core issue 1962 >>>>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>>>> >>>>>>> If we don't emit those into the debug info, will the debugger be >>>>>>> able to handle __FUNCTION__ etc. properly? >>>>>> >>>>>> No, debugger with the patch can't handled these. Similar to how clang >>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>>>> >>>>>>> Admittedly, right now we emit it into debug info only if those decls >>>>>>> are actually used, say on: >>>>>>> const char * foo () { return __FUNCTION__; } >>>>>>> const char * bar () { return ""; } >>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>>>> has to have some handling of it anyway. But while in functions >>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>>>> to synthetize those and thus they will be always pointer-equal, >>>>>>> if there are some uses of it and for other uses the debugger would >>>>>>> synthetize it, there is the possibility that the debugger synthetized >>>>>>> string will not be the same object as actually used in the function. >>>>>> >>>>>> You're right, currently one has to use a special function to be able to >>>>>> print it in debugger. I believe we've already discussed that, according >>>>>> to spec, the strings don't have to point to a same string. >>>>>> >>>>>> Suggestions what we should do with the patch? >>>>> >>>>> We need to emit debug information for these variables. From Jim's >>>>> description in 70422 it seems that the problem is that the reference >>>>> to the string from the debug information is breaking >>>>> function_mergeable_rodata_prefix, which relies on >>>>> current_function_decl. It seems to me that its callers should pass >>>>> along their decl parameter so that f_m_r_p can use the decl's >>>>> DECL_CONTEXT rather than rely on current_function_decl being set >>>>> properly> >>>>> Jason >>>>> >>>> >>>> Ok, after some time I returned back to it. I followed your advises and >>>> changed the function function_mergeable_rodata_prefix. Apart from a small >>>> rebase was needed. >>>> >>>> May I ask Jim to test the patch? >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>> >>>> + DECL_IGNORED_P (decl) = 1; >>> >>> As I said before, we do need to emit debug information for these variables, so this is wrong. >> >> Hello. >> >> Sorry for overlooking that. >> >>> >>>> - section *s = targetm.asm_out.function_rodata_section (current_function_decl); >>>> + tree decl = current_function_decl; >>>> + if (decl && DECL_CONTEXT (decl) >>>> + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >>>> + decl = DECL_CONTEXT (decl); >>> >>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. >> >> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time. >> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section >> is called with STRING_CST (which is VALUE_EXPR of created fname_decl): > [...] >> And idea how to resolve this? > > Well, when we're being called from the expander, current_function_decl is fine. > > Hmm, why would current_function_decl be wrong in dwarf2out? Can we fix that? > > Why does your change to function_mergeable_rodata_prefix make any difference? > > Jason > Hi Jason. Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7 The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration that was inlined. And such references are only seen in debug mode. The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9 Note that both LLVM and ICC don't allow: $ 3 __builtin_printf ("%s\n", __PRETTY_FUNCTION__); (gdb) p __PRETTY_FUNCTION__ No symbol "__PRETTY_FUNCTION__" in current context. Would it be feasible solution to ignore the declaration? Thanks, Martin
On Mon, May 21, 2018 at 9:33 AM, Martin Liška <mliska@suse.cz> wrote: > On 10/24/2017 10:24 PM, Jason Merrill wrote: >> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote: >>> On 08/10/2017 09:43 PM, Jason Merrill wrote: >>>> On 07/14/2017 01:35 AM, Martin Liška wrote: >>>>> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>>>>> Hello. >>>>>>>>> >>>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>>>>> >>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>>>>> >>>>>>>>> Ready to be installed? >>>>>>>>> Martin >>>>>>>> >>>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>>>>> From: marxin <mliska@suse.cz> >>>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>>>>> symbol entry. >>>>>>>>> >>>>>>>>> gcc/cp/ChangeLog: >>>>>>>>> >>>>>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>>>>> Martin Liska <mliska@suse.cz> >>>>>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>>>>> >>>>>>>>> PR c++/64266 >>>>>>>>> PR c++/70353 >>>>>>>>> PR bootstrap/70422 >>>>>>>>> Core issue 1962 >>>>>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>>>>> >>>>>>>> If we don't emit those into the debug info, will the debugger be >>>>>>>> able to handle __FUNCTION__ etc. properly? >>>>>>> >>>>>>> No, debugger with the patch can't handled these. Similar to how clang >>>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>>>>> >>>>>>>> Admittedly, right now we emit it into debug info only if those decls >>>>>>>> are actually used, say on: >>>>>>>> const char * foo () { return __FUNCTION__; } >>>>>>>> const char * bar () { return ""; } >>>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>>>>> has to have some handling of it anyway. But while in functions >>>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>>>>> to synthetize those and thus they will be always pointer-equal, >>>>>>>> if there are some uses of it and for other uses the debugger would >>>>>>>> synthetize it, there is the possibility that the debugger synthetized >>>>>>>> string will not be the same object as actually used in the function. >>>>>>> >>>>>>> You're right, currently one has to use a special function to be able to >>>>>>> print it in debugger. I believe we've already discussed that, according >>>>>>> to spec, the strings don't have to point to a same string. >>>>>>> >>>>>>> Suggestions what we should do with the patch? >>>>>> >>>>>> We need to emit debug information for these variables. From Jim's >>>>>> description in 70422 it seems that the problem is that the reference >>>>>> to the string from the debug information is breaking >>>>>> function_mergeable_rodata_prefix, which relies on >>>>>> current_function_decl. It seems to me that its callers should pass >>>>>> along their decl parameter so that f_m_r_p can use the decl's >>>>>> DECL_CONTEXT rather than rely on current_function_decl being set >>>>>> properly> >>>>>> Jason >>>>>> >>>>> >>>>> Ok, after some time I returned back to it. I followed your advises and >>>>> changed the function function_mergeable_rodata_prefix. Apart from a small >>>>> rebase was needed. >>>>> >>>>> May I ask Jim to test the patch? >>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>>> + DECL_IGNORED_P (decl) = 1; >>>> >>>> As I said before, we do need to emit debug information for these variables, so this is wrong. >>> >>> Hello. >>> >>> Sorry for overlooking that. >>> >>>> >>>>> - section *s = targetm.asm_out.function_rodata_section (current_function_decl); >>>>> + tree decl = current_function_decl; >>>>> + if (decl && DECL_CONTEXT (decl) >>>>> + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >>>>> + decl = DECL_CONTEXT (decl); >>>> >>>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. >>> >>> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time. >>> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section >>> is called with STRING_CST (which is VALUE_EXPR of created fname_decl): >> [...] >>> And idea how to resolve this? >> >> Well, when we're being called from the expander, current_function_decl is fine. >> >> Hmm, why would current_function_decl be wrong in dwarf2out? Can we fix that? >> >> Why does your change to function_mergeable_rodata_prefix make any difference? > > Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7 > > The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration > that was inlined. And such references are only seen in debug mode. > > The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9 > > Note that both LLVM and ICC don't allow: > $ 3 __builtin_printf ("%s\n", __PRETTY_FUNCTION__); > (gdb) p __PRETTY_FUNCTION__ > No symbol "__PRETTY_FUNCTION__" in current context. > > Would it be feasible solution to ignore the declaration? It seems a rather user-unfriendly solution; being able to look at the value of an identifier used in the program is a pretty basic expectation for the debugging experience. In Jim's comment you mention, he says, "It seems like there are some conflicting goals here. We want to share the string across functions, but we also want to put it in function specific comdat sections." The recent discussion thread at https://gcc.gnu.org/ml/gcc/2018-05/threads.html#00109 deals with a very similar issue, of wanting to both merge constant data and associate it with a function. The discussion there seems to be tending toward merging rather than function-grouping, which seems to match the desire in this PR. Why do __*FUNCTION__ cause problems that constexpr const char *p = "function name"; doesn't? Jason
On 05/21/2018 07:19 PM, Jason Merrill wrote: > On Mon, May 21, 2018 at 9:33 AM, Martin Liška <mliska@suse.cz> wrote: >> On 10/24/2017 10:24 PM, Jason Merrill wrote: >>> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška <mliska@suse.cz> wrote: >>>> On 08/10/2017 09:43 PM, Jason Merrill wrote: >>>>> On 07/14/2017 01:35 AM, Martin Liška wrote: >>>>>> On 05/01/2017 09:13 PM, Jason Merrill wrote: >>>>>>> On Wed, Apr 26, 2017 at 6:58 AM, Martin Liška <mliska@suse.cz> wrote: >>>>>>>> On 04/25/2017 01:58 PM, Jakub Jelinek wrote: >>>>>>>>> On Tue, Apr 25, 2017 at 01:48:05PM +0200, Martin Liška wrote: >>>>>>>>>> Hello. >>>>>>>>>> >>>>>>>>>> This is patch that was originally installed by Jason and later reverted due to PR70422. >>>>>>>>>> In the later PR Richi suggested a fix for that and Segher verified that it helped him >>>>>>>>>> to survive regression tests. That's reason why I'm resending that. >>>>>>>>>> >>>>>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>>>>>>> >>>>>>>>>> Ready to be installed? >>>>>>>>>> Martin >>>>>>>>> >>>>>>>>>> >From a34ce0ef37ae00609c9f3ff98a9cb0b7db6a8bd0 Mon Sep 17 00:00:00 2001 >>>>>>>>>> From: marxin <mliska@suse.cz> >>>>>>>>>> Date: Thu, 20 Apr 2017 14:56:30 +0200 >>>>>>>>>> Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate >>>>>>>>>> symbol entry. >>>>>>>>>> >>>>>>>>>> gcc/cp/ChangeLog: >>>>>>>>>> >>>>>>>>>> 2017-04-20 Jason Merrill <jason@redhat.com> >>>>>>>>>> Martin Liska <mliska@suse.cz> >>>>>>>>>> Segher Boessenkool <segher@kernel.crashing.org> >>>>>>>>>> >>>>>>>>>> PR c++/64266 >>>>>>>>>> PR c++/70353 >>>>>>>>>> PR bootstrap/70422 >>>>>>>>>> Core issue 1962 >>>>>>>>>> * decl.c (cp_fname_init): Decay the initializer to pointer. >>>>>>>>>> (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, >>>>>>>>>> * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, >>>>>>>>>> DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and >>>>>>>>>> DECL_IGNORED_P. Don't call cp_finish_decl. >>>>>>>>> >>>>>>>>> If we don't emit those into the debug info, will the debugger be >>>>>>>>> able to handle __FUNCTION__ etc. properly? >>>>>>>> >>>>>>>> No, debugger with the patch can't handled these. Similar to how clang >>>>>>>> behaves currently. Maybe it can be conditionally enabled with -g3, or -g? >>>>>>>> >>>>>>>>> Admittedly, right now we emit it into debug info only if those decls >>>>>>>>> are actually used, say on: >>>>>>>>> const char * foo () { return __FUNCTION__; } >>>>>>>>> const char * bar () { return ""; } >>>>>>>>> we'd emit foo::__FUNCTION__, but not bar::__FUNCTION__, so the debugger >>>>>>>>> has to have some handling of it anyway. But while in functions >>>>>>>>> that don't refer to __FUNCTION__ it is always the debugger that needs >>>>>>>>> to synthetize those and thus they will be always pointer-equal, >>>>>>>>> if there are some uses of it and for other uses the debugger would >>>>>>>>> synthetize it, there is the possibility that the debugger synthetized >>>>>>>>> string will not be the same object as actually used in the function. >>>>>>>> >>>>>>>> You're right, currently one has to use a special function to be able to >>>>>>>> print it in debugger. I believe we've already discussed that, according >>>>>>>> to spec, the strings don't have to point to a same string. >>>>>>>> >>>>>>>> Suggestions what we should do with the patch? >>>>>>> >>>>>>> We need to emit debug information for these variables. From Jim's >>>>>>> description in 70422 it seems that the problem is that the reference >>>>>>> to the string from the debug information is breaking >>>>>>> function_mergeable_rodata_prefix, which relies on >>>>>>> current_function_decl. It seems to me that its callers should pass >>>>>>> along their decl parameter so that f_m_r_p can use the decl's >>>>>>> DECL_CONTEXT rather than rely on current_function_decl being set >>>>>>> properly> >>>>>>> Jason >>>>>>> >>>>>> >>>>>> Ok, after some time I returned back to it. I followed your advises and >>>>>> changed the function function_mergeable_rodata_prefix. Apart from a small >>>>>> rebase was needed. >>>>>> >>>>>> May I ask Jim to test the patch? >>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>>> >>>>>> + DECL_IGNORED_P (decl) = 1; >>>>> >>>>> As I said before, we do need to emit debug information for these variables, so this is wrong. >>>> >>>> Hello. >>>> >>>> Sorry for overlooking that. >>>> >>>>> >>>>>> - section *s = targetm.asm_out.function_rodata_section (current_function_decl); >>>>>> + tree decl = current_function_decl; >>>>>> + if (decl && DECL_CONTEXT (decl) >>>>>> + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) >>>>>> + decl = DECL_CONTEXT (decl); >>>>> >>>>> I don't see how this would help; it still relies on current_function_decl being set correctly, which was the problem we were running into before. >>>> >>>> I see, that's what I wanted to discuss on Cauldron with you, but eventually I did not find time. >>>> Well problem that I see is that we want to make decision based on DECL_CONTEXT(fname_decl), but mergeable_string_section >>>> is called with STRING_CST (which is VALUE_EXPR of created fname_decl): >>> [...] >>>> And idea how to resolve this? >>> >>> Well, when we're being called from the expander, current_function_decl is fine. >>> >>> Hmm, why would current_function_decl be wrong in dwarf2out? Can we fix that? >>> >>> Why does your change to function_mergeable_rodata_prefix make any difference? >> >> Going through unresolved issues I still have in ML, I noticed this one. As perfectly analyzed here: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c7 >> >> The issue is that we can't find proper function decl in dwarf2out.c because it's function declaration >> that was inlined. And such references are only seen in debug mode. >> >> The only solution I see is DECL_IGNORED_P, which was also suggested by Richi in: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70422#c9 >> >> Note that both LLVM and ICC don't allow: >> $ 3 __builtin_printf ("%s\n", __PRETTY_FUNCTION__); >> (gdb) p __PRETTY_FUNCTION__ >> No symbol "__PRETTY_FUNCTION__" in current context. >> >> Would it be feasible solution to ignore the declaration? > > It seems a rather user-unfriendly solution; being able to look at the > value of an identifier used in the program is a pretty basic > expectation for the debugging experience. Hello. I see, not that clang++ also does not emit debug info for a constexprs: (gdb) list 1 constexpr const char *myconstexpr = "function name"; 2 3 int main() 4 { 5 __builtin_printf (__PRETTY_FUNCTION__); 6 __builtin_printf (myconstexpr); 7 } (gdb) p myconstexpr $1 = <optimized out> > > In Jim's comment you mention, he says, "It seems like there are some > conflicting goals here. We want to share the string across functions, > but we also want to put it in function specific comdat sections." > > The recent discussion thread at > https://gcc.gnu.org/ml/gcc/2018-05/threads.html#00109 deals with a > very similar issue, of wanting to both merge constant data and > associate it with a function. The discussion there seems to be > tending toward merging rather than function-grouping, which seems to > match the desire in this PR. That's promising. > > Why do __*FUNCTION__ cause problems that > > constexpr const char *p = "function name"; > > doesn't? Difference is that for 'p' we do @object in assembly: .section .rodata .LC0: .string "function name" .align 8 .type _ZL11myconstexpr, @object .size _ZL11myconstexpr, 8 _ZL11myconstexpr: .quad .LC0 Motivation for the original patch was to remove need of doing a symbol. The issues I still see is how (and where) to assign to a string constant (of a __PRETTY_FUNCTION name) a section? Martin > > Jason >
From 8a7a8b421d81c47cf686175f309f79c0b7a8ce76 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Thu, 20 Apr 2017 14:56:30 +0200 Subject: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry. gcc/cp/ChangeLog: 2017-04-20 Jason Merrill <jason@redhat.com> Martin Liska <mliska@suse.cz> Segher Boessenkool <segher@kernel.crashing.org> PR c++/64266 PR c++/70353 PR bootstrap/70422 Core issue 1962 * decl.c (cp_fname_init): Decay the initializer to pointer. (cp_make_fname_decl): Set DECL_DECLARED_CONSTEXPR_P, * pt.c (tsubst_expr) [DECL_EXPR]: Set DECL_VALUE_EXPR, DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P and DECL_IGNORED_P. Don't call cp_finish_decl. * decl.c (cp_make_fname_decl): Add only non error mark expression. gcc/testsuite/ChangeLog: 2017-04-20 Jason Merrill <jason@redhat.com> Segher Boessenkool <segher@kernel.crashing.org> PR c++/64266 PR c++/70353 PR bootstrap/70422 Core issue 1962 * g++.dg/cpp0x/constexpr-__func__2.C: Add static assert test. * g++.dg/ext/fnname5.C: New test. * g++.old-deja/g++.ext/pretty4.C: Remove. gcc/ChangeLog: 2017-07-14 Martin Liska <mliska@suse.cz> * varasm.c (function_mergeable_rodata_prefix): Use DECL_CONTEXT for function declarations. --- gcc/cp/decl.c | 24 +++++-- gcc/cp/pt.c | 26 +++++--- gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C | 7 +- gcc/testsuite/g++.dg/ext/fnname5.C | 33 +++++++++ gcc/testsuite/g++.old-deja/g++.ext/pretty4.C | 85 ------------------------ gcc/varasm.c | 7 +- 6 files changed, 76 insertions(+), 106 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ext/fnname5.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.ext/pretty4.C diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 5b8e6a22b01..3f27b72052a 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -4294,13 +4294,15 @@ cp_fname_init (const char* name, tree *type_p) type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST); type = build_cplus_array_type (type, domain); - *type_p = type; + *type_p = type_decays_to (type); if (init) TREE_TYPE (init) = type; else init = error_mark_node; + init = decay_conversion (init, tf_warning_or_error); + return init; } @@ -4323,23 +4325,35 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep) if (name) free (CONST_CAST (char *, name)); - TREE_STATIC (decl) = 1; + /* As we're using pushdecl_with_scope, we must set the context. */ + DECL_CONTEXT (decl) = current_function_decl; + TREE_READONLY (decl) = 1; DECL_ARTIFICIAL (decl) = 1; + DECL_IGNORED_P (decl) = 1; + DECL_DECLARED_CONSTEXPR_P (decl) = 1; TREE_USED (decl) = 1; + if (init) + { + SET_DECL_VALUE_EXPR (decl, init); + DECL_HAS_VALUE_EXPR_P (decl) = 1; + /* For decl_constant_var_p. */ + DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1; + } + if (current_function_decl) { DECL_CONTEXT (decl) = current_function_decl; decl = pushdecl_outermost_localscope (decl); - cp_finish_decl (decl, init, /*init_const_expr_p=*/false, NULL_TREE, - LOOKUP_ONLYCONVERTING); + if (decl != error_mark_node) + add_decl_expr (decl); } else { DECL_THIS_STATIC (decl) = true; - pushdecl_top_level_and_finish (decl, init); + pushdecl_top_level_and_finish (decl, NULL_TREE); } return decl; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index bd02951cb85..f8e096b9b52 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -15862,21 +15862,25 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, DECL_CONTEXT (decl) = current_function_decl; cp_check_omp_declare_reduction (decl); } + else if (VAR_P (decl) + && DECL_PRETTY_FUNCTION_P (decl)) + { + /* For __PRETTY_FUNCTION__ we have to adjust the + initializer. */ + const char *const name + = cxx_printable_name (current_function_decl, 2); + init = cp_fname_init (name, &TREE_TYPE (decl)); + SET_DECL_VALUE_EXPR (decl, init); + DECL_HAS_VALUE_EXPR_P (decl) = 1; + DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1; + maybe_push_decl (decl); + } else { int const_init = false; maybe_push_decl (decl); - if (VAR_P (decl) - && DECL_PRETTY_FUNCTION_P (decl)) - { - /* For __PRETTY_FUNCTION__ we have to adjust the - initializer. */ - const char *const name - = cxx_printable_name (current_function_decl, 2); - init = cp_fname_init (name, &TREE_TYPE (decl)); - } - else - init = tsubst_init (init, decl, args, complain, in_decl); + + init = tsubst_init (init, decl, args, complain, in_decl); if (VAR_P (decl)) const_init = (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C index e6782905423..673fb4f3a93 100644 --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-__func__2.C @@ -1,5 +1,5 @@ // PR c++/70353 -// { dg-do link { target c++11 } } +// { dg-do compile { target c++11 } } constexpr const char* ce () { @@ -8,6 +8,5 @@ constexpr const char* ce () const char *c = ce(); -int main() -{ -} +#define SA(X) static_assert((X),#X) +SA(ce()[0] == 'c'); diff --git a/gcc/testsuite/g++.dg/ext/fnname5.C b/gcc/testsuite/g++.dg/ext/fnname5.C new file mode 100644 index 00000000000..04fa7d3f92d --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/fnname5.C @@ -0,0 +1,33 @@ +// PR c++/64266 +/* { dg-do compile } */ + +extern "C" int printf (const char *, ...); + +struct A +{ + void foo(int i) + { + printf ("__FUNCTION__ = %s\n", __FUNCTION__); + printf ("__PRETTY_FUNCTION__ = %s\n", __PRETTY_FUNCTION__); + } + + void foo() + { + printf ("__FUNCTION__ = %s\n", __FUNCTION__); + } +}; + +int +main () +{ + A a; + a.foo (0); + a.foo (); + return 0; +} + +/* { dg-final { scan-assembler-not "_ZZN1A3fooEvE12__FUNCTION__" } } */ +/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE12__FUNCTION__" } } */ +/* { dg-final { scan-assembler-not "_ZZN1A3fooEiE19__PRETTY_FUNCTION__" } } */ +/* { dg-final { scan-assembler ".(string|ascii)\\s+\"void A::foo\\(int\\)(.0)?\"" } } */ +/* { dg-final { scan-assembler ".(string|ascii)\\s+\"foo(.0)?\"" } } */ diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C deleted file mode 100644 index 9017d567132..00000000000 --- a/gcc/testsuite/g++.old-deja/g++.ext/pretty4.C +++ /dev/null @@ -1,85 +0,0 @@ -// { dg-do run } -// Copyright (C) 2000 Free Software Foundation, Inc. -// Contributed by Nathan Sidwell 3 Mar 2000 <nathan@codesourcery.com> - -// __PRETTY_FUNCTION__, __FUNCTION__ and __function__ should have the -// type char const [X], where X is the right value for that particular function - -static void const *strings[4]; -static void const *tpls[4]; -static unsigned pos = 0; -static int fail; -static void const *ptr = 0; - -void unover (char const (*)[5]) {} -void foo (char const (*)[5]) {} -void foo (void *) {fail = 1;} -void foo (void const *) {fail = 1;} -void baz (char const (&)[5]) {} - -template<unsigned I> void PV (char const (&objRef)[I]) -{ - strings[pos] = objRef; - tpls[pos] = __PRETTY_FUNCTION__; - pos++; -} - -void fn () -{ - PV (__FUNCTION__); - PV (__func__); - PV (__PRETTY_FUNCTION__); - PV ("wibble"); -} - -void baz () -{ - ptr = __FUNCTION__; - // there should be no string const merging - if (ptr == "baz") - fail = 1; - // but all uses should be the same. - if (ptr != __FUNCTION__) - fail = 1; -} -int baz (int) -{ - return ptr == __FUNCTION__; -} - -int main () -{ - // make sure we actually emit the VAR_DECL when needed, and things have the - // expected type. - foo (&__FUNCTION__); - baz (__FUNCTION__); - unover (&__FUNCTION__); - if (fail) - return 1; - - // __FUNCTION__ should be unique across functions with the same base name - // (it's a local static, _not_ a string). - baz (); - if (fail) - return 1; - if (baz (1)) - return 1; - fn (); - - // Check the names of fn. They should all be distinct strings (though two - // will have the same value). - if (strings[0] == strings[1]) - return 1; - if (strings[0] == strings[2]) - return 1; - if (strings[1] == strings[2]) - return 1; - - // check the names of the template functions so invoked - if (tpls[0] != tpls[1]) - return 1; - if (tpls[0] == tpls[2]) - return 1; - - return 0; -} diff --git a/gcc/varasm.c b/gcc/varasm.c index fbaebc1b5c0..0b56898d8f3 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -760,7 +760,12 @@ default_no_function_rodata_section (tree decl ATTRIBUTE_UNUSED) static const char * function_mergeable_rodata_prefix (void) { - section *s = targetm.asm_out.function_rodata_section (current_function_decl); + tree decl = current_function_decl; + if (decl && DECL_CONTEXT (decl) + && TREE_CODE (DECL_CONTEXT (decl)) == FUNCTION_DECL) + decl = DECL_CONTEXT (decl); + + section *s = targetm.asm_out.function_rodata_section (decl); if (SECTION_STYLE (s) == SECTION_NAMED) return s->named.name; else -- 2.13.2