| Message ID | ormsmojcv7.fsf_-_@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [i386] adjust flag_omit_frame_pointer in a single function [PR113719] (was: Re: [PATCH] [i386] restore recompute to override opts after change [PR113719]) | expand |
Hi Alexandre, > Regstrapped on x86_64-linux-gnu. Also verified that the regression is > cured with a i686-solaris cross compiler. Ok to install? FWIW, I've also included the patch in last night's i386-pc-solaris2.11 bootstrap: the failures are gone, no regressions. Thanks. Rainer
On Thu, Jul 11, 2024 at 9:07 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jul 4, 2024, Alexandre Oliva <oliva@adacore.com> wrote: > > > On Jul 3, 2024, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > > > Hmm, I wonder if leaf frame pointer has to do with that. > > It did, in a way. > > ---- > > The first two patches for PR113719 have each regressed > gcc.dg/ipa/iinline-attr.c on a different target. The reason for this > instability is that there are competing flag_omit_frame_pointer > overriders on x86: > > - ix86_recompute_optlev_based_flags computes and sets a > -f[no-]omit-frame-pointer default depending on > USE_IX86_FRAME_POINTER and, in 32-bit mode, optimize_size > > - ix86_option_override_internal enables flag_omit_frame_pointer for > -momit-leaf-frame-pointer to take effect > > ix86_option_override[_internal] calls > ix86_recompute_optlev_based_flags before setting > flag_omit_frame_pointer. It is called during global process_options. > > But ix86_recompute_optlev_based_flags is also called by > parse_optimize_options, during attribute processing, and at that > point, ix86_option_override is not called, so the final overrider for > global options is not applied to the optimize attributes. If they > differ, the testcase fails. > > In order to fix this, we need to process all overriders of this option > whenever we process any of them. Since this setting is affected by > optimization options, it makes sense to compute it in > parse_optimize_options, rather than in process_options. > > Regstrapped on x86_64-linux-gnu. Also verified that the regression is > cured with a i686-solaris cross compiler. Ok to install? Ok. thanks. > > > for gcc/ChangeLog > > PR target/113719 > * config/i386/i386-options.cc (ix86_option_override_internal): > Move flag_omit_frame_pointer final overrider... > (ix86_recompute_optlev_based_flags): ... here. > --- > gcc/config/i386/i386-options.cc | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc > index 5824c0cb072eb..059ef3ae6ad44 100644 > --- a/gcc/config/i386/i386-options.cc > +++ b/gcc/config/i386/i386-options.cc > @@ -1911,6 +1911,12 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, > opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; > } > } > + > + /* Keep nonleaf frame pointers. */ > + if (opts->x_flag_omit_frame_pointer) > + opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; > + else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags)) > + opts->x_flag_omit_frame_pointer = 1; > } > > /* Implement part of TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook. */ > @@ -2590,12 +2596,6 @@ ix86_option_override_internal (bool main_args_p, > opts->x_target_flags |= MASK_NO_RED_ZONE; > } > > - /* Keep nonleaf frame pointers. */ > - if (opts->x_flag_omit_frame_pointer) > - opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; > - else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags)) > - opts->x_flag_omit_frame_pointer = 1; > - > /* If we're doing fast math, we don't care about comparison order > wrt NaNs. This lets us use a shorter comparison sequence. */ > if (opts->x_flag_finite_math_only) > > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity > Excluding neuro-others for not behaving ""normal"" is *not* inclusive
On Jul 15, 2024, Hongtao Liu <crazylht@gmail.com> wrote: >> Regstrapped on x86_64-linux-gnu. Also verified that the regression is >> cured with a i686-solaris cross compiler. Ok to install? > Ok. thanks. As recommended in the PR, I've backported both this patch and the previous one to the same branches that had the initial PR's patch. >> PR target/113719 >> * config/i386/i386-options.cc (ix86_option_override_internal): >> Move flag_omit_frame_pointer final overrider... >> (ix86_recompute_optlev_based_flags): ... here.
diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc index 5824c0cb072eb..059ef3ae6ad44 100644 --- a/gcc/config/i386/i386-options.cc +++ b/gcc/config/i386/i386-options.cc @@ -1911,6 +1911,12 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN; } } + + /* Keep nonleaf frame pointers. */ + if (opts->x_flag_omit_frame_pointer) + opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; + else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags)) + opts->x_flag_omit_frame_pointer = 1; } /* Implement part of TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook. */ @@ -2590,12 +2596,6 @@ ix86_option_override_internal (bool main_args_p, opts->x_target_flags |= MASK_NO_RED_ZONE; } - /* Keep nonleaf frame pointers. */ - if (opts->x_flag_omit_frame_pointer) - opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER; - else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags)) - opts->x_flag_omit_frame_pointer = 1; - /* If we're doing fast math, we don't care about comparison order wrt NaNs. This lets us use a shorter comparison sequence. */ if (opts->x_flag_finite_math_only)