| Message ID | 20240516062256.4144064-1-hongyu.wang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | i386: Fix ix86_option override after change [PR 113719] | expand |
On Thu, May 16, 2024 at 8:25 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > Hi, > > In ix86_override_options_after_change, calls to ix86_default_align > and ix86_recompute_optlev_based_flags will cause mismatched target > opt_set when doing cl_optimization_restore. Move them back to > ix86_option_override_internal to solve the issue. > > Bootstrapped & regtested on x86_64-pc-linux-gnu, and Rainer helped to > test with i386-pc-solaris2.11 which also passed 32/64bit tests. Since this is a tricky area apparently without too much test coverage can we have a testcase for this? > Ok for trunk and backport down to gcc12? > > gcc/ChangeLog: > > PR target/113719 > * config/i386/i386-options.cc (ix86_override_options_after_change): > Remove call to ix86_default_align and > ix86_recompute_optlev_based_flags. > (ix86_option_override_internal): Call ix86_default_align and > ix86_recompute_optlev_based_flags. > --- > gcc/config/i386/i386-options.cc | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc > index ac48b5c61c4..d97464f2c74 100644 > --- a/gcc/config/i386/i386-options.cc > +++ b/gcc/config/i386/i386-options.cc > @@ -1930,11 +1930,6 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, > void > ix86_override_options_after_change (void) > { > - /* Default align_* from the processor table. */ > - ix86_default_align (&global_options); > - > - ix86_recompute_optlev_based_flags (&global_options, &global_options_set); > - > /* Disable unrolling small loops when there's explicit > -f{,no}unroll-loop. */ > if ((OPTION_SET_P (flag_unroll_loops)) > @@ -2530,6 +2525,8 @@ ix86_option_override_internal (bool main_args_p, > > set_ix86_tune_features (opts, ix86_tune, opts->x_ix86_dump_tunes); > > + ix86_recompute_optlev_based_flags (opts, opts_set); > + > ix86_override_options_after_change (); > > ix86_tune_cost = processor_cost_table[ix86_tune]; > @@ -2565,6 +2562,9 @@ ix86_option_override_internal (bool main_args_p, > || TARGET_64BIT_P (opts->x_ix86_isa_flags)) > opts->x_ix86_regparm = REGPARM_MAX; > > + /* Default align_* from the processor table. */ > + ix86_default_align (&global_options); > + > /* Provide default for -mbranch-cost= value. */ > SET_OPTION_IF_UNSET (opts, opts_set, ix86_branch_cost, > ix86_tune_cost->branch_cost); > -- > 2.31.1 >
Richard Biener <richard.guenther@gmail.com> 于2024年5月16日周四 15:05写道： > > On Thu, May 16, 2024 at 8:25 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > > > Hi, > > > > In ix86_override_options_after_change, calls to ix86_default_align > > and ix86_recompute_optlev_based_flags will cause mismatched target > > opt_set when doing cl_optimization_restore. Move them back to > > ix86_option_override_internal to solve the issue. > > > > Bootstrapped & regtested on x86_64-pc-linux-gnu, and Rainer helped to > > test with i386-pc-solaris2.11 which also passed 32/64bit tests. > > Since this is a tricky area apparently without too much test coverage can > we have a testcase for this? This is a fix for my previous change on PR 107692, which moved these 2 functions to ix86_override_options_after_change and it caused the PR113719 regression. The PR103696 test is the one that expose the issue. Actually the previous change will cause these 2 function be called in cl_optimization_restore which is redundant and incorrect. I cannot find another test to expose other functional regressions. > > > Ok for trunk and backport down to gcc12? > > > > gcc/ChangeLog: > > > > PR target/113719 > > * config/i386/i386-options.cc (ix86_override_options_after_change): > > Remove call to ix86_default_align and > > ix86_recompute_optlev_based_flags. > > (ix86_option_override_internal): Call ix86_default_align and > > ix86_recompute_optlev_based_flags. > > --- > > gcc/config/i386/i386-options.cc | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc > > index ac48b5c61c4..d97464f2c74 100644 > > --- a/gcc/config/i386/i386-options.cc > > +++ b/gcc/config/i386/i386-options.cc > > @@ -1930,11 +1930,6 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, > > void > > ix86_override_options_after_change (void) > > { > > - /* Default align_* from the processor table. */ > > - ix86_default_align (&global_options); > > - > > - ix86_recompute_optlev_based_flags (&global_options, &global_options_set); > > - > > /* Disable unrolling small loops when there's explicit > > -f{,no}unroll-loop. */ > > if ((OPTION_SET_P (flag_unroll_loops)) > > @@ -2530,6 +2525,8 @@ ix86_option_override_internal (bool main_args_p, > > > > set_ix86_tune_features (opts, ix86_tune, opts->x_ix86_dump_tunes); > > > > + ix86_recompute_optlev_based_flags (opts, opts_set); > > + > > ix86_override_options_after_change (); > > > > ix86_tune_cost = processor_cost_table[ix86_tune]; > > @@ -2565,6 +2562,9 @@ ix86_option_override_internal (bool main_args_p, > > || TARGET_64BIT_P (opts->x_ix86_isa_flags)) > > opts->x_ix86_regparm = REGPARM_MAX; > > > > + /* Default align_* from the processor table. */ > > + ix86_default_align (&global_options); > > + > > /* Provide default for -mbranch-cost= value. */ > > SET_OPTION_IF_UNSET (opts, opts_set, ix86_branch_cost, > > ix86_tune_cost->branch_cost); > > -- > > 2.31.1 > >
On Thu, May 16, 2024 at 5:15 PM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > Richard Biener <richard.guenther@gmail.com> 于2024年5月16日周四 15:05写道： > > > > > On Thu, May 16, 2024 at 8:25 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > > > > > Hi, > > > > > > In ix86_override_options_after_change, calls to ix86_default_align > > > and ix86_recompute_optlev_based_flags will cause mismatched target > > > opt_set when doing cl_optimization_restore. Move them back to > > > ix86_option_override_internal to solve the issue. > > > > > > Bootstrapped & regtested on x86_64-pc-linux-gnu, and Rainer helped to > > > test with i386-pc-solaris2.11 which also passed 32/64bit tests. > > > > Since this is a tricky area apparently without too much test coverage can > > we have a testcase for this? > > This is a fix for my previous change on PR 107692, which moved these 2 > functions to ix86_override_options_after_change and it caused the > PR113719 regression. The PR103696 test is the one that expose the > issue. Actually the previous change will cause these 2 function be > called in cl_optimization_restore > which is redundant and incorrect. I cannot find another test to expose > other functional regressions. > > > > > > Ok for trunk and backport down to gcc12? Ok. > > > > > > gcc/ChangeLog: > > > > > > PR target/113719 > > > * config/i386/i386-options.cc (ix86_override_options_after_change): > > > Remove call to ix86_default_align and > > > ix86_recompute_optlev_based_flags. > > > (ix86_option_override_internal): Call ix86_default_align and > > > ix86_recompute_optlev_based_flags. > > > --- > > > gcc/config/i386/i386-options.cc | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc > > > index ac48b5c61c4..d97464f2c74 100644 > > > --- a/gcc/config/i386/i386-options.cc > > > +++ b/gcc/config/i386/i386-options.cc > > > @@ -1930,11 +1930,6 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, > > > void > > > ix86_override_options_after_change (void) > > > { > > > - /* Default align_* from the processor table. */ > > > - ix86_default_align (&global_options); > > > - > > > - ix86_recompute_optlev_based_flags (&global_options, &global_options_set); > > > - > > > /* Disable unrolling small loops when there's explicit > > > -f{,no}unroll-loop. */ > > > if ((OPTION_SET_P (flag_unroll_loops)) > > > @@ -2530,6 +2525,8 @@ ix86_option_override_internal (bool main_args_p, > > > > > > set_ix86_tune_features (opts, ix86_tune, opts->x_ix86_dump_tunes); > > > > > > + ix86_recompute_optlev_based_flags (opts, opts_set); > > > + > > > ix86_override_options_after_change (); > > > > > > ix86_tune_cost = processor_cost_table[ix86_tune]; > > > @@ -2565,6 +2562,9 @@ ix86_option_override_internal (bool main_args_p, > > > || TARGET_64BIT_P (opts->x_ix86_isa_flags)) > > > opts->x_ix86_regparm = REGPARM_MAX; > > > > > > + /* Default align_* from the processor table. */ > > > + ix86_default_align (&global_options); > > > + > > > /* Provide default for -mbranch-cost= value. */ > > > SET_OPTION_IF_UNSET (opts, opts_set, ix86_branch_cost, > > > ix86_tune_cost->branch_cost); > > > -- > > > 2.31.1 > > >
diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc index ac48b5c61c4..d97464f2c74 100644 --- a/gcc/config/i386/i386-options.cc +++ b/gcc/config/i386/i386-options.cc @@ -1930,11 +1930,6 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts, void ix86_override_options_after_change (void) { - /* Default align_* from the processor table. */ - ix86_default_align (&global_options); - - ix86_recompute_optlev_based_flags (&global_options, &global_options_set); - /* Disable unrolling small loops when there's explicit -f{,no}unroll-loop. */ if ((OPTION_SET_P (flag_unroll_loops)) @@ -2530,6 +2525,8 @@ ix86_option_override_internal (bool main_args_p, set_ix86_tune_features (opts, ix86_tune, opts->x_ix86_dump_tunes); + ix86_recompute_optlev_based_flags (opts, opts_set); + ix86_override_options_after_change (); ix86_tune_cost = processor_cost_table[ix86_tune]; @@ -2565,6 +2562,9 @@ ix86_option_override_internal (bool main_args_p, || TARGET_64BIT_P (opts->x_ix86_isa_flags)) opts->x_ix86_regparm = REGPARM_MAX; + /* Default align_* from the processor table. */ + ix86_default_align (&global_options); + /* Provide default for -mbranch-cost= value. */ SET_OPTION_IF_UNSET (opts, opts_set, ix86_branch_cost, ix86_tune_cost->branch_cost);