From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55616 invoked by alias); 15 Sep 2016 11:21:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 55591 invoked by uid 89); 15 Sep 2016 11:21:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_00,LIKELY_SPAM_BODY,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=profit X-HELO: mail-it0-f52.google.com Received: from mail-it0-f52.google.com (HELO mail-it0-f52.google.com) (209.85.214.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 Sep 2016 11:21:37 +0000 Received: by mail-it0-f52.google.com with SMTP id n143so69791006ita.1 for ; Thu, 15 Sep 2016 04:21:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=gAAvx4yiaETmSGapSJKxXN9STRWQ6uZkYuCbuLq1FQo=; b=FxTKxl1lJ56BVqlRxV0rHGIWSwXUwNzGKo1U9jgQlY+Lh4euA6W1pzWj3P5a9EHKmU t8VcHVL9OLoRsLgnOuWEGi1JLsInKrJ0yaA6uzZYDvWTWouFksRM/eiLD0A0lnijydRI Shsq690YB/qFOAHefQ/ZqWdZRqcoN79ELfFszU+k/QywvsM3CwqY0vS9ndR9cCQaefel SRXkSHB42GnZO2VNPO5DA0qjhBv302XqbpGmvRhmilNteygRiF4TZx1daNO22dxvJMXs ra2wMD3zyzBNlfaDNhkrizLh190MHLbVXHshoV8LvF2DkA+1jueKpA0st9uLerjBfHLV 8mYw== X-Gm-Message-State: AE9vXwOH6+HBbVFrQBSjVSNyhYknDJnuKJAK4xpHMabnSlocWamFm1yOnpl9XADTxPVDtL26oi/B3XPhSWMikmX2 X-Received: by 10.107.132.157 with SMTP id o29mr18959156ioi.109.1473938494599; Thu, 15 Sep 2016 04:21:34 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.81.85 with HTTP; Thu, 15 Sep 2016 04:21:33 -0700 (PDT) In-Reply-To: <878tut77fg.fsf@e105548-lin.cambridge.arm.com> References: <87y42urtza.fsf@googlemail.com> <87twdirt7c.fsf@googlemail.com> <878tut77fg.fsf@e105548-lin.cambridge.arm.com> From: Prathamesh Kulkarni Date: Thu, 15 Sep 2016 12:16:00 -0000 Message-ID: Subject: Re: [PING] set libfunc entry for sdivmod_optab to NULL in optabs.def To: Prathamesh Kulkarni , gcc Patches , Richard Sandiford Content-Type: multipart/mixed; boundary=001a113f296464dfb2053c8a0c21 X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00933.txt.bz2 --001a113f296464dfb2053c8a0c21 Content-Type: text/plain; charset=UTF-8 Content-length: 5044 On 15 September 2016 at 16:31, Richard Sandiford wrote: > Prathamesh Kulkarni writes: >> On 15 September 2016 at 04:21, Richard Sandiford >> wrote: >>> Richard Sandiford writes: >>>> Prathamesh Kulkarni 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 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. Thanks, Prathamesh > > Thanks, > Richard --001a113f296464dfb2053c8a0c21 Content-Type: text/plain; charset=US-ASCII; name="foo.diff" Content-Disposition: attachment; filename="foo.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_it48johm0 Content-length: 1277 ZGlmZiAtLWdpdCBhL2djYy9vcHRhYnMuZGVmIGIvZ2NjL29wdGFicy5kZWYK aW5kZXggODg3NWUzMC4uYjM3YWMyZSAxMDA2NDQKLS0tIGEvZ2NjL29wdGFi cy5kZWYKKysrIGIvZ2NjL29wdGFicy5kZWYKQEAgLTExNiw4ICsxMTYsOCBA QCBPUFRBQl9OTChzc2Rpdl9vcHRhYiwgInNzZGl2JFEkYTMiLCBTU19ESVYs ICJzc2RpdiIsICczJywgZ2VuX3NpZ25lZF9maXhlZF9saWJmdQogT1BUQUJf TkwodWRpdl9vcHRhYiwgInVkaXYkSSRhMyIsIFVESVYsICJ1ZGl2IiwgJzMn LCBnZW5faW50X3Vuc2lnbmVkX2ZpeGVkX2xpYmZ1bmMpCiBPUFRBQl9OWCh1 ZGl2X29wdGFiLCAidWRpdiRRJGEzIikKIE9QVEFCX05MKHVzZGl2X29wdGFi LCAidXNkaXYkUSRhMyIsIFVTX0RJViwgInVzZGl2IiwgJzMnLCBnZW5fdW5z aWduZWRfZml4ZWRfbGliZnVuYykKLU9QVEFCX05MKHNkaXZtb2Rfb3B0YWIs ICJkaXZtb2QkYTQiLCBVTktOT1dOLCAiZGl2bW9kIiwgJzQnLCBnZW5faW50 X2xpYmZ1bmMpCi1PUFRBQl9OTCh1ZGl2bW9kX29wdGFiLCAidWRpdm1vZCRh NCIsIFVOS05PV04sICJ1ZGl2bW9kIiwgJzQnLCBnZW5faW50X2xpYmZ1bmMp CitPUFRBQl9OTChzZGl2bW9kX29wdGFiLCAiZGl2bW9kJGE0IiwgVU5LTk9X TiwgImRpdm1vZCIsICc0JywgTlVMTCkgCitPUFRBQl9OTCh1ZGl2bW9kX29w dGFiLCAidWRpdm1vZCRhNCIsIFVOS05PV04sICJ1ZGl2bW9kIiwgJzQnLCBO VUxMKSAKIE9QVEFCX05MKHNtb2Rfb3B0YWIsICJtb2QkYTMiLCBNT0QsICJt b2QiLCAnMycsIGdlbl9pbnRfbGliZnVuYykKIE9QVEFCX05MKHVtb2Rfb3B0 YWIsICJ1bW9kJGEzIiwgVU1PRCwgInVtb2QiLCAnMycsIGdlbl9pbnRfbGli ZnVuYykKIE9QVEFCX05MKGZ0cnVuY19vcHRhYiwgImZ0cnVuYyRGJGEyIiwg VU5LTk9XTiwgImZ0cnVuYyIsICcyJywgZ2VuX2ZwX2xpYmZ1bmMpCg== --001a113f296464dfb2053c8a0c21--