Re: [Patch SRA] Fix PR66119 by calling get_move_ratio in SRA

[James Greenhalgh <james dot greenhalgh at arm dot com>]

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.

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)