On Wed, Oct 29, 2014 at 3:39 PM, James Greenhalgh <james.greenha...@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.greenha...@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 >
