diff mbox series

lto: pass through -funwind-tables and -fasynchronous-unwind-tables

Message ID mvmlem0ywoy.fsf@suse.de
State New
Headers show
Series lto: pass through -funwind-tables and -fasynchronous-unwind-tables | expand

Commit Message

Andreas Schwab Jan. 18, 2023, 10:16 a.m. UTC 
The -funwind-tables and -fasynchronous-unwind-tables options are relevant
for the output pass, thus they need to be passed through by the lto
wrapper.

gcc/
	* lto-wrapper.cc (merge_and_complain): Pass through
	-funwind-tables and -fasynchronous-unwind-tables.
	(append_compiler_options): Likewise.
---
 gcc/lto-wrapper.cc | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Richard Biener Jan. 18, 2023, 11:05 a.m. UTC | #1 
On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
> for the output pass, thus they need to be passed through by the lto
> wrapper.

They are already stored per function, and ...

> gcc/
>         * lto-wrapper.cc (merge_and_complain): Pass through
>         -funwind-tables and -fasynchronous-unwind-tables.
>         (append_compiler_options): Likewise.
> ---
>  gcc/lto-wrapper.cc | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
> index 11c4d1b38a4..627e8238606 100644
> --- a/gcc/lto-wrapper.cc
> +++ b/gcc/lto-wrapper.cc
> @@ -314,6 +314,8 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
>         case OPT_fshow_column:
>         case OPT_fcommon:
>         case OPT_fgnu_tm:
> +       case OPT_funwind_tables:
> +       case OPT_fasynchronous_unwind_tables:

this would pick -fno-unwind-tables if picked up first?

If in global/IPA context the setting of flag_unwind_tables matters
then should we
compute it in lto1 from the set of functions in the partition instead?
 Thus enable
it when the user requested unwind tables from at least one function?

Handling of options in lto-wrapper that are marked Optimization and thus
streamed per function is somewhat dubious.

What exactly are you fixing?

Richard.

