* [PATCH] Record that -gtoggle is already used in gcc_options.
@ 2021-11-02 14:10 Martin Liška
2021-11-02 14:33 ` Richard Biener
2021-11-02 22:49 ` Andrew Pinski
0 siblings, 2 replies; 10+ messages in thread
From: Martin Liška @ 2021-11-02 14:10 UTC (permalink / raw)
To: gcc-patches
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
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-02 14:10 [PATCH] Record that -gtoggle is already used in gcc_options Martin Liška
@ 2021-11-02 14:33 ` Richard Biener
2021-11-02 15:11 ` Martin Liška
2021-11-02 22:49 ` Andrew Pinski
1 sibling, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-11-02 14:33 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-02 14:33 ` Richard Biener
@ 2021-11-02 15:11 ` Martin Liška
2021-11-02 16:45 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-11-02 15:11 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-02 15:11 ` Martin Liška
@ 2021-11-02 16:45 ` Richard Biener
2021-11-04 12:51 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-11-02 16:45 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-02 16:45 ` Richard Biener
@ 2021-11-04 12:51 ` Martin Liška
2021-11-04 13:09 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-11-04 12:51 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-04 12:51 ` Martin Liška
@ 2021-11-04 13:09 ` Richard Biener
2021-11-04 15:11 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-11-04 13:09 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-04 13:09 ` Richard Biener
@ 2021-11-04 15:11 ` Martin Liška
2021-11-05 10:23 ` Richard Biener
0 siblings, 1 reply; 10+ messages in thread
From: Martin Liška @ 2021-11-04 15:11 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
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
[-- Attachment #2: 0001-Reset-when-gtoggle-is-used-in-gcc_options.patch --]
[-- Type: text/x-patch, Size: 1308 bytes --]
From b458389805f3d0677b04850d95990a88df40806f Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Mon, 1 Nov 2021 16:28:34 +0100
Subject: [PATCH] Reset when -gtoggle is used in gcc_options.
PR debug/102955
gcc/ChangeLog:
* opts.c (finish_options): Reset flag_gtoggle when it is used.
gcc/testsuite/ChangeLog:
* g++.dg/pr102955.C: New test.
---
gcc/opts.c | 1 +
gcc/testsuite/g++.dg/pr102955.C | 14 ++++++++++++++
2 files changed, 15 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/pr102955.C
diff --git a/gcc/opts.c b/gcc/opts.c
index 3f80fce82bc..442e6597b63 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1377,6 +1377,7 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
if (flag_gtoggle)
{
+ flag_gtoggle = false;
if (debug_info_level == DINFO_LEVEL_NONE)
{
debug_info_level = DINFO_LEVEL_NORMAL;
diff --git a/gcc/testsuite/g++.dg/pr102955.C b/gcc/testsuite/g++.dg/pr102955.C
new file mode 100644
index 00000000000..de9689edec4
--- /dev/null
+++ b/gcc/testsuite/g++.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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-04 15:11 ` Martin Liška
@ 2021-11-05 10:23 ` Richard Biener
2021-11-05 12:01 ` Martin Liška
0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2021-11-05 10:23 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Record that -gtoggle is already used in gcc_options.
2021-11-02 14:10 [PATCH] Record that -gtoggle is already used in gcc_options Martin Liška
2021-11-02 14:33 ` Richard Biener
@ 2021-11-02 22:49 ` Andrew Pinski
1 sibling, 0 replies; 10+ messages in thread
From: Andrew Pinski @ 2021-11-02 22:49 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-05 12:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02 14:10 [PATCH] Record that -gtoggle is already used in gcc_options Martin Liška
2021-11-02 14:33 ` Richard Biener
2021-11-02 15:11 ` Martin Liška
2021-11-02 16:45 ` Richard Biener
2021-11-04 12:51 ` Martin Liška
2021-11-04 13:09 ` Richard Biener
2021-11-04 15:11 ` Martin Liška
2021-11-05 10:23 ` Richard Biener
2021-11-05 12:01 ` Martin Liška
2021-11-02 22:49 ` Andrew Pinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).