From patchwork Wed Sep 2 13:00:23 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 52968 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f197.google.com (mail-wi0-f197.google.com [209.85.212.197]) by patches.linaro.org (Postfix) with ESMTPS id E2D5222DC0 for ; Wed, 2 Sep 2015 13:00:47 +0000 (UTC) Received: by wicgc1 with SMTP id gc1sf6068531wic.3 for ; Wed, 02 Sep 2015 06:00: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:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:message-id:date:from:user-agent:mime-version:to:cc :subject:references:in-reply-to:content-type:x-original-sender :x-original-authentication-results; bh=+X7OGB95nNK/fKDqf1+bOiLIVVHbKqA2tybUak4PhfM=; b=iMjYTRcrfIIsqNn+t/fxBuuytUs6gRDCYzhdh9u7FM4PNJBJA4WT99emYh+8nWsX/o lYHRhvVyBdm3syYli/nHnmtwpVhly+4ERohDIDJESxs48mYOBWWXW4TEUmqpXSfAwKp6 GPc3wnFUu4hIAMKsh7bRCTSibCJp1qwfKdwM0R+nIWz43uTW6+YApafCa4gJTIdS/v/Y 1pjceKSpVeG4ui1zhuSrr258qa1HklY/+s4QVlt6zwW/e8nLP4PXi0gyLtYTdPrm49GP Dg1ZEHAV8s4UeKS+RZgudDsS8C+VNB6/aN4nbTznb8hbubIYk9RDzFjOp7tPyzRRjeLf N4RA== X-Gm-Message-State: ALoCoQnyCSLnbIdN8yNDpkOem55S8PkDcZU8k9LHge5Q8EQV6c3QstmqUWZKO4NSZEOLk6ardBS6 X-Received: by 10.152.5.164 with SMTP id t4mr9170961lat.0.1441198847078; Wed, 02 Sep 2015 06:00:47 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.9.7 with SMTP id v7ls76876laa.107.gmail; Wed, 02 Sep 2015 06:00:46 -0700 (PDT) X-Received: by 10.152.6.162 with SMTP id c2mr10986693laa.1.1441198846940; Wed, 02 Sep 2015 06:00:46 -0700 (PDT) Received: from mail-la0-x232.google.com (mail-la0-x232.google.com. [2a00:1450:4010:c03::232]) by mx.google.com with ESMTPS id b9si19572843lbc.161.2015.09.02.06.00.46 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Sep 2015 06:00:46 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::232 as permitted sender) client-ip=2a00:1450:4010:c03::232; Received: by lagj9 with SMTP id j9so6511591lag.2 for ; Wed, 02 Sep 2015 06:00:46 -0700 (PDT) X-Received: by 10.152.18.194 with SMTP id y2mr6541270lad.88.1441198846768; Wed, 02 Sep 2015 06:00:46 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.164.42 with SMTP id yn10csp556064lbb; Wed, 2 Sep 2015 06:00:45 -0700 (PDT) X-Received: by 10.66.241.2 with SMTP id we2mr54903316pac.99.1441198844901; Wed, 02 Sep 2015 06:00:44 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id uw4si28571817pac.106.2015.09.02.06.00.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Sep 2015 06:00:44 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-406522-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 65781 invoked by alias); 2 Sep 2015 13:00:32 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 65761 invoked by uid 89); 2 Sep 2015 13:00:31 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 02 Sep 2015 13:00:29 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-20-YM6GWRDvS0-AqdXNNQP9LQ-1; Wed, 02 Sep 2015 14:00:24 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 2 Sep 2015 14:00:24 +0100 Message-ID: <55E6F2E7.1080904@arm.com> Date: Wed, 02 Sep 2015 14:00:23 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: James Greenhalgh CC: GCC Patches , Marcus Shawcroft , Richard Earnshaw Subject: Re: [PATCH][AArch64][1/3] Expand signed mod by power of 2 using CSNEG References: <55B219A5.8060307@arm.com> <20150803130104.GA13835@arm.com> <55CC8F62.4080604@arm.com> <20150901092501.GA28861@arm.com> <55E580AC.2060200@arm.com> In-Reply-To: <55E580AC.2060200@arm.com> X-MC-Unique: YM6GWRDvS0-AqdXNNQP9LQ-1 X-IsSubscribed: yes X-Original-Sender: kyrylo.tkachov@arm.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c03::232 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 On 01/09/15 11:40, Kyrill Tkachov wrote: > Hi James, > > On 01/09/15 10:25, James Greenhalgh wrote: >> On Thu, Aug 13, 2015 at 01:36:50PM +0100, Kyrill Tkachov wrote: >> >> Some comments below. > Thanks, I'll incorporate them, with one clarification inline. And here's the updated patch. Thanks, Kyrill 2015-09-02 Kyrylo Tkachov * config/aarch64/aarch64.md (mod3): New define_expand. (*neg2_compare0): Rename to... (neg2_compare0): ... This. * config/aarch64/aarch64.c (aarch64_rtx_costs, MOD case): Move check for speed inside the if-then-elses. Reflect CSNEG sequence in MOD by power of 2 case. > >>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c >>> index 1394ed7..c8bd8d2 100644 >>> --- a/gcc/config/aarch64/aarch64.c >>> +++ b/gcc/config/aarch64/aarch64.c >>> @@ -6652,6 +6652,25 @@ cost_plus: >>> return true; >>> >>> case MOD: >>> + /* We can expand signed mod by power of 2 using a >>> + NEGS, two parallel ANDs and a CSNEG. Assume here >>> + that CSNEG is COSTS_N_INSNS (1). This case should >> Why do we want to hardcode this assumption rather than parameterise? Even >> if you model this as the cost of an unconditional NEG I think that is >> better than hardcoding zero cost. >> >> >>> + only ever be reached through the set_smod_pow2_cheap check >>> + in expmed.c. */ >>> + if (CONST_INT_P (XEXP (x, 1)) >>> + && exact_log2 (INTVAL (XEXP (x, 1))) > 0 >>> + && (mode == SImode || mode == DImode)) >>> + { >>> + *cost += COSTS_N_INSNS (3); >> Can you add am comment to make it clear why this is not off-by-one? By >> quick inspection it looks like you have made a typo trying to set the >> cost to be 3 instructions rather than 4 - a reader needs the extra >> knowledge that we already have a COSTS_N_INSNS(1) as a baseline. >> >> This would be clearer as: >> >> /* We will expand to four instructions, reset the baseline. */ >> *cost = COSTS_N_INSNS (4); >> >>> + >>> + if (speed) >>> + *cost += 2 * extra_cost->alu.logical >>> + + extra_cost->alu.arith; >>> + >>> + return true; >>> + } >>> + >>> + /* Fall-through. */ >>> case UMOD: >>> if (speed) >>> { >>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md >>> index b7b04c4..a515573 100644 >>> --- a/gcc/config/aarch64/aarch64.md >>> +++ b/gcc/config/aarch64/aarch64.md >>> @@ -302,6 +302,62 @@ (define_expand "cmp" >>> } >>> ) >>> >>> +;; AArch64-specific expansion of signed mod by power of 2 using CSNEG. >> Seems a strange comment given that we are in aarch64.md :-). >> >>> +;; For x0 % n where n is a power of 2 produce: >>> +;; negs x1, x0 >>> +;; and x0, x0, #(n - 1) >>> +;; and x1, x1, #(n - 1) >>> +;; csneg x0, x0, x1, mi >>> + >>> +(define_expand "mod3" >>> + [(match_operand:GPI 0 "register_operand" "") >>> + (match_operand:GPI 1 "register_operand" "") >>> + (match_operand:GPI 2 "const_int_operand" "")] >>> + "" >>> + { >>> + HOST_WIDE_INT val = INTVAL (operands[2]); >>> + >>> + if (val <= 0 >>> + || exact_log2 (INTVAL (operands[2])) <= 0 >>> + || !aarch64_bitmask_imm (INTVAL (operands[2]) - 1, mode)) >>> + FAIL; >>> + >>> + rtx mask = GEN_INT (val - 1); >>> + >>> + /* In the special case of x0 % 2 we can do the even shorter: >>> + cmp x0, xzr >>> + and x0, x0, 1 >>> + csneg x0, x0, x0, ge. */ >>> + if (val == 2) >>> + { >>> + rtx masked = gen_reg_rtx (mode); >>> + rtx ccreg = aarch64_gen_compare_reg (LT, operands[1], const0_rtx); >> Non-obvious why this is correct given the comment above saying we want GE. > We want to negate if the comparison earlier yielded "less than zero". > Unfortunately, the CSNEG form of that is written as > > csneg x0, x0, x0, ge > > which looks counter-intuitive at first glance. > With my other patch posted at https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00020.html > the generated assembly would be: > cneg x0, x0, lt > which more closely matches the RTL generation in this hunk. > If you think that patch should go in, I'll rewrite the CSNEG form in this comment to CNEG. > > Kyrill > > > >>> + emit_insn (gen_and3 (masked, operands[1], mask)); >>> + rtx x = gen_rtx_LT (VOIDmode, ccreg, const0_rtx); >>> + emit_insn (gen_csneg3_insn (operands[0], x, masked, masked)); >>> + DONE; >>> + } >>> + >>> + rtx neg_op = gen_reg_rtx (mode); >>> + rtx_insn *insn = emit_insn (gen_neg2_compare0 (neg_op, operands[1])); >>> + >>> + /* Extract the condition register and mode. */ >>> + rtx cmp = XVECEXP (PATTERN (insn), 0, 0); >>> + rtx cc_reg = SET_DEST (cmp); >>> + rtx cond = gen_rtx_GE (VOIDmode, cc_reg, const0_rtx); >>> + >>> + rtx masked_pos = gen_reg_rtx (mode); >>> + emit_insn (gen_and3 (masked_pos, operands[1], mask)); >>> + >>> + rtx masked_neg = gen_reg_rtx (mode); >>> + emit_insn (gen_and3 (masked_neg, neg_op, mask)); >>> + >>> + emit_insn (gen_csneg3_insn (operands[0], cond, >>> + masked_neg, masked_pos)); >>> + DONE; >>> + } >>> +) >>> + >> Thanks, >> James >> commit 534676af75436d7e11865403bdd231fadc7c19aa Author: Kyrylo Tkachov Date: Wed Jul 15 17:01:13 2015 +0100 [AArch64][1/3] Expand signed mod by power of 2 using CSNEG diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index ce6e116..c97cd95 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -6763,6 +6763,25 @@ cost_plus: return true; case MOD: + /* We can expand signed mod by power of 2 using a NEGS, two parallel + ANDs and a CSNEG. Assume here that CSNEG is the same as the cost of + an unconditional negate. This case should only ever be reached through + the set_smod_pow2_cheap check in expmed.c. */ + if (CONST_INT_P (XEXP (x, 1)) + && exact_log2 (INTVAL (XEXP (x, 1))) > 0 + && (mode == SImode || mode == DImode)) + { + /* We expand to 4 instructions. Reset the baseline. */ + *cost = COSTS_N_INSNS (4); + + if (speed) + *cost += 2 * extra_cost->alu.logical + + 2 * extra_cost->alu.arith; + + return true; + } + + /* Fall-through. */ case UMOD: if (speed) { diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 2522982..594bef0 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -313,6 +313,62 @@ (define_expand "cmp" } ) +;; Expansion of signed mod by a power of 2 using CSNEG. +;; For x0 % n where n is a power of 2 produce: +;; negs x1, x0 +;; and x0, x0, #(n - 1) +;; and x1, x1, #(n - 1) +;; csneg x0, x0, x1, mi + +(define_expand "mod3" + [(match_operand:GPI 0 "register_operand" "") + (match_operand:GPI 1 "register_operand" "") + (match_operand:GPI 2 "const_int_operand" "")] + "" + { + HOST_WIDE_INT val = INTVAL (operands[2]); + + if (val <= 0 + || exact_log2 (val) <= 0 + || !aarch64_bitmask_imm (val - 1, mode)) + FAIL; + + rtx mask = GEN_INT (val - 1); + + /* In the special case of x0 % 2 we can do the even shorter: + cmp x0, xzr + and x0, x0, 1 + cneg x0, x0, lt. */ + if (val == 2) + { + rtx masked = gen_reg_rtx (mode); + rtx ccreg = aarch64_gen_compare_reg (LT, operands[1], const0_rtx); + emit_insn (gen_and3 (masked, operands[1], mask)); + rtx x = gen_rtx_LT (VOIDmode, ccreg, const0_rtx); + emit_insn (gen_csneg3_insn (operands[0], x, masked, masked)); + DONE; + } + + rtx neg_op = gen_reg_rtx (mode); + rtx_insn *insn = emit_insn (gen_neg2_compare0 (neg_op, operands[1])); + + /* Extract the condition register and mode. */ + rtx cmp = XVECEXP (PATTERN (insn), 0, 0); + rtx cc_reg = SET_DEST (cmp); + rtx cond = gen_rtx_GE (VOIDmode, cc_reg, const0_rtx); + + rtx masked_pos = gen_reg_rtx (mode); + emit_insn (gen_and3 (masked_pos, operands[1], mask)); + + rtx masked_neg = gen_reg_rtx (mode); + emit_insn (gen_and3 (masked_neg, neg_op, mask)); + + emit_insn (gen_csneg3_insn (operands[0], cond, + masked_neg, masked_pos)); + DONE; + } +) + (define_insn "*condjump" [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator" [(match_operand 1 "cc_register" "") (const_int 0)]) @@ -2457,7 +2513,7 @@ (define_insn "*ngcsi_uxtw" [(set_attr "type" "adc_reg")] ) -(define_insn "*neg2_compare0" +(define_insn "neg2_compare0" [(set (reg:CC_NZ CC_REGNUM) (compare:CC_NZ (neg:GPI (match_operand:GPI 1 "register_operand" "r")) (const_int 0)))