>         case OPT_g:
>           /* Do what the old LTO code did - collect exactly one option
>              setting per OPT code, we pick the first we encounter.
> @@ -737,6 +739,8 @@ append_compiler_options (obstack *argv_obstack, vec<cl_decoded_option> opts)
>         case OPT_fopenacc_dim_:
>         case OPT_foffload_abi_:
>         case OPT_fcf_protection_:
> +       case OPT_funwind_tables:
> +       case OPT_fasynchronous_unwind_tables:
>         case OPT_g:
>         case OPT_O:
>         case OPT_Ofast:
> --
> 2.39.1
>
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Andreas Schwab Jan. 18, 2023, 11:25 a.m. UTC | #2 
On Jan 18 2023, Richard Biener wrote:

> On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
>> for the output pass, thus they need to be passed through by the lto
>> wrapper.
>
> They are already stored per function, and ...

Are they?  Are you sure you don't confuse that with -fexceptions?

> What exactly are you fixing?

Making -funwind-tables effective in LTO mode.
Jakub Jelinek Jan. 18, 2023, 11:31 a.m. UTC | #3 
On Wed, Jan 18, 2023 at 12:25:11PM +0100, Andreas Schwab via Gcc-patches wrote:
> On Jan 18 2023, Richard Biener wrote:
> 
> > On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
> >> for the output pass, thus they need to be passed through by the lto
> >> wrapper.
> >
> > They are already stored per function, and ...
> 
> Are they?  Are you sure you don't confuse that with -fexceptions?

They clearly are:
fasynchronous-unwind-tables
Common Var(flag_asynchronous_unwind_tables) Optimization
Generate unwind tables that are exact at each instruction boundary.
and
funwind-tables
Common Var(flag_unwind_tables) Optimization
Just generate unwind tables for exception handling.

The Optimization keyword is what implies that, as documented:
'PerFunction'
     This is an option that can be overridden on a per-function basis.
     'Optimization' implies 'PerFunction', but options that do not
     affect executable code generation may use this flag instead, so
     that the option is not taken into account in ways that might affect
     executable code generation.

	Jakub
Andreas Schwab Jan. 18, 2023, 12:30 p.m. UTC | #4 
On Jan 18 2023, Jakub Jelinek wrote:

> On Wed, Jan 18, 2023 at 12:25:11PM +0100, Andreas Schwab via Gcc-patches wrote:
>> On Jan 18 2023, Richard Biener wrote:
>> 
>> > On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
>> > <gcc-patches@gcc.gnu.org> wrote:
>> >>
>> >> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
>> >> for the output pass, thus they need to be passed through by the lto
>> >> wrapper.
>> >
>> > They are already stored per function, and ...
>> 
>> Are they?  Are you sure you don't confuse that with -fexceptions?
>
> They clearly are:
> fasynchronous-unwind-tables
> Common Var(flag_asynchronous_unwind_tables) Optimization
> Generate unwind tables that are exact at each instruction boundary.
> and
> funwind-tables
> Common Var(flag_unwind_tables) Optimization
> Just generate unwind tables for exception handling.

How is that supposed to work then?
Jakub Jelinek Jan. 18, 2023, 12:35 p.m. UTC | #5 
On Wed, Jan 18, 2023 at 01:30:53PM +0100, Andreas Schwab wrote:
> On Jan 18 2023, Jakub Jelinek wrote:
> 
> > On Wed, Jan 18, 2023 at 12:25:11PM +0100, Andreas Schwab via Gcc-patches wrote:
> >> On Jan 18 2023, Richard Biener wrote:
> >> 
> >> > On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
> >> > <gcc-patches@gcc.gnu.org> wrote:
> >> >>
> >> >> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
> >> >> for the output pass, thus they need to be passed through by the lto
> >> >> wrapper.
> >> >
> >> > They are already stored per function, and ...
> >> 
> >> Are they?  Are you sure you don't confuse that with -fexceptions?
> >
> > They clearly are:
> > fasynchronous-unwind-tables
> > Common Var(flag_asynchronous_unwind_tables) Optimization
> > Generate unwind tables that are exact at each instruction boundary.
> > and
> > funwind-tables
> > Common Var(flag_unwind_tables) Optimization
> > Just generate unwind tables for exception handling.
> 
> How is that supposed to work then?

With LTO each function has the DECL_FUNCTION_SPECIFIC_OPTIMIZATION
(and _TARGET), for functions with optimize attribute obviously as without
LTO specific to what options have been overridden (but with defaults from
TU's command line etc.), for functions without that simply with what
options has the TU.
lto1 then streams in those options and when switching functions switches
the global options.

	Jakub
Andreas Schwab Jan. 18, 2023, 12:39 p.m. UTC | #6 
On Jan 18 2023, Jakub Jelinek wrote:

> With LTO each function has the DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> (and _TARGET), for functions with optimize attribute obviously as without
> LTO specific to what options have been overridden (but with defaults from
> TU's command line etc.), for functions without that simply with what
> options has the TU.

Sorry, I cannot parse that sentence.  Could you please try again?

> lto1 then streams in those options and when switching functions switches
> the global options.

Why does that not work then?
Jakub Jelinek Jan. 18, 2023, 12:47 p.m. UTC | #7 
On Wed, Jan 18, 2023 at 01:39:18PM +0100, Andreas Schwab wrote:
> On Jan 18 2023, Jakub Jelinek wrote:
> 
> > With LTO each function has the DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> > (and _TARGET), for functions with optimize attribute obviously as without
> > LTO specific to what options have been overridden (but with defaults from
> > TU's command line etc.), for functions without that simply with what
> > options has the TU.
> 
> Sorry, I cannot parse that sentence.  Could you please try again?

After parsing options GCC creates an OPTIMIZATION_NODE tree with all the
PerFunction/Optimization option state recorded in it (and more).
That node is then when writing LTO attached to each function (different
node if a function has optimize attribute etc.).
That is streamed in by lto1 back and on each set_cfun such saved options
are stored into global_options{,_set}.

> > lto1 then streams in those options and when switching functions switches
> > the global options.
> 
> Why does that not work then?

I don't know what doesn't work.  You haven't mentioned what kind of PR
you're trying to fix or what problem you are seeing.

	Jakub
Andreas Schwab Jan. 18, 2023, 1 p.m. UTC | #8 
No unwind tables are generated, as if -funwind-tables is ignored.  If
LTO is disabled, everything works as expected.
Andreas Schwab Jan. 18, 2023, 1:36 p.m. UTC | #9 
On Jan 18 2023, Jakub Jelinek wrote:

> That is streamed in by lto1 back and on each set_cfun such saved options
> are stored into global_options{,_set}.

Is that done in time for dwarf2out_do_eh_frame?
Michael Matz Jan. 18, 2023, 2:03 p.m. UTC | #10 
On Wed, 18 Jan 2023, Andreas Schwab via Gcc-patches wrote:

> No unwind tables are generated, as if -funwind-tables is ignored.  If
> LTO is disabled, everything works as expected.

On Risc-V btw.  (which, unlike i386,aarch64,s390,rs6000 does not 
effectively enable funwind-tables always via either backend or 
common/config/$arch/ code, which is the reason why the problem can't be 
seen there).  It's an interaction with -g :

The problem (with cross compiler here, but shouldn't matter):

% riscv64-gcc -g -flto -funwind-tables -fPIC -c hello.c
% riscv64-gcc -shared hello.o
% readelf -wF a.out
... empty .eh_frame ...
Contents of the .debug_frame section:
... content ...

Using a compiler for any of the above archs makes this work (working means 
that the unwind info is placed into .eh_frame, _not_ into .debug_frame).  
Not using -g makes it work.  Adding -funwind-tables to the link command 
makes it work.  Adding -fexceptions to the compile command makes it work.  
Not using LTO makes it work.

So, it's quite clear that the option merging algorithm related to all this 
is somewhat broken, the global (or per function, or whatever) 
-funwind-tables option from hello.o doesn't make it correctly into the 
output (when -g is there).  Adding -fexception makes it work because then 
the functions will have personalities and on LTO-read-in _that_ will 
implicitely enable funwind-tables again (which should have been enabled 
already by the option-read-in).

As written initially the other archs are working because they all have 
various ways of essentially setting flag_unwind_tables always:

i386 via common/config/i386/i386-common.cc
   opts->x_flag_asynchronous_unwind_tables = 2;
  config/i386/i386-options.cc
         if (opts->x_flag_asynchronous_unwind_tables == 2)
           opts->x_flag_unwind_tables
             = opts->x_flag_asynchronous_unwind_tables = 1;

rs6000 via common/config/rs6000/rs6000-common.cc
   #ifdef OBJECT_FORMAT_ELF
     opts->x_flag_asynchronous_unwind_tables = 1;
   #endif
  (which ultimately also enabled flag_unwind_tables)

s390 via common/config/s390/s390-common.cc
    opts->x_flag_asynchronous_unwind_tables = 1;

aarch64 via common/config/aarch64/aarch64-common.cc
  #if (TARGET_DEFAULT_ASYNC_UNWIND_TABLES == 1)
    { OPT_LEVELS_ALL, OPT_fasynchronous_unwind_tables, NULL, 1 },
    { OPT_LEVELS_ALL, OPT_funwind_tables, NULL, 1},
  #endif

  (the #define here is set for aarch64*-*-linux* )

So the problem cannot be readily demonstrated on these architectures.  
risc-v has no such code (and doesn't need to).


Ciao,
Michael.
Andreas Schwab Jan. 18, 2023, 2:14 p.m. UTC | #11 
On Jan 18 2023, Michael Matz wrote:

> So, it's quite clear that the option merging algorithm related to all this 
> is somewhat broken, the global (or per function, or whatever) 
> -funwind-tables option from hello.o doesn't make it correctly into the 
> output (when -g is there).  Adding -fexception makes it work because then 
> the functions will have personalities and on LTO-read-in _that_ will 
> implicitely enable funwind-tables again (which should have been enabled 
> already by the option-read-in).

My guess is that flag_unwind_tables is not yet set when .cfi_sections is
emitted (which is done by dwarf2out_assembly_start before compile starts).
Jakub Jelinek Jan. 18, 2023, 2:25 p.m. UTC | #12 
On Wed, Jan 18, 2023 at 03:14:01PM +0100, Andreas Schwab wrote:
> On Jan 18 2023, Michael Matz wrote:
> 
> > So, it's quite clear that the option merging algorithm related to all this 
> > is somewhat broken, the global (or per function, or whatever) 
> > -funwind-tables option from hello.o doesn't make it correctly into the 
> > output (when -g is there).  Adding -fexception makes it work because then 
> > the functions will have personalities and on LTO-read-in _that_ will 
> > implicitely enable funwind-tables again (which should have been enabled 
> > already by the option-read-in).
> 
> My guess is that flag_unwind_tables is not yet set when .cfi_sections is
> emitted (which is done by dwarf2out_assembly_start before compile starts).

Well, the primary question for PerFunction/Optimization flag is what such
flag means outside of any function.
Because with such flags, it no longer is everything wants unwind tables (or
asynchronous unwind tables), but perhaps some functions want that and others
don't.
So, do we for .cfi_sections want to know whether at least one of the
functions in the TU (or partition for lto1) wants unwind tables /
asynchronous unwind tables, or whether all of them do, something else?

That isn't specific to LTO btw, one can compile say:
-g -O2 -fasynchronous-unwind-tables -funwind-tables
__attribute__((optimize ("no-asynchronous-unwind-tables,no-unwind-tables"))) int foo (int x) { return x; }
__attribute__((optimize ("no-asynchronous-unwind-tables,no-unwind-tables"))) int bar (int x) { return x; }
or
-g -O2 -fno-asynchronous-unwind-tables -fno-unwind-tables
__attribute__((optimize ("asynchronous-unwind-tables,unwind-tables"))) int foo (int x) { return x; }
__attribute__((optimize ("asynchronous-unwind-tables,unwind-tables"))) int bar (int x) { return x; }
Now, for non-LTO what you get in flag_asynchronous_unwind_tables or
flag_unwind_tables when cfun is NULL is I think whatever has been
set on the command line (or defaulted), which doesn't need to match
any of the emitted functions.
For LTO we currently get there just whatever has been defaulted.
Neither of that will always match all the states of all the functions.

	Jakub
Jakub Jelinek Jan. 18, 2023, 2:27 p.m. UTC | #13 
On Wed, Jan 18, 2023 at 02:03:42PM +0000, Michael Matz wrote:
> On Risc-V btw.  (which, unlike i386,aarch64,s390,rs6000 does not 
> effectively enable funwind-tables always via either backend or 
> common/config/$arch/ code, which is the reason why the problem can't be 
> seen there).  It's an interaction with -g :

Partly OT, what is riscv not defaulting that on as well?  Does it have
usable unwind info even without that option, something else?

	Jakub
Jan Hubicka Jan. 18, 2023, 3:07 p.m. UTC | #14 
> No unwind tables are generated, as if -funwind-tables is ignored.  If
> LTO is disabled, everything works as expected.
I think it is because dwaf2out_do_eh_frame is called out of function
context at the end of compilation. At that time cfun is NULL
and the flag is read from global settings that are wrong.
So we need to bookkeep if we saw function that needs EH tables and not.

Honza
> 
> -- 
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Andreas Schwab Jan. 18, 2023, 3:09 p.m. UTC | #15 
On Jan 18 2023, Jakub Jelinek wrote:

> Partly OT, what is riscv not defaulting that on as well?  Does it have
> usable unwind info even without that option, something else?

The RISC-V ABI does not address this, AFAICS.
Jakub Jelinek Jan. 18, 2023, 3:11 p.m. UTC | #16 
On Wed, Jan 18, 2023 at 04:09:08PM +0100, Andreas Schwab wrote:
> On Jan 18 2023, Jakub Jelinek wrote:
> 
> > Partly OT, what is riscv not defaulting that on as well?  Does it have
> > usable unwind info even without that option, something else?
> 
> The RISC-V ABI does not address this, AFAICS.

And neither do many other ABIs, still we default there to
-fasynchronous-unwind-tables because we've decided it is a good idea.

	Jakub
Andreas Schwab Jan. 18, 2023, 3:11 p.m. UTC | #17 
On Jan 18 2023, Jakub Jelinek wrote:

> Neither of that will always match all the states of all the functions.

But if the translation units are compiled with -funwind-tables, we want
the ltrans "units" to behave the same.
Michael Matz Jan. 18, 2023, 3:16 p.m. UTC | #18 
Hello,

On Wed, 18 Jan 2023, Jakub Jelinek wrote:

> On Wed, Jan 18, 2023 at 04:09:08PM +0100, Andreas Schwab wrote:
> > On Jan 18 2023, Jakub Jelinek wrote:
> > 
> > > Partly OT, what is riscv not defaulting that on as well?  Does it have
> > > usable unwind info even without that option, something else?
> > 
> > The RISC-V ABI does not address this, AFAICS.
> 
> And neither do many other ABIs, still we default there to
> -fasynchronous-unwind-tables because we've decided it is a good idea.

That might or might not be, but in the context of this thread that's 
immaterial.  Doing the same as the other archs will then simply hide the 
problem on risc-v as well, instead of fixing it.


Ciao,
Michael.
Jakub Jelinek Jan. 18, 2023, 3:19 p.m. UTC | #19 
On Wed, Jan 18, 2023 at 03:16:07PM +0000, Michael Matz wrote:
> On Wed, 18 Jan 2023, Jakub Jelinek wrote:
> 
> > On Wed, Jan 18, 2023 at 04:09:08PM +0100, Andreas Schwab wrote:
> > > On Jan 18 2023, Jakub Jelinek wrote:
> > > 
> > > > Partly OT, what is riscv not defaulting that on as well?  Does it have
> > > > usable unwind info even without that option, something else?
> > > 
> > > The RISC-V ABI does not address this, AFAICS.
> > 
> > And neither do many other ABIs, still we default there to
> > -fasynchronous-unwind-tables because we've decided it is a good idea.
> 
> That might or might not be, but in the context of this thread that's 
> immaterial.  Doing the same as the other archs will then simply hide the 
> problem on risc-v as well, instead of fixing it.

Yeah, that is why I've mentioned "Partly OT".  We want this bug to be fixed
(but the fix is not what has been posted but rather decide what we want to
ask there; if it is at the end of compilation, whether it is at least one
function with that flag has been compiled, or all functions have been with
that flag, something else), and IMHO riscv should switch to
-fasynchronous-unwind-tables by default.

	Jakub
Michael Matz Jan. 18, 2023, 3:32 p.m. UTC | #20 
Hello,

On Wed, 18 Jan 2023, Jakub Jelinek wrote:

> > > > > Partly OT, what is riscv not defaulting that on as well?  Does it have
> > > > > usable unwind info even without that option, something else?
> > > > 
> > > > The RISC-V ABI does not address this, AFAICS.
> > > 
> > > And neither do many other ABIs, still we default there to
> > > -fasynchronous-unwind-tables because we've decided it is a good idea.
> > 
> > That might or might not be, but in the context of this thread that's 
> > immaterial.  Doing the same as the other archs will then simply hide the 
> > problem on risc-v as well, instead of fixing it.
> 
> Yeah, that is why I've mentioned "Partly OT".  We want this bug to be fixed
> (but the fix is not what has been posted but rather decide what we want to
> ask there; if it is at the end of compilation, whether it is at least one
> function with that flag has been compiled, or all functions have been with
> that flag, something else),

The answer to this should be guided by normal use cases.  The normal use 
case is that a whole input file is compiled with or without 
-funwind-tables, and not that individual functions change this.  So any 
solution in which that usecase doesn't work is not a solution.

The purest solution is to emit unwind tables for all functions that 
request it into .eh_frame and for those that don't request it put 
into .debug_frame (if also -g is on).  If that requires enabling 
unwind-tables globally first (i.e. if even just one input function 
requests it) then that is what needs to be done.  (this seems to be the 
problem currently, that the unwind-table activation on a per-function 
basis comes too late).

The easier solution might be to make funwind-tables also be a global 
special-cased option for LTO (so that the usual use-case above works), 
that would trade one or another bug, but I'd say the current bug is more 
serious than the other bug that would be introduced.

> and IMHO riscv should switch to
> -fasynchronous-unwind-tables by default.

I don't disagree, as long as that doesn't lead to the bug being ignored :)


Ciao,
Michael.
Andreas Schwab Jan. 18, 2023, 4:09 p.m. UTC | #21 
On Jan 18 2023, Michael Matz wrote:

> The purest solution is to emit unwind tables for all functions that 
> request it into .eh_frame and for those that don't request it put 
> into .debug_frame (if also -g is on).

The assembler does not allow switching back to .eh_frame once a
different format has been chosen, so .eh_frame must be either on or off
all the way through.
Jan Hubicka Jan. 18, 2023, 4:25 p.m. UTC | #22 
> On Jan 18 2023, Michael Matz wrote:
> 
> > The purest solution is to emit unwind tables for all functions that 
> > request it into .eh_frame and for those that don't request it put 
> > into .debug_frame (if also -g is on).
> 
> The assembler does not allow switching back to .eh_frame once a
> different format has been chosen, so .eh_frame must be either on or off
> all the way through.

This is unforutnate (and I did not noticed this earlier).
Would it be hard to fix assembler? In general situations like this can
be handled by forced partitioning in GCC, but it is not a good solution
since we want to keep partitioning algorithm an optional step by design.

Honza
> 
> -- 
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Jakub Jelinek Jan. 18, 2023, 4:28 p.m. UTC | #23 
On Wed, Jan 18, 2023 at 05:25:10PM +0100, Jan Hubicka wrote:
> > On Jan 18 2023, Michael Matz wrote:
> > 
> > > The purest solution is to emit unwind tables for all functions that 
> > > request it into .eh_frame and for those that don't request it put 
> > > into .debug_frame (if also -g is on).
> > 
> > The assembler does not allow switching back to .eh_frame once a
> > different format has been chosen, so .eh_frame must be either on or off
> > all the way through.
> 
> This is unforutnate (and I did not noticed this earlier).
> Would it be hard to fix assembler? In general situations like this can
> be handled by forced partitioning in GCC, but it is not a good solution
> since we want to keep partitioning algorithm an optional step by design.

If it was just about compiler emitted .cfi_* directives, we could say
use .cfi_* directives for .eh_frame and hand emitted .debug_line for
.debug_frame or vice versa in the case of mixing functions with different
flags.
But inline asm can contain user written .cfi_* directives, so I think we
need to do something on the assembler side and then adjust gcc.

	Jakub
Richard Biener Jan. 19, 2023, 7:14 a.m. UTC | #24 
On Wed, Jan 18, 2023 at 4:07 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > No unwind tables are generated, as if -funwind-tables is ignored.  If
> > LTO is disabled, everything works as expected.
> I think it is because dwaf2out_do_eh_frame is called out of function
> context at the end of compilation. At that time cfun is NULL
> and the flag is read from global settings that are wrong.
> So we need to bookkeep if we saw function that needs EH tables and not.

I think we do that with the do_eh_frame variable which was introduced to
fix a similar problem (PR81351).  But then dwarf2out_do_eh_frame seems
to be used both at a function and at the TU level and the latter now should
check do_eh_frame?  (and not be called early).  That is, the caller in
dwarf2out_do_frame wants to check that variable.

I see the first invocation of the function with cfun == null from c_cpp_builtins
but all others seem to have cfun set?

Richard.

> Honza
> >
> > --
> > Andreas Schwab, SUSE Labs, schwab@suse.de
> > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> > "And now for something completely different."
Richard Biener Jan. 25, 2023, 1:16 p.m. UTC | #25 
On Wed, Jan 18, 2023 at 11:17 AM Andreas Schwab via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The -funwind-tables and -fasynchronous-unwind-tables options are relevant
> for the output pass, thus they need to be passed through by the lto
> wrapper.
>
> gcc/
>         * lto-wrapper.cc (merge_and_complain): Pass through
>         -funwind-tables and -fasynchronous-unwind-tables.
>         (append_compiler_options): Likewise.
> ---
>  gcc/lto-wrapper.cc | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
> index 11c4d1b38a4..627e8238606 100644
> --- a/gcc/lto-wrapper.cc
> +++ b/gcc/lto-wrapper.cc
> @@ -314,6 +314,8 @@ merge_and_complain (vec<cl_decoded_option> &decoded_options,
>         case OPT_fshow_column:
>         case OPT_fcommon:
>         case OPT_fgnu_tm:
> +       case OPT_funwind_tables:
> +       case OPT_fasynchronous_unwind_tables:
>         case OPT_g:
>           /* Do what the old LTO code did - collect exactly one option
>              setting per OPT code, we pick the first we encounter.

A better place is

        case OPT_fopenmp:
        case OPT_fopenacc:
          /* For selected options we can merge conservatively.  */
          if (existing_opt == -1)
            decoded_options.safe_push (*foption);
          /* -fopenmp > -fno-openmp,
             -fopenacc > -fno-openacc  */
          else if (foption->value > decoded_options[existing_opt].value)
            decoded_options[existing_opt] = *foption;
          break;

