From patchwork Mon May 23 12:05:18 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 625156 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rCy155K3gz9t3Z for ; Mon, 23 May 2016 22:05:45 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=KSeF7vVU; dkim-atps=neutral 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:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=a+I+u6YSYVSpo0/Ocq LIpl/Y/Nsv/lTMoAefsgbkFJ1HqF4tRCMmqpNb70FlJfpdEQ5vAH7kpF5njb/t+c vFr6Jb3p13Es3vxG3I+P/QPx6MkCVa2JyA2rapX7+HOTciae9g6tGNitvsW3F0ij 9jx7OAG5T4Gq7IqAxHdOqIAE8= 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:date:message-id:subject :from:to:cc:content-type; s=default; bh=R6Rv+Haigj5tnSO28Un/1RHQ 5Yk=; b=KSeF7vVUxz/gISJud009NWzaElP1OPj6WtjmnH1OfXxsusMk8EAsNkgi QpWsHjUA+WLIGzRUmDTlcgkce6b07UdumgP1Utuq+IHzUSi/SVYdTIsmWHjMHERL edZCJJ2I9KmZ1wEDuBpG7RVkmjBTErU/bi0ULJz6s6nvWQ9jFAk= Received: (qmail 65375 invoked by alias); 23 May 2016 12:05:33 -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 65362 invoked by uid 89); 23 May 2016 12:05:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=transfer X-HELO: mail-wm0-f48.google.com Received: from mail-wm0-f48.google.com (HELO mail-wm0-f48.google.com) (74.125.82.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 23 May 2016 12:05:22 +0000 Received: by mail-wm0-f48.google.com with SMTP id a136so18528334wme.0 for ; Mon, 23 May 2016 05:05:21 -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:date :message-id:subject:from:to:cc; bh=yuwNIC8oCGBKzEcN1zv5eiVzKXbxrxebXOpFth3ZwQw=; b=ZcNagJzDvaj9cZz018e6kqBtdb7n54GvYJfHA+NewS+1jl6V8XCiiHdr3ReDE1kG34 Kq+Wved0P3MH1DL5AFo1ATskzrisk4m2kmCDneVn8pDicxzE18v5pADVkGmEm8vVsWsJ jL8UixaVTCj66NRdlYWcCij3ehCIlNS74xdksZM4HtPWHO6qa+l6ua5xBZkpSKtJWNvn EnyHcM1wSK+iIEkbGkP6BgML0RdrgO7tTbw9advOq0XOobKeryT6/FnEaivCfuAc5VzZ aqYWoXQdx+ehnyyPo7MfFmMP4JxA1nVUleoFA30/Mt60s9zAQ/EOSy+m6KIJZInwAs33 ZXDg== X-Gm-Message-State: AOPr4FUxxYVu4foPCNzA5ldKxb+JyDV58ccVfHTxhIfVyOaBVOblGNJx4pAM8nN9pa7fMMHIqlanYbcgmNkRsA== MIME-Version: 1.0 X-Received: by 10.194.24.38 with SMTP id r6mr15610257wjf.88.1464005118941; Mon, 23 May 2016 05:05:18 -0700 (PDT) Received: by 10.194.87.34 with HTTP; Mon, 23 May 2016 05:05:18 -0700 (PDT) In-Reply-To: References: Date: Mon, 23 May 2016 14:05:18 +0200 Message-ID: Subject: Re: RFC [1/2] divmod transform From: Richard Biener To: Prathamesh Kulkarni Cc: gcc Patches , Richard Biener , Ramana Radhakrishnan , Kugan Vivekanandarajah , Jim Wilson X-IsSubscribed: yes On Mon, May 23, 2016 at 10:58 AM, Prathamesh Kulkarni wrote: > Hi, > I have updated my patch for divmod (attached), which was originally > based on Kugan's patch. > The patch transforms stmts with code TRUNC_DIV_EXPR and TRUNC_MOD_EXPR > having same operands to divmod representation, so we can cse computation of mod. > > t1 = a TRUNC_DIV_EXPR b; > t2 = a TRUNC_MOD_EXPR b > is transformed to: > complex_tmp = DIVMOD (a, b); > t1 = REALPART_EXPR (complex_tmp); > t2 = IMAGPART_EXPR (complex_tmp); > > * New hook divmod_expand_libfunc > The rationale for introducing the hook is that different targets have > incompatible calling conventions for divmod libfunc. > Currently three ports define divmod libfunc: c6x, spu and arm. > c6x and spu follow the convention of libgcc2.c:__udivmoddi4: > return quotient and store remainder in argument passed as pointer, > while the arm version takes two arguments and returns both > quotient and remainder having mode double the size of the operand mode. > The port should hence override the hook expand_divmod_libfunc > to generate call to target-specific divmod. > Ports should define this hook if: > a) The port does not have divmod or div insn for the given mode. > b) The port defines divmod libfunc for the given mode. > The default hook default_expand_divmod_libfunc() generates call > to libgcc2.c:__udivmoddi4 provided the operands are unsigned and > are of DImode. > > Patch passes bootstrap+test on x86_64-unknown-linux-gnu and > cross-tested on arm*-*-*. > Bootstrap+test in progress on arm-linux-gnueabihf. > Does this patch look OK ? supports a specific operation (for example SImode divmod would use DImode divmod by means of widening operands - for the unsigned case of course). + /* Disable the transform if either is a constant, since division-by-constant + may have specialized expansion. */ + if (TREE_CONSTANT (op1) || TREE_CONSTANT (op2)) + return false; please use CONSTANT_CLASS_P (op1) || CONSTANT_CLASS_P (op2) + if (TYPE_OVERFLOW_TRAPS (type)) + return false; why's that? Generally please first test cheap things (trapping, constant-ness) before checking expensive stuff (target_supports_divmod_p). +static bool +convert_to_divmod (gassign *stmt) +{ + if (!divmod_candidate_p (stmt)) + return false; + + tree op1 = gimple_assign_rhs1 (stmt); + tree op2 = gimple_assign_rhs2 (stmt); + + vec stmts = vNULL; use an auto_vec - you currently leak it in at least one place. + if (maybe_clean_or_replace_eh_stmt (use_stmt, use_stmt)) + cfg_changed = true; note that this suggests you should check whether any of the stmts may throw internally as you don't update / transfer EH info correctly. So for 'stmt' and all 'use_stmt' check stmt_can_throw_internal and if so do not add it to the list of stmts to modify. Btw, I think you should not add 'stmt' immediately but when iterating over all uses also gather uses in TRUNC_MOD_EXPR. Otherwise looks ok. Thanks, Richard. > Thanks, > Prathamesh diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 6b4601b..e4a021a 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1965,4 +1965,31 @@ default_optab_supported_p (int, machine_mode, machine_mode, optimization_type) return true; } +void +default_expand_divmod_libfunc (bool unsignedp, machine_mode mode, + rtx op0, rtx op1, + rtx *quot_p, rtx *rem_p) functions need a comment. ISTR it was suggested that ARM change to libgcc2.c__udivmoddi4 style? In that case we could avoid the target hook. + /* If target overrides expand_divmod_libfunc hook + then perform divmod by generating call to the target-specifc divmod libfunc. */ + if (targetm.expand_divmod_libfunc != default_expand_divmod_libfunc) + return true; + + /* Fall back to using libgcc2.c:__udivmoddi4. */ + return (mode == DImode && unsignedp); I don't understand this - we know optab_libfunc returns non-NULL for 'mode' but still restrict this to DImode && unsigned? Also if targetm.expand_divmod_libfunc is not the default we expect the target to handle all modes? That said - I expected the above piece to be simply a 'return true;' ;) Usually we use some can_expand_XXX helper in optabs.c to query if the target