Re: [Patch 1/2] Control SRA and IPA-SRA by a param rather than MOVE_RATIO

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Aug 20, 2014 at 11:09 AM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
>
> Hi,
>
> Presently the decision as to whether to completely scalarize an aggregate
> or not is made based on MOVE_RATIO. This is an undocumented, and unexpected,
> overloading of the target macro.
>
> In this patch we fix this.
>
> First, we we add a new target hook
> TARGET_DEFAULT_MAX_TOTAL_SCALARIZATION_SIZE, which returns MOVE_RATIO
> by default.
>
> Then we add two new parameters:
>
>   sra-max-total-scalarization-size-Ospeed - The maximum size of aggregate
>   to consider when compiling for speed
>   sra-max-total-scalarization-size-Osize - The maximum size of aggregate
>   to consider when compiling for size.
>
> Set to default to 0.
>
> Finally we wire up SRA to prefer using the parameters, and if it doesn't
> find values for them, fallback to the target hook.
>
> Bootstrapped and regression tested for x86, arm and aarch64 with no
> issues, I've also thrown a smoke-test of popular small benchmarks at
> each platform without seeing meaningful differences (as you would expect).
>
> OK?

I think this is overly complicated and instead SRA should only
use the parameters.  Targets can adjust their default (like they
do for other parameters).

The default should be MOVE_RATIO which should be applied
where the common code adjusts parameters (see existing
examples for not overriding user specified ones).

Thanks,
Richard.

> Thanks,
> James
>
> ---
> gcc/
>
> 2014-08-20  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * doc/invoke.texi (sra-max-total-scalarization-size-Ospeed): Document.
>         (sra-max-total-scalarization-size-Osize): Likewise.
>         * doc/tm.texi.in
>         (TARGET_DEFAULT_MAX_TOTAL_SCALARIZATION_SIZE): Add hook.
>         * doc/tm.texi: Regenerate.
>         * params.def (sra-max-total-scalarization-size-Ospeed): New.
>         (sra-max-total-scalarization-size-Osize): Likewise.
>         * target.def (default_max_total_scalarization_size): New.
>         * targhooks.c (default_max_total_scalarization_size): New.
>         * targhooks.h (default_max_total_scalarization_size): New.
>         * tree-sra.c (get_max_total_scalarization_size): New.
>         (analyze_all_variable_accesses): Use it.