From patchwork Thu Sep 15 10:39:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prathamesh Kulkarni X-Patchwork-Id: 76261 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp2365528qgf; Thu, 15 Sep 2016 03:40:15 -0700 (PDT) X-Received: by 10.98.56.207 with SMTP id f198mr13402822pfa.83.1473936015368; Thu, 15 Sep 2016 03:40:15 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id o3si2317808pax.24.2016.09.15.03.40.15 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Sep 2016 03:40:15 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-435965-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-435965-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-435965-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:content-type; q=dns; s=default; b=i956K/X3QCmL3Vv6NC Mogd1J/qYw94lJUYAzyXeXFMecb9blW+ZR5Q7Gq8kGHglIi/A3kjl09sjNH2DOWr mDfpqIdentMw26nb+wBHMtLJgoG8a5+QEovivw/Di9P6/jEmz4ce7/v+aQgUcuSD DNRj0ITvs1HB0m32memzIVwL4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:content-type; s=default; bh=pz5ZSrUXhKePOj/kcxLLA8ve W54=; b=x+2wXudTG/9CKWoPDhrL92+qDhW6QNLEGstRDoYtczl+nDRq13eXBKD4 YvNMetjRaVnLUCbet2OACiCC26Q52lvdHZNhBNvXGN2Gsb9V7a1leGbn6310T6Rm P//uQLnNZXx9qlis6zr2H1uvuFHoExZOO3Jl7FUoNlo2WPMWCac= Received: (qmail 73313 invoked by alias); 15 Sep 2016 10:39:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 70515 invoked by uid 89); 15 Sep 2016 10:39:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=prathamesh.kulkarni@linaro.org, prathameshkulkarnilinaroorg, rdsandiford@googlemail.com, rdsandifordgooglemailcom 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 10:39:48 +0000 Received: by mail-it0-f52.google.com with SMTP id r192so87883011ita.0 for ; Thu, 15 Sep 2016 03:39:47 -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=xjq81rDtZH/IjZNw5G0jp4RZsmFAeLHslWWVwdz64lY=; b=Pw09At++l/w6/v4tCVxRagGfoxWSwhYb/LnCkxHg34wiw3W+12nDcHftMpWjJoj+fc bfHouqZ0tENh+WfRvApvlAgRVRWD+tESUyADmpKbJ5OeE50GKnb9///CwSfaQiLoYikX RjJG1IiCW1tXnYh3N2tIJeWWQ6jImLf74y0+mfDD+p8RCyGAtcnb758lM09xLFQkkzEV 738pBqehvf8jtGtYphdtvIuABeUOUiS/DkziGZUIr2ce/UjXyQWN+7Z9IxVaV0e1yNfO xVt06DmxwiDjTKj1xCo7oRzqMR0Yi2F4Bwz4r58jIjx6u7SK8G9YjAmcK2+r6sgAz9u5 HimA== X-Gm-Message-State: AE9vXwM2QL6Y97Qqw3GH0SEz7xpNUy2EiABXLQgp9JyP2nu7oQ7NY9IXE5iW/e4Eof7as2LnMwBzTnRAHzYSAx1o X-Received: by 10.107.40.73 with SMTP id o70mr18103158ioo.137.1473935986425; Thu, 15 Sep 2016 03:39:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.81.85 with HTTP; Thu, 15 Sep 2016 03:39:45 -0700 (PDT) In-Reply-To: <87twdirt7c.fsf@googlemail.com> References: <87y42urtza.fsf@googlemail.com> <87twdirt7c.fsf@googlemail.com> From: Prathamesh Kulkarni Date: Thu, 15 Sep 2016 16:09:45 +0530 Message-ID: Subject: Re: [PING] set libfunc entry for sdivmod_optab to NULL in optabs.def To: Prathamesh Kulkarni , gcc Patches , Richard Sandiford , rdsandiford@googlemail.com X-IsSubscribed: yes 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). 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 --- 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;