From patchwork Sat Aug 13 11:43:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prathamesh Kulkarni X-Patchwork-Id: 73890 Delivered-To: patch@linaro.org Received: by 10.140.29.52 with SMTP id a49csp588364qga; Sat, 13 Aug 2016 04:43:56 -0700 (PDT) X-Received: by 10.66.234.161 with SMTP id uf1mr35622158pac.93.1471088636180; Sat, 13 Aug 2016 04:43:56 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id b132si14494951pfb.196.2016.08.13.04.43.55 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 13 Aug 2016 04:43:56 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-433944-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-433944-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-433944-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:cc:content-type; q=dns; s=default; b=q0mHWz4U+WmVKO3 gXu2zt5gF/czdhi5pYdW+CCODQ9SVtH9AK9XZ9mEHM/9Ium9g68jzSUtObqpUKR/ vS3peaSU9QkRvc+eKABuvbDk/PMOj/oPPDsk+MlhwMxY87Qo2yY1L4sN4M6xmOnV +ihTELpCQpc1gX96crxLadblACjc= 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:cc:content-type; s=default; bh=Uq9H2UpQoExUs54ChLQ1p ok+Fl8=; b=D8AtseM3tKE7/fCFHZWMiqH9CcqonzNasUn3mN76vBfOOkBxf/0xC 91BVedbDc24ivnT6OrXulrzTvy31Cn7fFRpkyZGdijBO27rccI40FEoqtwoJZ2Xw 08IoNNzfrmMQZe3SRtNZ2otfjMAWJLiO/Msk0hvo06K1lsCJ5cN3o8= Received: (qmail 35399 invoked by alias); 13 Aug 2016 11:43:39 -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 35388 invoked by uid 89); 13 Aug 2016 11:43:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-io0-f176.google.com Received: from mail-io0-f176.google.com (HELO mail-io0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 13 Aug 2016 11:43:28 +0000 Received: by mail-io0-f176.google.com with SMTP id 38so46227747iol.0 for ; Sat, 13 Aug 2016 04:43:27 -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:cc; bh=e62bPNf+KQVkCK8sZFRNDD3/XTFa7ooPNs3WQkfaY58=; b=WjdyQIAg40URo+kD+di43jUgbScG41dsYSZ1qusk5EYAa9dMIY9W0LOHYwK3WUj2fK IXasTnb18rErrLDsBFq1G/eMD6PSK4bRmFg3WOy3ybDLFqKKg4YtgKejgBTNmvmRyk06 3ifSgLdGqNJEyq6RkUbrn/jQwo5/uWCNG7StSLa8Jw9pKv/zCTKt9/CbyUetofvngrbq s0z+BASYWE00jzaTyNWsm7fnNw/bBsaq7MY1Yh3FunzXu8IvPb4+tbjzSdJzxugnPReu iPc6LDonJL8iCS3jv9YTUwkOV4pj/E9kaee+KeB5+zWouLUf6CuC08pwjIuXu8ZKUHUk V2tw== X-Gm-Message-State: AEkoouuIToh6jKBKiU+6yUg5JmZ6sTi24emBetMGu+Wp7TFkRIDpBj5YxTOriXoFug0a74uB+3J6J51+17Jbi0xX X-Received: by 10.107.130.81 with SMTP id e78mr25536015iod.137.1471088606326; Sat, 13 Aug 2016 04:43:26 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.208.18 with HTTP; Sat, 13 Aug 2016 04:43:25 -0700 (PDT) In-Reply-To: References: From: Prathamesh Kulkarni Date: Sat, 13 Aug 2016 17:13:25 +0530 Message-ID: Subject: Re: RFC [1/2] divmod transform To: Richard Biener Cc: Jim Wilson , Richard Biener , gcc Patches , Ramana Radhakrishnan , Kugan Vivekanandarajah , "Joseph S. Myers" X-IsSubscribed: yes On 13 August 2016 at 16:56, Prathamesh Kulkarni wrote: > On 28 July 2016 at 19:05, Prathamesh Kulkarni > wrote: >> On 8 June 2016 at 19:53, Richard Biener wrote: >>> On Fri, 3 Jun 2016, Jim Wilson wrote: >>> >>>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener wrote: >>>> > Joseph - do you know sth about why there's not a full set of divmod >>>> > libfuncs in libgcc? >>>> >>>> Because udivmoddi4 isn't a libfunc, it is a helper function for the >>>> div and mov libfuncs. Since we can compute the signed div and mod >>>> results from udivmoddi4, there was no need to also add a signed >>>> version of it. It was given a libfunc style name so that we had the >>>> option of making it a libfunc in the future, but that never happened. >>>> There was no support for calling any divmod libfunc until it was added >>>> as a special case to call an ARM library (not libgcc) function. This >>>> happened here >>>> >>>> 2004-08-09 Mark Mitchell >>>> >>>> * config.gcc (arm*-*-eabi*): New target. >>>> * defaults.h (TARGET_LIBGCC_FUNCS): New macro. >>>> (TARGET_LIB_INT_CMP_BIASED): Likewise. >>>> * expmed.c (expand_divmod): Try a two-valued divmod function as a >>>> last resort. >>>> ... >>>> * config/arm/arm.c (arm_init_libfuncs): New function. >>>> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT. >>>> (TARGET_INIT_LIBFUNCS): Define it. >>>> ... >>>> >>>> Later, two ports added their own divmod libfuncs, but I don't see any >>>> evidence that they were ever used, since there is no support for >>>> calling divmod other than the expand_divmod last resort code that only >>>> triggers for ARM. >>>> >>>> It is only now that Prathamesh is adding gimple support for divmod >>>> operations that we need to worry about getting this right, without >>>> breaking the existing ARM library support or the existing udivmoddi4 >>>> support. >>> >>> Ok, so as he is primarily targeting the special arm divmod libcall >>> I suppose we can live with special-casing libcall handling to >>> udivmoddi3. It would be nice to not lie about divmod availablilty >>> as libcall though... - it looks like the libcall is also guarded >>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically >>> like on x86). >>> >>> So not sure where to go from here. >> Hi, >> I have attached patch, which is rebased on trunk. >> Needed to update divmod-7.c, which now gets transformed to divmod >> thanks to your code-hoisting patch -;) >> We still have the issue of optab_libfunc() returning non-existent >> libcalls. As in previous patch, I am checking >> explicitly for "__udivmoddi4", with a FIXME note. >> I hope that's okay for now ? >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu, >> armv8l-unknown-linux-gnueabihf. >> Bootstrap+test in progress on i686-linux-gnu. >> Cross-tested on arm*-*-*. > Hi Richard, > I have following two approaches to workaround optab_libfunc issue: > > a) Not lie about divmod libfunc availability by setting libcall entry to NULL > for sdivmod_optab in optabs.def. > Patch posted for that here: > https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html > Although it doesn't cause any regressions with the gcc testsuite, > I am not sure if this change is correct. > > b) Perform the transform only if target-specific divmod is available, > ie, drop targeting > __udivmoddi4. I have attached (untested) patch for that. > When/If we have the optab_libfunc issue resolved, we can later target "generic" > divmod libfunc. Oops, small mistake in the previous patch. We also want to check if target has optab_libfunc set for the given mode. Corrected in this version. Thanks, Prathamesh > > Do either of these approaches look reasonable ? > > PS: I am on vacation next week, will get back to working on patch > after returning. > > Thanks, > Prathamesh >> >> Thanks, >> Prathamesh >>> >>> Richard. diff --git a/gcc/targhooks.c b/gcc/targhooks.c index f506a83..618c810 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -2012,28 +2012,14 @@ default_max_noce_ifcvt_seq_cost (edge e) DImode __udivmoddi4 (DImode op0, DImode op1, DImode *rem). */ void -default_expand_divmod_libfunc (bool unsignedp, machine_mode mode, - rtx op0, rtx op1, - rtx *quot_p, rtx *rem_p) +default_expand_divmod_libfunc (bool unsignedp ATTRIBUTE_UNUSED, + machine_mode mode ATTRIBUTE_UNUSED, + rtx op0 ATTRIBUTE_UNUSED, + rtx op1 ATTRIBUTE_UNUSED, + rtx *quot_p ATTRIBUTE_UNUSED, + rtx *rem_p ATTRIBUTE_UNUSED) { - gcc_assert (mode == DImode); - gcc_assert (unsignedp); - - rtx libfunc = optab_libfunc (udivmod_optab, DImode); - gcc_assert (libfunc); - gcc_assert (!strcmp (XSTR (libfunc, 0), "__udivmoddi4")); - - rtx remainder = assign_stack_temp (DImode, GET_MODE_SIZE (DImode)); - rtx address = XEXP (remainder, 0); - - rtx quotient = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST, - DImode, 3, - op0, GET_MODE (op0), - op1, GET_MODE (op1), - address, GET_MODE (address)); - - *quot_p = quotient; - *rem_p = remainder; + gcc_unreachable (); } diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index ad32744..933db67 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -3808,9 +3808,13 @@ target_supports_divmod_p (optab divmod_optab, optab div_optab, machine_mode mode if (optab_handler (divmod_optab, mode) != CODE_FOR_nothing) return true; - /* Check if libfunc for divmod is available. */ - rtx libfunc = optab_libfunc (divmod_optab, mode); - if (libfunc != NULL_RTX) + /* Check if target-specific divmod libfunc is available. + If target overrides expand_divmod_libfunc, then it *has to* + set_optab_libfunc (divmod_optab, mode) to target-specific divmod + libfunc or NULL for unsupported modes. */ + + if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc + && optab_libfunc (divmod_optab, mode)) { /* If optab_handler exists for div_optab, perhaps in a wider mode, we don't want to use the libfunc even if it exists for given mode. */ @@ -3820,12 +3824,7 @@ target_supports_divmod_p (optab divmod_optab, optab div_optab, machine_mode mode if (optab_handler (div_optab, div_mode) != CODE_FOR_nothing) return false; - /* FIXME: This is a hack to workaround an issue with optab_libfunc(). - optab_libfunc (sdivmod_optab, DImode) returns libfunc "__divmoddi4", - although __divmoddi4() does not exist in libgcc. For now, enable the - transform only if libfunc is guaranteed to be __udivmoddi4. */ - return (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc - || !strcmp (XSTR (libfunc, 0), "__udivmoddi4")); + return true; } return false;