[SRA] Fix PR66119 by calling get_move_ratio in SRA

Hi,

The problem in PR66119 is that we assume MOVE_RATIO will be constant
for a compilation run, such that we only need to read it once at compiler
startup if we want to set up defaults for
--param sra-max-scalarization-size-Osize and
--param sra-max-scalarization-size-Osize.

This assumption is faulty. Some targets may have MOVE_RATIO set up
to use state which depends on the selected processor - which may vary
per function for a switchable target.

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.

Bootstrapped and tested on x86-64 and AArch64 with no issues.

OK for trunk (and 5.2 after a few days watching for fallout)?

Thanks,
James

---
gcc/

2015-06-23  James Greenhalgh  <james.greenhalgh@arm.com>

	PR tree-optimization/66119
	* doc/invoke.texi (sra-max-scalarization-size-Osize): Mention that
	"0" is used as a sentinel value.
	(sra-max-scalarization-size-Ospeed): Likewise.
	* 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.

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.

> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index 8e34244..e2419af 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -2549,12 +2549,24 @@ analyze_all_variable_accesses (void)
>    bitmap tmp = BITMAP_ALLOC (NULL);
>    bitmap_iterator bi;
>    unsigned i;
> +  bool optimize_speed_p = !optimize_function_for_size_p (cfun);
> +
>    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))
> +    = (optimize_speed_p
> +	? PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED)
> +	: PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE))
>        * BITS_PER_UNIT;
>  
> +  /* If the user didn't set PARAM_SRA_MAX_SCALARIZATION_SIZE_<...>,
> +     fall back to a target default.  This means that zero cannot be
> +     used to disable scalarization as we've taken it as a sentinel
> +     value.  This is not ideal, but see PR66119 for the reason we
> +     can't simply set the target defaults ahead of time during option
> +     handling.  */
> +  if (!max_scalarization_size)

  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.

Or you could allow value of -1 for those params and make that the default
- -1 would mean the special get_move_ration value, 0 would disable, > 0
would be the requested scalarization.
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.

	Jakub