On Thu, Sep 25, 2014 at 4:57 PM, James Greenhalgh
<james.greenha...@arm.com> wrote:
>
> Hi,
>
> After hookizing MOVE_BY_PIECES_P and migrating tree-inline.c, we are
> left with only one user of MOVE_RATIO - deciding the maximum size of
> aggregate for SRA.
>
> Past discussions have made it clear [1] that keeping this use of
> MOVE_RATIO is undesirable. Clearly it is now also misnamed.
>
> The previous iteration of this patch was rejected as too complicated. I
> went off and tried simplifying it to use MOVE_RATIO, but if we do that we
> end up breaking some interface boundaries between the driver and the
> backend.
>
> This patch partially hookizes MOVE_RATIO under the new name
> TARGET_MAX_SCALARIZATION_SIZE and uses it to set default values for two
> new parameters:
>
> sra-max-scalarization-size-Ospeed - The maximum size of aggregate
> to consider when compiling for speed
> sra-max-scalarization-size-Osize - The maximum size of aggregate
> to consider when compiling for size.
>
> We then modify SRA to use these parameters rather than MOVE_RATIO.
>
> Bootstrapped and regression tested for x86, arm and aarch64 with no
> issues.
>
> OK for trunk?
+/* Return the maximum size in bytes of aggregate which will be considered
+ for replacement by SRA/IP-SRA. */
+DEFHOOK
+(max_scalarization_size,
+ "This target hook is used by the Scalar Replacement of Aggregates passes\n\
+(SRA and IPA-SRA). This hook gives the maximimum size, in storage units,\n\
+of aggregate to consider for replacement. @var{speed_p} is true if we are\n\
+currently compiling for speed.\n\
+\n\
+By default, the maximum scalarization size is determined by MOVE_RATIO,\n\
+if it is defined. Otherwise, a sensible default is chosen.\n\
doesn't match
+unsigned int
+default_max_scalarization_size (bool speed_p ATTRIBUTE_UNUSED)
+{
+ return get_move_ratio (speed_p) * MOVE_MAX_PIECES;
+unsigned int
+get_max_scalarization_size (bool speed_p)
+{
+ unsigned param_max_scalarization_size
+ = speed_p
+ ? PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED)
+ : PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE);
+
+ if (!param_max_scalarization_size)
+ return targetm.max_scalarization_size (speed_p);
+
the target-hook takes a size_p parameter, here you have a speed_p
parameter but call it as
+ unsigned i;
+ unsigned int max_scalarization_size
+ = get_max_scalarization_size (optimize_function_for_size_p (cfun))
+ * BITS_PER_UNIT;
there is some mismatch. Not sure if we generally prefer speed_p
over size_p, grepping headers shows zero size_p parameters and
some speed_p ones.
Given the special value to note the default for the new --params is
zero a user cannot disable scalarization that way.
I still somehow dislike that you need a target hook to compute the
default. Why doesn't it work to do, in opts.c:default_options_optimization
maybe_set_param_value
(PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED,
get_move_ratio (speed_p) * MOVE_MAX_PIECES,
opts->x_param_values, opts_set->x_param_values);
and override that default in targets option_override hook the same way?
Thanks,
Richard.
> [1]: https://gcc.gnu.org/ml/gcc-patches/2014-08/msg01997.html
>
> ---
> gcc/
>
> 2014-09-25 James Greenhalgh <james.greenha...@arm.com>
>
> * doc/invoke.texi (sra-max-scalarization-size-Ospeed): Document.
> (sra-max-scalarization-size-Osize): Likewise.
> * doc/tm.texi.in
> (MOVE_RATIO): Reduce documentation to a stub, deprecate.
> (TARGET_MAX_SCALARIZATION_SIZE): Add hook.
> * doc/tm.texi: Regenerate.
> * defaults.h (MOVE_RATIO): Remove default implementation.
> (SET_RATIO): Add a default implementation if MOVE_RATIO
> is not defined.
> * params.def (sra-max-scalarization-size-Ospeed): New.
> (sra-max-scalarization-size-Osize): Likewise.
> * target.def (max_scalarization_size): New.
> * targhooks.c (default_max_scalarization_size): New.
> * targhooks.h (default_max_scalarization_size): New.
> * tree-sra.c (get_max_scalarization_size): New.
> (analyze_all_variable_accesses): Use it.
