On Thu, Sep 15, 2016 at 1:21 PM, Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> wrote: > On 15 September 2016 at 16:31, Richard Sandiford > <richard.sandif...@arm.com> wrote: >> Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes: >>> On 15 September 2016 at 04:21, Richard Sandiford >>> <rdsandif...@googlemail.com> wrote: >>>> Richard Sandiford <rdsandif...@googlemail.com> writes: >>>>> Prathamesh Kulkarni <prathamesh.kulka...@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
