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

[Richard Sandiford <richard.sandiford@arm.com>]

"Kewen.Lin" <linkw@linux.ibm.com> writes:
> on 2020/1/20 下午9:14, Segher Boessenkool wrote:
>> Hi!
>> 
>> On Mon, Jan 20, 2020 at 10:42:12AM +0000, Richard Sandiford wrote:
>>> "Kewen.Lin" <linkw@linux.ibm.com> writes:
>>>> gcc/ChangeLog
>>>>
>>>> 2020-01-16  Kewen Lin  <linkw@gcc.gnu.org>
>>>>
>>>> 	* config/rs6000/rs6000.c (TARGET_STRIDE_DFORM_VALID_P): New macro.
>>>> 	(rs6000_stride_dform_valid_p): New function.
>>>> 	* doc/tm.texi: Regenerate.
>>>> 	* doc/tm.texi.in (TARGET_STRIDE_DFORM_VALID_P): New hook.
>>>> 	* target.def (stride_dform_valid_p): New hook.
>>>
>>> It looks like we should able to derive this information from the normal
>>> legitimate_address_p hook.
>> 
>> Yes, probably.
>> 
>>> Also, "D-form" vs. "X-form" is AFAIK a PowerPC-specific classification.
>>> It would be good to use a more generic term in target-independent code.
>> 
>> Yeah.  X-form is [reg+reg] addressing; D-form is [reg+imm] addressing.
>> We can do simple [reg] addressing in either form as well.  Whether D-form
>> can be used for some access depends on many factors (ISA version, mode of
>> the datum, alignment, and how big the offset is of course).  But the usual
>> legitimate_address_p hook should do fine.  The ivopts code already has an
>> addr_offset_valid_p function, maybe that could be adjusted for this?
>> 
>> 
>> Segher
>> 
>
> 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