[PATCH V3] Use preferred mode for doloop IV [PR61837]

[guojiufu]

On 2021-07-27 23:40, Jeff Law wrote:
> On 7/27/2021 12:27 AM, Richard Biener wrote:
>> On Fri, 23 Jul 2021, Jeff Law wrote:
>> 
>>> 
>>> On 7/15/2021 4:08 AM, Jiufu Guo via Gcc-patches wrote:
>>>> Refine code for V2 according to review comments:
>>>> * Use if check instead assert, and refine assert
>>>> * Use better RE check for test case, e.g. (?n)/(?p)
>>>> * Use better wording for target.def
>>>> 
>>>> Currently, doloop.xx variable is using the type as niter which may 
>>>> be
>>>> shorter than word size.  For some targets, it would be better to use
>>>> word size type.  For example, on 64bit system, to access 32bit 
>>>> value,
>>>> subreg maybe used.  Then using 64bit type maybe better for niter if
>>>> it can be present in both 32bit and 64bit.
>>>> 
>>>> This patch add target hook for querg perferred mode for doloop IV.
>>>> And update mode accordingly.
>>>> 
>>>> Bootstrap and regtest pass on powerpc64le, is this ok for trunk?
>>>> 
>>>> BR.
>>>> Jiufu
>>>> 
>>>> gcc/ChangeLog:
>>>> 
>>>> 2021-07-15  Jiufu Guo  <guojiufu@linux.ibm.com>
>>>> 
>>>>   PR target/61837
>>>>   * config/rs6000/rs6000.c (TARGET_PREFERRED_DOLOOP_MODE): New hook.
>>>>   (rs6000_preferred_doloop_mode): New hook.
>>>>   * doc/tm.texi: Regenerate.
>>>>   * doc/tm.texi.in: Add hook preferred_doloop_mode.
>>>>   * target.def (preferred_doloop_mode): New hook.
>>>>   * targhooks.c (default_preferred_doloop_mode): New hook.
>>>>   * targhooks.h (default_preferred_doloop_mode): New hook.
>>>>   * tree-ssa-loop-ivopts.c (compute_doloop_base_on_mode): New 
>>>> function.
>>>>   (add_iv_candidate_for_doloop): Call targetm.preferred_doloop_mode
>>>>   and compute_doloop_base_on_mode.
>>>> 
>>>> gcc/testsuite/ChangeLog:
>>>> 
>>>> 2021-07-15  Jiufu Guo  <guojiufu@linux.ibm.com>
>>>> 
>>>>   PR target/61837
>>>>   * gcc.target/powerpc/pr61837.c: New test.
>>> My first reaction was that whatever type corresponds to the target's 
>>> word_mode
>>> would be the right choice.  But then I remembered things like dbCC on 
>>> m68k
>>> which had a more limited range.  While I don't think m68k uses the 
>>> doloop
>>> bits, it's a clear example that the most desirable type may not 
>>> correspond to
>>> the word type for the target.
>>> 
>>> So my concern with this patch is its introducing more target 
>>> dependencies into
>>> the gimple pipeline which is generally considered undesirable from a 
>>> design
>>> standpoint.  Is there any way to lower from whatever type is chosen 
>>> by ivopts
>>> to the target's desired type at the gimple->rtl border rather than 
>>> doing it in
>>> ivopts?
>> I think that's difficult - after all we want to base other IV uses on
>> the doloop IV if possible.  So IMHO it's not different from IVOPTs
>> choosing different IVs based on RTL costing and target addressing mode
>> availability so I wasn't worried about those additional target
>> dependences at this point of the GIMPLE pipeline.
> Yea, you're probably right on both accounts.   With that resolved I
> think this is OK for the trunk.
> 
> Thanks for your patience Jiufu and thanks for chiming in Richi.

Thanks for all your help!

The patch was committed to r12-2585.

I notice that I ignored one guality case(gfortran.dg/guality/arg1.f90).
It becomes 'unsupported' from 'pass'.  The issue could be reproduced
on a similar test case without this patch.  Just opened PR101669 for it.


BR,
Jiufu

> 
> jeff