Re: [Patchv2 3/4] Control SRA and IPA-SRA by a param rather than MOVE_RATIO

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Oct 29, 2014 at 3:39 PM, James Greenhalgh
<james.greenhalgh@arm.com> wrote:
> On Wed, Oct 01, 2014 at 05:38:12PM +0100, James Greenhalgh wrote:
>> On Fri, Sep 26, 2014 at 10:11:13AM +0100, Richard Biener wrote:
>> > On Thu, Sep 25, 2014 at 4:57 PM, James Greenhalgh
>> > <james.greenhalgh@arm.com> wrote:
>> > 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?
>>
>> The problem I am having is getting "get_move_ratio" right, without breaking
>> the modular design.
>>
>> default_options_optimization, and the rest of opts.c is going to end up in
>> libcommon-target.a, so we are not going to have access to any
>> backend-specific symbols.
>>
>> An early draft of this patch used the MOVE_RATIO macro to set the default
>> value. This worked fine for AArch64 and ARM targets (which both use a
>> simple C expression for MOVE_RATIO), but failed for x86_64 which defines
>> MOVE_RATIO as so:
>>
>>   #define MOVE_RATIO(speed) ((speed) ? ix86_cost->move_ratio : 3)
>>
>> Dealing with that ix86_cost symbol is what causes us the pain.
>>
>> It seems reasonable that a target might want to define MOVE_RATIO
>> as some function of their tuning parameters, so I don't want to
>> disallow that usage.
>>
>> This inspired me to try turning this in to a target hook, but this
>> doesn't help as opts.c only gets access to common-target.def target
>> hooks. These suffer the same problem, they don't have access to any
>> backend symbols.
>>
>> I suppose I could port any target with a definition of MOVE_RATIO to
>> override the default parameter value in their option overriding code,
>> but that makes this a very large patch set (many targets define
>> MOVE_RATIO).
>>
>> Is this an avenue worth exploring? I agree the very special target
>> hook is not ideal.
>
> Hi,
>
> Did you have any further thoughts on this? I'm still unable to come up
> with a way to set these parameters which allows them to default to their
> current (MOVE_RATIO derived) values.
>
> If the only way to make this work is to add code to
> TARGET_OPTION_OVERRIDE for all targets that define MOVE_RATIO, then I
> suppose I can do that, but I'd prefer a neater way to make it work, if
> you can think of one.

Maybe instead of putting the code in opts.c put it right before we call
targetm.target_option.override () in toplev.c:process_options.  With
a comment on why it cannot be in opts.c.

Thanks,
Richard.

> Thanks,
> James
>