From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75555 invoked by alias); 14 Sep 2017 09:22:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 75489 invoked by uid 89); 14 Sep 2017 09:22:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=1962 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 Sep 2017 09:22:21 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id B027EABDB; Thu, 14 Sep 2017 09:22:18 +0000 (UTC) Subject: Re: [PATCH] Make __FUNCTION__ a mergeable string and do not generate symbol entry. To: Jason Merrill Cc: Jakub Jelinek , Pedro Alves , Jan Kratochvil , GCC Patches , Segher Boessenkool , wilson@gcc.gnu.org References: <20170425115827.GX1809@tucnak> <3b197eef-db36-7df9-417e-e17be0616b6c@suse.cz> <3f912b8a-0153-f559-e3fe-489b57800697@redhat.com> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <894565dc-5c64-cb9f-8bb4-35ee7035c075@suse.cz> Date: Thu, 14 Sep 2017 09:22:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <3f912b8a-0153-f559-e3fe-489b57800697@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00875.txt.bz2 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 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 >>>>>> 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  >>>>>>           Martin Liska  >>>>>>           Segher Boessenkool  >>>>>> >>>>>>       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) unit-size align:8 warn_if_not_align:0 symtab:-1408509840 alias-set -1 canonical-type 0x2aaaac0c15e8 precision:8 min max pointer_to_this > SI size unit-size align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c4348 domain type_6 DI size unit-size align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x2aaaac2c41f8 precision:64 min max > pointer_to_this > constant "foo\000"> And idea how to resolve this? Thank you, Martin > > Jason