On Wed, Nov 6, 2019 at 9:56 AM Martin Liška <mli...@suse.cz> wrote: > > On 11/5/19 5:01 PM, Richard Biener wrote: > > On Tue, Nov 5, 2019 at 4:22 PM Martin Liška <mli...@suse.cz> wrote: > >> > >> On 11/5/19 3:13 PM, Richard Biener wrote: > >>> On Thu, Oct 31, 2019 at 2:17 PM Martin Liška <mli...@suse.cz> wrote: > >>>> > >>>> On 10/31/19 2:16 PM, Martin Liška wrote: > >>>>> On 10/31/19 2:01 PM, Martin Liška wrote: > >>>>>> Hi. > >>>>>> > >>>>>> Based on the discussion with Honza and Richard I'm sending a proposal > >>>>>> for conversion of param machinery into the existing option machinery. > >>>>>> Our motivation for the change is to provide per function param values, > >>>>>> similarly what 'Optimization' keyword does for options. > >>>>>> > >>>>>> Right now, we support the following format: > >>>>>> gcc --param=lto-partitions=4 /tmp/main.c -c > >>>>>> > >>>>>> And so that I decided to name newly the params like: > >>>>>> > >>>>>> -param=ipa-sra-ptr-growth-factor= > >>>>>> Common Joined UInteger Var(param_ipa_sra_ptr_growth_factor) Init(2) > >>>>>> Param Optimization > >>>>>> Maximum allowed growth of number and total size of new parameters > >>>>>> that ipa-sra replaces a pointer to an aggregate with. > >>>>>> > >>>>>> And I learnt decoder to parse '--param' 'name=value' as > >>>>>> '--param=name=value'. Doing that > >>>>>> the transformation works. Help provides reasonable output as well: > >>>>>> > >>>>>> $ ./xgcc -B. --param predictable-branch-outcome=5 /tmp/main.c -c -Q > >>>>>> --help=param > >>>>>> The --param option recognizes the following as parameters: > >>>>>> --param=ipa-sra-ptr-growth-factor= 2 > >>>>>> --param=predictable-branch-outcome=<0,50> 5 > >>>>>> > >>>>>> Thoughts? > >>>>>> Thanks, > >>>>>> Martin > >>>>>> > >>>>>> --- > >>>>>> gcc/common.opt | 18 +++++++++++------- > >>>>>> gcc/ipa-sra.c | 3 +-- > >>>>>> gcc/opt-functions.awk | 3 ++- > >>>>>> gcc/opts-common.c | 9 +++++++++ > >>>>>> gcc/opts.c | 36 ------------------------------------ > >>>>>> gcc/params.def | 10 ---------- > >>>>>> gcc/predict.c | 4 ++-- > >>>>>> 7 files changed, 25 insertions(+), 58 deletions(-) > >>>>>> > >>>>>> > >>>>> > >>>>> I forgot to add gcc-patches to To. > >>>>> > >>>>> Martin > >>>>> > >>>> > >>>> + the patch. > >>> > >>> Nice. > >> > >> Thanks. > >> > >>> > >>> I wonder if we can auto-generate params.h so that > >>> PARAM_VALUE (...) can continue to "work"? But maybe that's too much > >>> and against making them first-class (but "unsupported") options. At least > >>> it would make the final patch _much_ smaller... (one could think of > >>> auto-generating an enum and using an array of params for the storage > >>> again - but then possibly split for [non-]Optimization - ugh). If we > >>> (auto-)name > >>> the variables all-uppercase like PARAM_IPA_SRA_PTR_GROWTH_FACTOR > >>> we could have > >>> > >>> #define PARAM_VALUE (x) x > >>> > >>> ... (that said, everything that helps making the transition hit GCC 10 > >>> is appreciated ;)) > >> > >> Well, to be honest I would like to get rid of the current syntax: > >> PARAM_VALUE (PARAM_PREDICTABLE_BRANCH_OUTCOME) and replace it with > >> something > >> what we have for normal options: param_predictable_branch_outcome. > >> It will require a quite big mechanical change in usage, but I can do > >> the replacement almost automatically. > > > > OK, the more interesting uses are probably maybe_set_param_value ... > > Which is actually for free ;) Please see the attached patch where I introduced > a new macro SET_OPTION_IF_UNSET.
works for me. > Do you like it so that I can transform current options with that: > > - if (!opts_set->x_flag_branch_probabilities) > - opts->x_flag_branch_probabilities = value; > + SET_OPTION_IF_UNSET (opts, opts_set, flag_branch_probabilities, value); > > ? > > > > >>> > >>> For > >>> > >>> +-param=ipa-sra-ptr-growth-factor= > >>> +Common Joined UInteger Var(param_ipa_sra_ptr_growth_factor) Init(2) > >>> Param Optimization > >>> > >>> I wonder if both Var(...) and Param can be "autodetected" (aka > >>> actually required)? > >> > >> Right now, Var is probably not required. Param can be handled by putting > >> all > >> the params into a new param.opt file. > > > > Param could be also autodetected from the name of the option (so could Var). > > Well, looking at the current options, we are also quite explicit about > various option flags. > I'll auto-generate the new params.opt file, so setting Var and Param will be > for free. ok > > Why is Var not required? > > You are right, it's required. > > > > >>> > >>> At least the core of the patch looks nicely small! How do the OPT_ enum > >>> values > >>> for a --param look like? > >> > >> Yep, I also like how small it is. > >> > >> OPT__param_ipa_sra_ptr_growth_factor_ = 62,/* > >> --param=ipa-sra-ptr-growth-factor= */ > >> OPT__param_predictable_branch_outcome_ = 63,/* > >> --param=predictable-branch-outcome= */ > > > > OK, looks fine. > > > > So I guess go ahead unless somebody objects over the next weekend? (in > > case not > > Sure. I asked Honza about feedback and he looks happy with what I suggested. Yeah, looks nice. handle_param should be dead so should quite some more param infrastructure. Please try to sort params alphabetically. Thanks, Richard. > > you have another week to do the mechanical change?) Maybe post a final > > patch > > earlier w/o the mechanical work. > > Martin > > > > > Richard. > > > >> Martin > >> > >>> > >>> Thanks, > >>> Richard. > >>> > >>>> Martin > >> >
