Message ID | ortv8skfjk.fsf@lxoliva.fsfla.org |
---|---|
State | New |
Headers | show |
Series | [x86] recompute opt flags after opt level change | expand |
On Tue, Oct 1, 2019 at 10:59 AM Alexandre Oliva <oliva@adacore.com> wrote: > > flag_omit_frame_pointer is set in machine-independent code depending > on the optimization level. It is then overridden in x86 > target-specific code depending on a macro defined by > --enable-frame-pointer. > > Uses of attribute optimize go through machine-independent overriding > of flag_omit_frame_pointer, but the x86-specific overriding code did > NOT cover this flag, so, even if the attribute does not change the > optimization level, flag_omit_frame_pointer may end up with a > different value, and prevent inlining because of incompatible flags, > as detected by the gcc.dg/ipa/iinline-attr.c test on an > --enable-frame-pointer x86 toolchain. > > Regstrapped on x86_64-linux-gnu. Ok to install? > > > for gcc/ChangeLog > > * config/i386/i386-options.c > (ix86_recompute_optlev_based_flags): New, moved out of... > (ix86_option_override_internal): ... this. Call it. > (ix86_override_options_after_change): Call it here too. OK. Thanks, Uros. > --- > gcc/config/i386/i386-options.c | 89 ++++++++++++++++++++++------------------ > 1 file changed, 50 insertions(+), 39 deletions(-) > > diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c > index c148aa20511db..ed286bffaaa3b 100644 > --- a/gcc/config/i386/i386-options.c > +++ b/gcc/config/i386/i386-options.c > @@ -1527,12 +1527,61 @@ ix86_default_align (struct gcc_options *opts) > opts->x_str_align_functions = processor_cost_table[ix86_tune]->align_func; > } > > +#ifndef USE_IX86_FRAME_POINTER > +#define USE_IX86_FRAME_POINTER 0 > +#endif > + > +/* (Re)compute option overrides affected by optimization levels in > + target-specific ways. */ > + > +static void > +ix86_recompute_optlev_based_flags (struct gcc_options *opts, > + struct gcc_options *opts_set) > +{ > + /* Set the default values for switches whose default depends on TARGET_64BIT > + in case they weren't overwritten by command line options. */ > + if (TARGET_64BIT_P (opts->x_ix86_isa_flags)) > + { > + if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) > + opts->x_flag_omit_frame_pointer = !USE_IX86_FRAME_POINTER; > + if (opts->x_flag_asynchronous_unwind_tables > + && !opts_set->x_flag_unwind_tables > + && TARGET_64BIT_MS_ABI) > + opts->x_flag_unwind_tables = 1; > + if (opts->x_flag_asynchronous_unwind_tables == 2) > + opts->x_flag_unwind_tables > + = opts->x_flag_asynchronous_unwind_tables = 1; > + if (opts->x_flag_pcc_struct_return == 2) > + opts->x_flag_pcc_struct_return = 0; > + } > + else > + { > + if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) > + opts->x_flag_omit_frame_pointer > + = !(USE_IX86_FRAME_POINTER || opts->x_optimize_size); > + if (opts->x_flag_asynchronous_unwind_tables == 2) > + opts->x_flag_asynchronous_unwind_tables = !USE_IX86_FRAME_POINTER; > + if (opts->x_flag_pcc_struct_return == 2) > + { > + /* Intel MCU psABI specifies that -freg-struct-return should > + be on. Instead of setting DEFAULT_PCC_STRUCT_RETURN to 1, > + we check -miamcu so that -freg-struct-return is always > + turned on if -miamcu is used. */ > + if (TARGET_IAMCU_P (opts->x_target_flags)) > + opts->x_flag_pcc_struct_return = 0; > + else > + opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; > + } > + } > +} > + > /* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook. */ > > void > ix86_override_options_after_change (void) > { > ix86_default_align (&global_options); > + ix86_recompute_optlev_based_flags (&global_options, &global_options_set); > } > > /* Clear stack slot assignments remembered from previous functions. > @@ -2220,45 +2269,7 @@ ix86_option_override_internal (bool main_args_p, > > set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes); > > -#ifndef USE_IX86_FRAME_POINTER > -#define USE_IX86_FRAME_POINTER 0 > -#endif > - > - /* Set the default values for switches whose default depends on TARGET_64BIT > - in case they weren't overwritten by command line options. */ > - if (TARGET_64BIT_P (opts->x_ix86_isa_flags)) > - { > - if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) > - opts->x_flag_omit_frame_pointer = !USE_IX86_FRAME_POINTER; > - if (opts->x_flag_asynchronous_unwind_tables > - && !opts_set->x_flag_unwind_tables > - && TARGET_64BIT_MS_ABI) > - opts->x_flag_unwind_tables = 1; > - if (opts->x_flag_asynchronous_unwind_tables == 2) > - opts->x_flag_unwind_tables > - = opts->x_flag_asynchronous_unwind_tables = 1; > - if (opts->x_flag_pcc_struct_return == 2) > - opts->x_flag_pcc_struct_return = 0; > - } > - else > - { > - if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) > - opts->x_flag_omit_frame_pointer > - = !(USE_IX86_FRAME_POINTER || opts->x_optimize_size); > - if (opts->x_flag_asynchronous_unwind_tables == 2) > - opts->x_flag_asynchronous_unwind_tables = !USE_IX86_FRAME_POINTER; > - if (opts->x_flag_pcc_struct_return == 2) > - { > - /* Intel MCU psABI specifies that -freg-struct-return should > - be on. Instead of setting DEFAULT_PCC_STRUCT_RETURN to 1, > - we check -miamcu so that -freg-struct-return is always > - turned on if -miamcu is used. */ > - if (TARGET_IAMCU_P (opts->x_target_flags)) > - opts->x_flag_pcc_struct_return = 0; > - else > - opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; > - } > - } > + ix86_recompute_optlev_based_flags (opts, opts_set); > > ix86_tune_cost = processor_cost_table[ix86_tune]; > /* TODO: ix86_cost should be chosen at instruction or function granuality > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF VP & FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara
On 10/1/19 10:58 AM, Alexandre Oliva wrote: > + if (opts->x_flag_asynchronous_unwind_tables == 2) > + opts->x_flag_unwind_tables > + = opts->x_flag_asynchronous_unwind_tables = 1; Hello. Spotted that when I tried to make x_flag_asynchronous_unwind_tables a boolean flag. Anyway the code seems fishy, shouldn't it be: if (opts->x_flag_asynchronous_unwind_tables == 2) opts->x_flag_unwind_tables = opts->x_flag_asynchronous_unwind_tables == 1; <---- '==' instead of '=' Thanks, Martin
On Wed, Dec 16, 2020 at 2:18 PM Martin Liška <mliska@suse.cz> wrote: > > On 10/1/19 10:58 AM, Alexandre Oliva wrote: > > + if (opts->x_flag_asynchronous_unwind_tables == 2) > > + opts->x_flag_unwind_tables > > + = opts->x_flag_asynchronous_unwind_tables = 1; > > Hello. > > Spotted that when I tried to make x_flag_asynchronous_unwind_tables a boolean flag. > > Anyway the code seems fishy, shouldn't it be: > > if (opts->x_flag_asynchronous_unwind_tables == 2) > opts->x_flag_unwind_tables > = opts->x_flag_asynchronous_unwind_tables == 1; <---- '==' instead of '=' It should be OK. Please note that opts->x_flag_asynchronous_unwind_tables == 1 will never be true due to preceding condition. Uros. > > Thanks, > Martin
Hello, Martin, On Dec 16, 2020, Martin Liška <mliska@suse.cz> wrote: > On 10/1/19 10:58 AM, Alexandre Oliva wrote: >> + if (opts->x_flag_asynchronous_unwind_tables == 2) >> + opts->x_flag_unwind_tables >> + = opts->x_flag_asynchronous_unwind_tables = 1; > Anyway the code seems fishy, shouldn't it be: > if (opts->x_flag_asynchronous_unwind_tables == 2) > opts->x_flag_unwind_tables > = opts->x_flag_asynchronous_unwind_tables == 1; <---- '==' instead of '=' No, IIRC the idea was to assign 1 to both fields. I can't tell for sure because I don't think I wrote this myself, IIRC my patch just moved it.
diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c index c148aa20511db..ed286bffaaa3b 100644 --- a/gcc/config/i386/i386-options.c +++ b/gcc/config/i386/i386-options.c @@ -1527,12 +1527,61 @@ ix86_default_align (struct gcc_options *opts) opts->x_str_align_functions = processor_cost_table[ix86_tune]->align_func; } +#ifndef USE_IX86_FRAME_POINTER +#define USE_IX86_FRAME_POINTER 0 +#endif + +/* (Re)compute option overrides affected by optimization levels in + target-specific ways. */ + +static void +ix86_recompute_optlev_based_flags (struct gcc_options *opts, + struct gcc_options *opts_set) +{ + /* Set the default values for switches whose default depends on TARGET_64BIT + in case they weren't overwritten by command line options. */ + if (TARGET_64BIT_P (opts->x_ix86_isa_flags)) + { + if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) + opts->x_flag_omit_frame_pointer = !USE_IX86_FRAME_POINTER; + if (opts->x_flag_asynchronous_unwind_tables + && !opts_set->x_flag_unwind_tables + && TARGET_64BIT_MS_ABI) + opts->x_flag_unwind_tables = 1; + if (opts->x_flag_asynchronous_unwind_tables == 2) + opts->x_flag_unwind_tables + = opts->x_flag_asynchronous_unwind_tables = 1; + if (opts->x_flag_pcc_struct_return == 2) + opts->x_flag_pcc_struct_return = 0; + } + else + { + if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) + opts->x_flag_omit_frame_pointer + = !(USE_IX86_FRAME_POINTER || opts->x_optimize_size); + if (opts->x_flag_asynchronous_unwind_tables == 2) + opts->x_flag_asynchronous_unwind_tables = !USE_IX86_FRAME_POINTER; + if (opts->x_flag_pcc_struct_return == 2) + { + /* Intel MCU psABI specifies that -freg-struct-return should + be on. Instead of setting DEFAULT_PCC_STRUCT_RETURN to 1, + we check -miamcu so that -freg-struct-return is always + turned on if -miamcu is used. */ + if (TARGET_IAMCU_P (opts->x_target_flags)) + opts->x_flag_pcc_struct_return = 0; + else + opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; + } + } +} + /* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook. */ void ix86_override_options_after_change (void) { ix86_default_align (&global_options); + ix86_recompute_optlev_based_flags (&global_options, &global_options_set); } /* Clear stack slot assignments remembered from previous functions. @@ -2220,45 +2269,7 @@ ix86_option_override_internal (bool main_args_p, set_ix86_tune_features (ix86_tune, opts->x_ix86_dump_tunes); -#ifndef USE_IX86_FRAME_POINTER -#define USE_IX86_FRAME_POINTER 0 -#endif - - /* Set the default values for switches whose default depends on TARGET_64BIT - in case they weren't overwritten by command line options. */ - if (TARGET_64BIT_P (opts->x_ix86_isa_flags)) - { - if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) - opts->x_flag_omit_frame_pointer = !USE_IX86_FRAME_POINTER; - if (opts->x_flag_asynchronous_unwind_tables - && !opts_set->x_flag_unwind_tables - && TARGET_64BIT_MS_ABI) - opts->x_flag_unwind_tables = 1; - if (opts->x_flag_asynchronous_unwind_tables == 2) - opts->x_flag_unwind_tables - = opts->x_flag_asynchronous_unwind_tables = 1; - if (opts->x_flag_pcc_struct_return == 2) - opts->x_flag_pcc_struct_return = 0; - } - else - { - if (opts->x_optimize >= 1 && !opts_set->x_flag_omit_frame_pointer) - opts->x_flag_omit_frame_pointer - = !(USE_IX86_FRAME_POINTER || opts->x_optimize_size); - if (opts->x_flag_asynchronous_unwind_tables == 2) - opts->x_flag_asynchronous_unwind_tables = !USE_IX86_FRAME_POINTER; - if (opts->x_flag_pcc_struct_return == 2) - { - /* Intel MCU psABI specifies that -freg-struct-return should - be on. Instead of setting DEFAULT_PCC_STRUCT_RETURN to 1, - we check -miamcu so that -freg-struct-return is always - turned on if -miamcu is used. */ - if (TARGET_IAMCU_P (opts->x_target_flags)) - opts->x_flag_pcc_struct_return = 0; - else - opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; - } - } + ix86_recompute_optlev_based_flags (opts, opts_set); ix86_tune_cost = processor_cost_table[ix86_tune]; /* TODO: ix86_cost should be chosen at instruction or function granuality