diff mbox

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

Message ID CAAgBjMmsPq5az_nvHSv1sui_KXTLSvb7b9aHmm7C-oEfinpD2g@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Sept. 15, 2016, 10:39 a.m. UTC 
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).

However then the issue with expand_twoval_optab_libfunc() still remains, and
I am not sure what to do about that.
As a temporary hack, we could punt if the divmod function's name is
"__udivmoddi4":

which is admittedly quite ugly :/

Thanks,
Prathamesh
>

> Sorry if this has already been covered.

>

> Thanks,

> Richard
diff mbox

Patch

--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -2083,7 +2083,7 @@  expand_twoval_binop_libfunc (optab binoptab, rtx
op0, rtx op1,

   mode = GET_MODE (op0);
   libfunc = optab_libfunc (binoptab, mode);
-  if (!libfunc)
+  if (!libfunc || !strcmp (XSTR (libfunc, 0), "__udivmoddi4"))
     return false;