Re: [PATCH 2/4 GCC11] Add target hook stride_dform_valid_p

["Kewen.Lin" <linkw@linux.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2116 bytes --]

>> 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  <linkw@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.

[-- Attachment #2: param.diff --]
[-- Type: text/plain, Size: 3232 bytes --]

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.