where we'd prefer -funwind-tables over -fno-unwind-tables when the
options do not match
across TUs.  Note that you likely want to add
-f[asynchronous-]unwind-tables handling
in lto-options.cc:lto_write_options as well so the default is streamed
as explicit option.
Otherwise a single TU with -fno-unwind-tables on x86-64 would cause
the whole LTO
compilation to be built without.

> @@ -737,6 +739,8 @@ append_compiler_options (obstack *argv_obstack, vec<cl_decoded_option> opts)
>         case OPT_fopenacc_dim_:
>         case OPT_foffload_abi_:
>         case OPT_fcf_protection_:
> +       case OPT_funwind_tables:
> +       case OPT_fasynchronous_unwind_tables:
>         case OPT_g:
>         case OPT_O:
>         case OPT_Ofast:
> --
> 2.39.1
>
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Andreas Schwab Jan. 25, 2023, 1:50 p.m. UTC | #26 
On Jan 25 2023, Richard Biener wrote:

> where we'd prefer -funwind-tables over -fno-unwind-tables when the
> options do not match
> across TUs.  Note that you likely want to add
> -f[asynchronous-]unwind-tables handling
> in lto-options.cc:lto_write_options as well so the default is streamed
> as explicit option.
> Otherwise a single TU with -fno-unwind-tables on x86-64 would cause
> the whole LTO
> compilation to be built without.

