[PING] set libfunc entry for sdivmod_optab to NULL in optabs.def

[Richard Biener]

On Thu, Sep 15, 2016 at 1:21 PM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 15 September 2016 at 16:31, Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>>> On 15 September 2016 at 04:21, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>>>> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes:
>>>>>> Hi,
>>>>>> I would like to ping the following patch:
>>>>>> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
>>>>>>
>>>>>> While implementing divmod transform:
>>>>>> https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01757.html
>>>>>>
>>>>>> I ran into an  issue with optab_libfunc().
>>>>>> It appears optab_libfunc (sdivmod_optab, mode) returns
>>>>>> bogus libfunc for unsupported modes, for instance
>>>>>> on x86_64, optab_libfunc (sdivmod_optab, DImode) returns
>>>>>> a libfunc with name "__divmoddi4", even though such a libfunc
>>>>>> does not exist in libgcc. This happens because in optabs.def
>>>>>> the libfunc entry for sdivmod_optab has gen_int_libfunc,
>>>>>> and call to optab_libfunc (sdivmo_optab, DImode) lazily
>>>>>> creates a bogus libfunc "__divmoddi4" by calling gen_int_libfunc().
>>>>>>
>>>>>> To work around this issue I set libfunc entry for sdivmod_optab to NULL
>>>>>> and verified that optab_libfunc (sdivmod_optab, DImode) returns NULL_RTX
>>>>>> instead of a bogus libfunc if it's not overriden by the target.
>>>>>>
>>>>>> Bootstrapped and tested on ppc64le-linux-gnu, x86_64-linux-gnu.
>>>>>> Cross tested on arm*-*-*, aarch64*-*-*.
>>>>>> OK for trunk ?
>>>>>
>>>>> I'm not a maintainer for this area, but:
>>>>
>>>> ...in https://gcc.gnu.org/ml/gcc-patches/2016-05/msg01757.html
>>>> you said that c6x follows the return-by-pointer convention.
>>>> I'm no c6x expert, but from a quick look, I think its divrem
>>>> function returns a div/mod pair in A4/A5, which matches the
>>>> ARM convention of returning both results by value.
>>>>
>>>> Does anyone know if the optab function registered by the SPU
>>>> backend is ever called directly?
>>>>
>>>> You mention that this is latent as far as expand_twoval_binop_libfunc
>>>> is concerned.  AIUI expand_twoval_binop_libfunc implements the ARM/c6x
>>>> convention and expects the two values to be returned as a pair.
>>>> It then extracts one half of the pair and discards the other.
>>>> So my worry is that we're leaving the udivmod entry intact even though
>>>> the standard __udivmoddi4 doesn't do what expand_twoval_binop_libfunc
>>>> expects.
>>>>
>>>> Would it make sense to set both entries to null and treat __udivmoddi4
>>>> as a non-optab function?  ARM and c6x could then continue to register
>>>> their current optab functions and a non-null optab function would
>>>> indicate a return value pair.
>>> AFAIU, there are only three targets (c6x, spu, arm) that override
>>> optab_libfunc for udivmod_optab for following modes:
>>> ./c6x/c6x.c:  set_optab_libfunc (udivmod_optab, SImode, "__c6xabi_divremu");
>>> ./c6x/c6x.c:  set_optab_libfunc (udivmod_optab, DImode, "__c6xabi_divremull");
>>> ./arm/arm.c:  set_optab_libfunc (udivmod_optab, DImode, "__aeabi_uldivmod");
>>> ./arm/arm.c:  set_optab_libfunc (udivmod_optab, SImode, "__aeabi_uidivmod");
>>> ./spu/spu.c:  set_optab_libfunc (udivmod_optab, DImode, "__udivmoddi4");
>>> ./spu/spu.c:  set_optab_libfunc (udivmod_optab, TImode, "__udivmodti4");
>>>
>>> Out of these only the arm, and c6x have target-specific divmod libfuncs which
>>> return <div, mod> pair, while spu merely makes it point to the
>>> standard functions.
>>>
>>> So we could set libfunc entry for udivmod_optab to NULL, thus dropping
>>> support for generic
>>> divmod functions (__udivmoddi4, __udivmodti4). For targets that
>>> require standard divmod libfuncs like __udivmoddi4,
>>> they could explicitly override  optab_libfunc and set it to
>>> __udivmoddi4, just as spu does.
>>>
>>> However this implies that non-null optab function doesn't necessarily
>>> follow arm/c6x convention.
>>> (i686-gcc for instance generates call to libgcc routines
>>> __udivdi3/__umoddi3 for DImode division/mod operations
>>> and could profit from divmod transform by calling __udivmoddi4).
>>
>> What I meant was that we shouldn't treat udivmoddi4 as an optab function
>> at all, but handle it with some on-the-side mechanism.  That seems like
>> quite a natural fit if we handle the fused div/mod operation as an
>> internal function during gimple.
> Ah right, thanks for pointing out. So if optab function for [us]divmod_optab
> is defined, then it must follow the arm/c6x convention ?
>>
>> I think the current SPU code is wrong, but it looks like a latent bug.
>> (Like I say, does the udivmodti4 function that it registers ever
>> actually get used?  It seems unlikely.)
>>
>> In that scenario no other targets should do what SPU does.
> I am testing the following patch which sets libfunc entries for both
> sdivmod_optab, udivmod_optab to NULL.
> This won't change the current (broken) behavior for SPU port since it
> explicitly overrides optab_libfunc for udivmod_optab
> and sets it to __udivmoddi4.

Just

-OPTAB_NL(sdivmod_optab, "divmod$a4", UNKNOWN, "divmod", '4', gen_int_libfunc)
-OPTAB_NL(udivmod_optab, "udivmod$a4", UNKNOWN, "udivmod", '4', gen_int_libfunc)
+OPTAB_NC(sdivmod_optab, "divmod$a4", UNKNOWN)
+OPTAB_NC(udivmod_optab, "udivmod$a4", UNKNOWN)

If I remember correctly.

Richard.


> Thanks,
> Prathamesh
>>
>> Thanks,
>> Richard