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.greenha...@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

Reply via email to