I don't think we actually need to handle -fasynchronous-unwind-tables
here, since that implies -funwind-tables, and only the latter is
relevant for dwarf2out_do_eh_frame.
diff mbox series

Patch

diff --git a/gcc/lto-wrapper.cc b/gcc/lto-wrapper.cc
index 11c4d1b38a4..627e8238606 100644
--- a/gcc/lto-wrapper.cc
+++ b/gcc/lto-wrapper.cc
@@ -314,6 +314,8 @@  merge_and_complain (vec<cl_decoded_option> &decoded_options,
 	case OPT_fshow_column:
 	case OPT_fcommon:
 	case OPT_fgnu_tm:
+	case OPT_funwind_tables:
+	case OPT_fasynchronous_unwind_tables:
 	case OPT_g:
 	  /* Do what the old LTO code did - collect exactly one option
 	     setting per OPT code, we pick the first we encounter.
@@ -737,6 +739,8 @@  append_compiler_options (obstack *argv_obstack, vec<cl_decoded_option> opts)
 	case OPT_fopenacc_dim_:
 	case OPT_foffload_abi_:
 	case OPT_fcf_protection_:
+	case OPT_funwind_tables:
+	case OPT_fasynchronous_unwind_tables:
 	case OPT_g:
 	case OPT_O:
 	case OPT_Ofast: