Message ID | 435f9ee2-88c6-392e-3584-2337dd5dd9c1@suse.cz |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1F84B3857C6A for <patchwork@sourceware.org>; Tue, 2 Nov 2021 14:11:00 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id E9FA93858D35 for <gcc-patches@gcc.gnu.org>; Tue, 2 Nov 2021 14:10:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E9FA93858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id E1EF6212C6 for <gcc-patches@gcc.gnu.org>; Tue, 2 Nov 2021 14:10:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1635862242; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=NK9uJ58fM1alE3GW45YUzYODRZ52Y8A7mpisvCN+JNs=; b=ICfzqEvN53w7znOvLALNuIgDfQdmoMOm8GPqzV+IdShGioXoR3X8/us+NHPj9lsPoLAPTi T/CCtsM5ha2G6fOo8ykFS4GK+OHi8KlHCjCfVyg2ethnum/lIGt8Mupi0WLACz3WSMxdnB Qp36jCgenmd3uaywvaBEzzW2d08kb1Y= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1635862242; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=NK9uJ58fM1alE3GW45YUzYODRZ52Y8A7mpisvCN+JNs=; b=qYJrndpvleTikxd4R6tPYih79Tc96eqMAUldUhuVigm1WS4mcfEh8LHG2B5Ngq3iVV700q ibpzabxJ3KldloCg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id D26B013BF7 for <gcc-patches@gcc.gnu.org>; Tue, 2 Nov 2021 14:10:42 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id OZpbMuJGgWHrIwAAMHmgww (envelope-from <mliska@suse.cz>) for <gcc-patches@gcc.gnu.org>; Tue, 02 Nov 2021 14:10:42 +0000 Message-ID: <435f9ee2-88c6-392e-3584-2337dd5dd9c1@suse.cz> Date: Tue, 2 Nov 2021 15:10:42 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 From: =?utf-8?q?Martin_Li=C5=A1ka?= <mliska@suse.cz> Subject: [PATCH] Record that -gtoggle is already used in gcc_options. To: gcc-patches@gcc.gnu.org Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
Record that -gtoggle is already used in gcc_options.
|
|
Commit Message
Martin Liška
Nov. 2, 2021, 2:10 p.m. UTC
Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin When doing flip based on -gtoggle, record it. Otherwise, we will apply it for the second time in finish_options. PR debug/102955 gcc/ChangeLog: * common.opt: Add new gtoggle_used variable. * opts.c (finish_options): Do not interpret flag_gtoggle twice. gcc/testsuite/ChangeLog: * gcc.dg/pr102955.c: New test. --- gcc/common.opt | 4 ++++ gcc/opts.c | 3 ++- gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/pr102955.c
Comments
On Tue, Nov 2, 2021 at 3:11 PM Martin Liška <mliska@suse.cz> wrote: > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? I think -gtoggle matches a Defered option and thus should be processed in handle_common_deferred_options. I'd argue that --help printing shouldn't be part of decode_options but done only in toplev.c (not from the parse_optimize_options caller). Richard. > Thanks, > Martin > > When doing flip based on -gtoggle, record it. Otherwise, we will > apply it for the second time in finish_options. > > PR debug/102955 > > gcc/ChangeLog: > > * common.opt: Add new gtoggle_used variable. > * opts.c (finish_options): Do not interpret flag_gtoggle twice. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr102955.c: New test. > --- > gcc/common.opt | 4 ++++ > gcc/opts.c | 3 ++- > gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/pr102955.c > > diff --git a/gcc/common.opt b/gcc/common.opt > index 1a5b9bfcca9..2568ecb98b8 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -3316,6 +3316,10 @@ gdescribe-dies > Common Driver Var(flag_describe_dies) Init(0) > Add description attributes to some DWARF DIEs that have no name attribute. > > +; True if -gtoggle option was already handled. > +Variable > +bool gtoggle_used > + > gtoggle > Common Driver Var(flag_gtoggle) > Toggle debug information generation. > diff --git a/gcc/opts.c b/gcc/opts.c > index 3f80fce82bc..ef38b8dbab0 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, > profile_flag = 0; > } > > - if (flag_gtoggle) > + if (flag_gtoggle && !gtoggle_used) > { > + gtoggle_used = true; > if (debug_info_level == DINFO_LEVEL_NONE) > { > debug_info_level = DINFO_LEVEL_NORMAL; > diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c > new file mode 100644 > index 00000000000..de9689edec4 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr102955.c > @@ -0,0 +1,14 @@ > +/* PR debug/102955 */ > +/* { dg-do compile } */ > +/* { dg-options "-g -gtoggle" } */ > + > +#pragma GCC optimize "0" > +struct j > +{ > + explicit j (); > + ~j (); > +}; > +void g (void) > +{ > + new j(); > +} > -- > 2.33.1 >
On 11/2/21 15:33, Richard Biener wrote: > I think -gtoggle matches a Defered option and thus should be processed > in handle_common_deferred_options. Well, that's quite problematic as I handle_common_deferred_options is called after decode_options (that calls finish_options). Note there's direct dependency at very end of finish_options in between -gtoggle and debug_nonbind_markers_p: if (flag_gtoggle) { if (debug_info_level == DINFO_LEVEL_NONE) { debug_info_level = DINFO_LEVEL_NORMAL; if (write_symbols == NO_DEBUG) write_symbols = PREFERRED_DEBUGGING_TYPE; } else debug_info_level = DINFO_LEVEL_NONE; } if (!OPTION_SET_P (debug_nonbind_markers_p)) debug_nonbind_markers_p = (optimize && debug_info_level >= DINFO_LEVEL_NORMAL && dwarf_debuginfo_p () && !(flag_selective_scheduling || flag_selective_scheduling2)); I don't see who you mean the possible fix? Martin
On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote: > > On 11/2/21 15:33, Richard Biener wrote: > > I think -gtoggle matches a Defered option and thus should be processed > > in handle_common_deferred_options. > > Well, that's quite problematic as I handle_common_deferred_options is called > after decode_options (that calls finish_options). > > Note there's direct dependency at very end of finish_options in between -gtoggle > and debug_nonbind_markers_p: > > > if (flag_gtoggle) > { > if (debug_info_level == DINFO_LEVEL_NONE) > { > debug_info_level = DINFO_LEVEL_NORMAL; > > if (write_symbols == NO_DEBUG) > write_symbols = PREFERRED_DEBUGGING_TYPE; > } > else > debug_info_level = DINFO_LEVEL_NONE; > } > > if (!OPTION_SET_P (debug_nonbind_markers_p)) > debug_nonbind_markers_p > = (optimize > && debug_info_level >= DINFO_LEVEL_NORMAL > && dwarf_debuginfo_p () > && !(flag_selective_scheduling || flag_selective_scheduling2)); > > I don't see who you mean the possible fix? So at first I thought we might have a place that post-processes 'decoded_options' so we could reflect -gtoggle on those but out-of-order (removing/adding -g). But that's going to be mightly complicated as well. I wonder what the original issue is you fix? You say we ap;ly it for a second time but we should apply it onto the same state as previously since we restore that for optimize attribute processing? Richard. > > Martin
On Tue, Nov 2, 2021 at 7:11 AM Martin Liška <mliska@suse.cz> wrote: > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin > > When doing flip based on -gtoggle, record it. Otherwise, we will > apply it for the second time in finish_options. > > PR debug/102955 > > gcc/ChangeLog: > > * common.opt: Add new gtoggle_used variable. > * opts.c (finish_options): Do not interpret flag_gtoggle twice. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr102955.c: New test. Hmm, this is a C++ testcase so shouldn't it be at g++.dg/pr102955.C ? Thanks, Andrew Pinski > --- > gcc/common.opt | 4 ++++ > gcc/opts.c | 3 ++- > gcc/testsuite/gcc.dg/pr102955.c | 14 ++++++++++++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.dg/pr102955.c > > diff --git a/gcc/common.opt b/gcc/common.opt > index 1a5b9bfcca9..2568ecb98b8 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -3316,6 +3316,10 @@ gdescribe-dies > Common Driver Var(flag_describe_dies) Init(0) > Add description attributes to some DWARF DIEs that have no name attribute. > > +; True if -gtoggle option was already handled. > +Variable > +bool gtoggle_used > + > gtoggle > Common Driver Var(flag_gtoggle) > Toggle debug information generation. > diff --git a/gcc/opts.c b/gcc/opts.c > index 3f80fce82bc..ef38b8dbab0 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, > profile_flag = 0; > } > > - if (flag_gtoggle) > + if (flag_gtoggle && !gtoggle_used) > { > + gtoggle_used = true; > if (debug_info_level == DINFO_LEVEL_NONE) > { > debug_info_level = DINFO_LEVEL_NORMAL; > diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c > new file mode 100644 > index 00000000000..de9689edec4 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr102955.c > @@ -0,0 +1,14 @@ > +/* PR debug/102955 */ > +/* { dg-do compile } */ > +/* { dg-options "-g -gtoggle" } */ > + > +#pragma GCC optimize "0" > +struct j > +{ > + explicit j (); > + ~j (); > +}; > +void g (void) > +{ > + new j(); > +} > -- > 2.33.1 >
On 11/2/21 17:45, Richard Biener wrote: > On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote: >> >> On 11/2/21 15:33, Richard Biener wrote: >>> I think -gtoggle matches a Defered option and thus should be processed >>> in handle_common_deferred_options. >> >> Well, that's quite problematic as I handle_common_deferred_options is called >> after decode_options (that calls finish_options). >> >> Note there's direct dependency at very end of finish_options in between -gtoggle >> and debug_nonbind_markers_p: >> >> >> if (flag_gtoggle) >> { >> if (debug_info_level == DINFO_LEVEL_NONE) >> { >> debug_info_level = DINFO_LEVEL_NORMAL; >> >> if (write_symbols == NO_DEBUG) >> write_symbols = PREFERRED_DEBUGGING_TYPE; >> } >> else >> debug_info_level = DINFO_LEVEL_NONE; >> } >> >> if (!OPTION_SET_P (debug_nonbind_markers_p)) >> debug_nonbind_markers_p >> = (optimize >> && debug_info_level >= DINFO_LEVEL_NORMAL >> && dwarf_debuginfo_p () >> && !(flag_selective_scheduling || flag_selective_scheduling2)); >> >> I don't see who you mean the possible fix? > > So at first I thought we might have a place that post-processes > 'decoded_options' so we could reflect -gtoggle on those but > out-of-order (removing/adding -g). But that's going to be mightly > complicated as well. That would be very complicated. > > I wonder what the original issue is you fix? You say we ap;ly > it for a second time but we should apply it onto the same > state as previously since we restore that for optimize attribute > processing? Well, finish_options is always called once we parse options and we want to finalize them. So that happens from toplev where we create initial global options. Later on, after the pragma is parsed (where we start with current global options), the finish_options is called. Problem of -gtoggle is that it does not directly influence an option, but it negates it. That said, I think my patch with gtoggle_used is a reasonable workaround. Cheers, Martin > > Richard. > >> >> Martin
On Thu, Nov 4, 2021 at 1:51 PM Martin Liška <mliska@suse.cz> wrote: > > On 11/2/21 17:45, Richard Biener wrote: > > On Tue, Nov 2, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote: > >> > >> On 11/2/21 15:33, Richard Biener wrote: > >>> I think -gtoggle matches a Defered option and thus should be processed > >>> in handle_common_deferred_options. > >> > >> Well, that's quite problematic as I handle_common_deferred_options is called > >> after decode_options (that calls finish_options). > >> > >> Note there's direct dependency at very end of finish_options in between -gtoggle > >> and debug_nonbind_markers_p: > >> > >> > >> if (flag_gtoggle) > >> { > >> if (debug_info_level == DINFO_LEVEL_NONE) > >> { > >> debug_info_level = DINFO_LEVEL_NORMAL; > >> > >> if (write_symbols == NO_DEBUG) > >> write_symbols = PREFERRED_DEBUGGING_TYPE; > >> } > >> else > >> debug_info_level = DINFO_LEVEL_NONE; > >> } > >> > >> if (!OPTION_SET_P (debug_nonbind_markers_p)) > >> debug_nonbind_markers_p > >> = (optimize > >> && debug_info_level >= DINFO_LEVEL_NORMAL > >> && dwarf_debuginfo_p () > >> && !(flag_selective_scheduling || flag_selective_scheduling2)); > >> > >> I don't see who you mean the possible fix? > > > > So at first I thought we might have a place that post-processes > > 'decoded_options' so we could reflect -gtoggle on those but > > out-of-order (removing/adding -g). But that's going to be mightly > > complicated as well. > > That would be very complicated. > > > > > I wonder what the original issue is you fix? You say we ap;ly > > it for a second time but we should apply it onto the same > > state as previously since we restore that for optimize attribute > > processing? > > Well, finish_options is always called once we parse options and we want to finalize them. > So that happens from toplev where we create initial global options. Later on, after the pragma > is parsed (where we start with current global options), the finish_options is called. But we shouldn't start with the current global options but with ones we saved for optimize attribute/pragma processing, no? > Problem of -gtoggle is that it does not directly influence an option, but it negates it. > > That said, I think my patch with gtoggle_used is a reasonable workaround. Well, then we could as well unset flag_gtoggle after processing it, no? Thanks, Richard. > Cheers, > Martin > > > > > Richard. > > > >> > >> Martin >
On 11/4/21 14:09, Richard Biener wrote: > But we shouldn't start with the current global options but with ones > we saved for > optimize attribute/pragma processing, no? We hit the issue when we combine cmdline and pragma optimize options. > >> Problem of -gtoggle is that it does not directly influence an option, but it negates it. >> >> That said, I think my patch with gtoggle_used is a reasonable workaround. > Well, then we could as well unset flag_gtoggle after processing it, no? Yeah, that work! :) Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Ready to be installed? Thanks, Martin
On Thu, Nov 4, 2021 at 4:11 PM Martin Liška <mliska@suse.cz> wrote: > > On 11/4/21 14:09, Richard Biener wrote: > > But we shouldn't start with the current global options but with ones > > we saved for > > optimize attribute/pragma processing, no? > > We hit the issue when we combine cmdline and pragma optimize options. > > > > >> Problem of -gtoggle is that it does not directly influence an option, but it negates it. > >> > >> That said, I think my patch with gtoggle_used is a reasonable workaround. > > Well, then we could as well unset flag_gtoggle after processing it, no? > > Yeah, that work! :) > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? OK if you add a comment like /* Make sure to process -gtoggle only once. */ Richard. > Thanks, > Martin
On 11/5/21 11:23, Richard Biener wrote: > OK if you add a comment like > > /* Make sure to process -gtoggle only once. */ Sure, added and installed as 14c7041a1f00ef4ee9a036e0b369c97646db5b5c. Cheers, Martin
diff --git a/gcc/common.opt b/gcc/common.opt index 1a5b9bfcca9..2568ecb98b8 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -3316,6 +3316,10 @@ gdescribe-dies Common Driver Var(flag_describe_dies) Init(0) Add description attributes to some DWARF DIEs that have no name attribute. +; True if -gtoggle option was already handled. +Variable +bool gtoggle_used + gtoggle Common Driver Var(flag_gtoggle) Toggle debug information generation. diff --git a/gcc/opts.c b/gcc/opts.c index 3f80fce82bc..ef38b8dbab0 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1375,8 +1375,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set, profile_flag = 0; } - if (flag_gtoggle) + if (flag_gtoggle && !gtoggle_used) { + gtoggle_used = true; if (debug_info_level == DINFO_LEVEL_NONE) { debug_info_level = DINFO_LEVEL_NORMAL; diff --git a/gcc/testsuite/gcc.dg/pr102955.c b/gcc/testsuite/gcc.dg/pr102955.c new file mode 100644 index 00000000000..de9689edec4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr102955.c @@ -0,0 +1,14 @@ +/* PR debug/102955 */ +/* { dg-do compile } */ +/* { dg-options "-g -gtoggle" } */ + +#pragma GCC optimize "0" +struct j +{ + explicit j (); + ~j (); +}; +void g (void) +{ + new j(); +}