From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36979 invoked by alias); 4 Jun 2018 12:11:04 -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 35427 invoked by uid 89); 4 Jun 2018 12:11:03 -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,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.2 spammy=survive, deals, ptc, UD:pt.c X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Jun 2018 12:11:01 +0000 Received: from relay1.suse.de (charybdis-ext-too.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C7BA7AE6C; Mon, 4 Jun 2018 12:10:58 +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> <894565dc-5c64-cb9f-8bb4-35ee7035c075@suse.cz> <47548cbc-69c9-d925-a107-891a960a2cd1@suse.cz> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: <714daf75-8c45-2503-23cc-675a7c258485@suse.cz> Date: Mon, 04 Jun 2018 12:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00128.txt.bz2 On 05/21/2018 07:19 PM, Jason Merrill wrote: > On Mon, May 21, 2018 at 9:33 AM, Martin Liška wrote: >> On 10/24/2017 10:24 PM, Jason Merrill wrote: >>> On Thu, Sep 14, 2017 at 5:22 AM, Martin Liška 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 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): >>> [...] >>>> 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 = > > 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 >