| Message ID | 1435074164-7206-1-git-send-email-james.greenhalgh@arm.com |
|---|---|
| State | New |
| Headers | show |
On 06/23/2015 09:42 AM, James Greenhalgh wrote: > > On Tue, Jun 23, 2015 at 09:52:01AM +0100, Jakub Jelinek wrote: >> On Tue, Jun 23, 2015 at 09:18:52AM +0100, James Greenhalgh wrote: >>> This patch fixes the issue by always calling get_move_ratio in the SRA >>> code, ensuring that an up-to-date value is used. >>> >>> Unfortunately, this means we have to use 0 as a sentinel value for >>> the parameter - indicating no user override of the feature - and >>> therefore cannot use it to disable scalarization. However, there >>> are other ways to disable scalarazation (-fno-tree-sra) so this is not >>> a great loss. >> >> You can handle even that. >> > > <snip> > >> enum compiler_param param >> = optimize_function_for_size_p (cfun) >> ? PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE >> : PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED; >> unsigned max_scalarization_size = PARAM_VALUE (param) * BITS_PER_UNIT; >> if (!max_scalarization_size && !global_options_set.x_param_values[param]) >> >> Then it will handle explicit --param sra-max-scalarization-size-Os*=0 >> differently from implicit 0. > > Ah hah! OK, I've respun the patch removing this extra justification in > the documentation and reshuffling the logic a little. > >> OT, shouldn't max_scalarization_size be at least unsigned HOST_WIDE_INT, >> so that it doesn't overflow for larger values (0x40000000 etc.)? >> Probably need some cast in the multiplication to avoid UB in the compiler. > > I've increased the size of max_scalarization_size to a UHWI in this spin. > > Bootstrapped and tested on AArch64 and x86-64 with no issues and checked > to see the PR is fixed. > > OK for trunk, and gcc-5 in a few days? > > Thanks, > James > > --- > gcc/ > > 2015-06-23 James Greenhalgh <james.greenhalgh@arm.com> > > PR tree-optimization/66119 > * toplev.c (process_options): Don't set up default values for > the sra_max_scalarization_size_{speed,size} parameters. > * tree-sra (analyze_all_variable_accesses): If no values > have been set for the sra_max_scalarization_size_{speed,size} > parameters, call get_move_ratio to get target defaults. Any testcase for this change? OK with a testcase. jeff
diff --git a/gcc/toplev.c b/gcc/toplev.c index 2f43a89..902bfc7 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1301,20 +1301,6 @@ process_options (void) so we can correctly initialize debug output. */ no_backend = lang_hooks.post_options (&main_input_filename); - /* Set default values for parameters relation to the Scalar Reduction - of Aggregates passes (SRA and IP-SRA). We must do this here, rather - than in opts.c:default_options_optimization as historically these - tuning heuristics have been based on MOVE_RATIO, which on some - targets requires other symbols from the backend. */ - maybe_set_param_value - (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED, - get_move_ratio (true) * UNITS_PER_WORD, - global_options.x_param_values, global_options_set.x_param_values); - maybe_set_param_value - (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE, - get_move_ratio (false) * UNITS_PER_WORD, - global_options.x_param_values, global_options_set.x_param_values); - /* Some machines may reject certain combinations of options. */ targetm.target_option.override (); diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 8e34244..5f573f6 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2549,11 +2549,20 @@ analyze_all_variable_accesses (void) bitmap tmp = BITMAP_ALLOC (NULL); bitmap_iterator bi; unsigned i; - unsigned max_scalarization_size - = (optimize_function_for_size_p (cfun) - ? PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE) - : PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED)) - * BITS_PER_UNIT; + bool optimize_speed_p = !optimize_function_for_size_p (cfun); + + enum compiler_param param = optimize_speed_p + ? PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED + : PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE; + + /* If the user didn't set PARAM_SRA_MAX_SCALARIZATION_SIZE_<...>, + fall back to a target default. */ + unsigned HOST_WIDE_INT max_scalarization_size + = global_options_set.x_param_values[param] + ? PARAM_VALUE (param) + : get_move_ratio (optimize_speed_p) * UNITS_PER_WORD; + + max_scalarization_size *= BITS_PER_UNIT; EXECUTE_IF_SET_IN_BITMAP (candidate_bitmap, 0, i, bi) if (bitmap_bit_p (should_scalarize_away_bitmap, i)