>> Hi Segher and Richard S., >> >> Sorry for late response. Thanks for your comments on legitimate_address_p >> hook >> and function addr_offset_valid_p. I updated the IVOPTs part with >> addr_offset_valid_p, although rs6000_legitimate_offset_address_p doesn't >> check >> strictly all the time (like worst_case is false), it works well with >> SPEC2017. >> Based on it, the hook is simplified as attached patch. > > Thanks for the update. I think it would be better to add a --param > rather than a bool hook though. Targets can then change the default > (if necessary) using SET_OPTION_IF_UNSET. The user can override the > default if they want to. > > It might also be better to start with an opt-out rather than an opt-in > (i.e. with the default param value being true rather than false). > With a default-off option, it's much harder to tell whether something > has been deliberately turned off or whether no-one's thought about it > either way. We can always flip the default later if it turns out that > nothing other than rs6000 benefits. > > Richard >
Hi Richard, Thanks for your comments! It's a good idea to use param due to the flexibility. And yes, it sounds good to have more targets to try and make it better. But I have a bit concern on turning it on by default. Since it replies on unroll factor estimation, as part 1/4 shows, it calls targetm.loop_unroll_adjust if target supports, which used to work on RTL level. To avoid possible ICE, I'm intended to turn it off for those targets (s390 & i386) with that hook, since without good understanding on those targets, it's hard for me to extend them with gimple level support. Does it make sense? The updated patch has been attached. BR, Kewen --------- gcc/ChangeLog 2020-03-03 Kewen Lin <li...@gcc.gnu.org> * doc/invoke.texi (iv-consider-reg-offset-for-unroll): Document new option. * params.opt (iv-consider-reg-offset-for-unroll): New. * config/s390/s390.c (s390_option_override_internal): Disable parameter iv-consider-reg-offset-for-unroll by default. * config/i386/i386-options.c (ix86_option_override_internal): Likewise.
diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c index e0be493..41c99b3 100644 --- a/gcc/config/i386/i386-options.c +++ b/gcc/config/i386/i386-options.c @@ -2902,6 +2902,12 @@ ix86_option_override_internal (bool main_args_p, if (ix86_indirect_branch != indirect_branch_keep) SET_OPTION_IF_UNSET (opts, opts_set, flag_jump_tables, 0); + /* Disable this for now till loop_unroll_adjust supports gimple level checks, + to avoid possible ICE. */ + if (opts->x_optimize >= 1) + SET_OPTION_IF_UNSET (opts, opts_set, + param_iv_consider_reg_offset_for_unroll, 0); + return true; } diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c index ebba670..ae4c2bd 100644 --- a/gcc/config/s390/s390.c +++ b/gcc/config/s390/s390.c @@ -15318,6 +15318,12 @@ s390_option_override_internal (struct gcc_options *opts, not the case when the code runs before the prolog. */ if (opts->x_flag_fentry && !TARGET_64BIT) error ("%<-mfentry%> is supported only for 64-bit CPUs"); + + /* Disable this for now till loop_unroll_adjust supports gimple level checks, + to avoid possible ICE. */ + if (opts->x_optimize >= 1) + SET_OPTION_IF_UNSET (opts, opts_set, + param_iv_consider_reg_offset_for_unroll, 0); } static void diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index fa98e2f..502031c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -12220,6 +12220,15 @@ If the number of candidates in the set is smaller than this value, always try to remove unnecessary ivs from the set when adding a new one. +@item iv-consider-reg-offset-for-unroll +When RTL unrolling performs on a loop, the duplicated loop iterations introduce +appropriate induction variable step update expressions. But if an induction +variable is derived from address object, it is profitable to fill its required +offset updates into appropriate memory access expressions if target memory +accessing supports the register offset mode and the resulted offset is in the +valid range. The induction variable optimizations consider this information +for better unrolling code. It requires unroll factor estimation in middle-end. + @item avg-loop-niter Average number of iterations of a loop. diff --git a/gcc/params.opt b/gcc/params.opt index 8e4217d..31424cf 100644 --- a/gcc/params.opt +++ b/gcc/params.opt @@ -270,6 +270,10 @@ Bound on number of candidates below that all candidates are considered in iv opt Common Joined UInteger Var(param_iv_max_considered_uses) Init(250) Param Optimization Bound on number of iv uses in loop optimized in iv optimizations. +-param=iv-consider-reg-offset-for-unroll= +Common Joined UInteger Var(param_iv_consider_reg_offset_for_unroll) Init(1) Optimization IntegerRange(0, 1) Param +Whether iv optimizations mark register offset valid groups and consider their derived iv candidates would be profitable with estimated unroll factor consideration. + -param=jump-table-max-growth-ratio-for-size= Common Joined UInteger Var(param_jump_table_max_growth_ratio_for_size) Init(300) Param Optimization The maximum code size growth ratio when expanding into a jump table (in percent). The parameter is used when optimizing for size.