From patchwork Thu Mar 3 07:23:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Collison X-Patchwork-Id: 63437 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp2792440lbc; Wed, 2 Mar 2016 23:24:22 -0800 (PST) X-Received: by 10.98.34.205 with SMTP id p74mr1607943pfj.93.1456989862057; Wed, 02 Mar 2016 23:24:22 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id ly8si837644pab.89.2016.03.02.23.24.21 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Mar 2016 23:24:22 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-422636-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; spf=pass (google.com: domain of gcc-patches-return-422636-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-422636-patch=linaro.org@gcc.gnu.org; dkim=pass header.i=@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=qSf77i0l3OST2eNUy LWWROdnY3QIZeRw05Z832mXO6/I6qevFkxxh1PSXUwfFi2QVufCwul/DkXOmdFxS pxgGaCZSUxeqrntvOH1inAwnBr7viScvcf2QnvBw7IHII0y6Yb9wCGziK0ZtbZ42 R+PaOLGDRYlKrBucwbLiKyMW8c= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=euzR0enC1uhm2qhBNuOoYGM elIo=; b=HTUAdTSoK+D7B2QK3hE8sc+PSYowlfjQ6w/Rgxmjwcdmum87hRbfdnR Cq7nY9R9k6xRIocD8tz+1ApTqtqlanspUeICs49oyUvoecsUrPbMmBPEvdSGAC/p VKjqvHCnJLPdj5EPQ8EKSr/zpijFoGLLEwN1lNZXCrXPe5wGBd+U= Received: (qmail 73395 invoked by alias); 3 Mar 2016 07:24:07 -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 73370 invoked by uid 89); 3 Mar 2016 07:24:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=retained, ri, carry, late X-HELO: mail-pf0-f175.google.com Received: from mail-pf0-f175.google.com (HELO mail-pf0-f175.google.com) (209.85.192.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 03 Mar 2016 07:24:04 +0000 Received: by mail-pf0-f175.google.com with SMTP id 63so9874827pfe.3 for ; Wed, 02 Mar 2016 23:24:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to; bh=C9OgYAr6xgiFUDaF6Gb7JT111HFDW6yS9P34JF0saYo=; b=aUaxdh+qyECMzubLd5hccH1s2Ns251SyGFIp3/W472gtrFFG+AO0aGXbndFoSFx+MS mL1Kur/nScptFU7GJBrZeXWgXT3XZdKLs33BNfqfKDT8gWxTjSi8+/d+sH5J2knuWO7H uGPNNz6UUt5m17xpYF9HcBr6wVpq+D6sl4Tf0Ui7FK7a1Z+rKL2R65Tryh30CrwDWzhW hD6fzBwzVMFNVKgHpQYgb5GdyaPOA4gQqGydchD/QOFx/YYFdEZPP0gtkGrC4K42w45/ GnlbeZN/Np46WS2pd9UXIxp8t5UBoiaAU8KtajFuTZcH5/59SPdduBh5W21IgfK7UVDI ji9A== X-Gm-Message-State: AD7BkJI7/ufyKa5Y5u2RRvE7h7dVpfLbFP1Q8+uhlytyvTNL50KRYr1lqrIUWVDv6lkyJrlF X-Received: by 10.98.34.28 with SMTP id i28mr1611991pfi.160.1456989842296; Wed, 02 Mar 2016 23:24:02 -0800 (PST) Received: from [192.168.1.9] (ip70-176-202-128.ph.ph.cox.net. [70.176.202.128]) by smtp.googlemail.com with ESMTPSA id x88sm57893007pfi.66.2016.03.02.23.24.00 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 02 Mar 2016 23:24:01 -0800 (PST) Subject: Re: [PATCH][ARM] PR target/70008 To: "Richard Earnshaw (lists)" , Kyrill Tkachov , GCC Patches , Ramana Radhakrishnan References: <56D3CD4F.6060301@linaro.org> <56D4263A.5040403@foss.arm.com> <56D429CA.9000202@linaro.org> <56D463E7.1040105@arm.com> From: Michael Collison Message-ID: <56D7E68F.4020500@linaro.org> Date: Thu, 3 Mar 2016 00:23:59 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <56D463E7.1040105@arm.com> I have attached a new patch which hopefully address Richard's concerns. I modified the predicate on operand 1 to to "arm_rhs_operand" to be consistent with the constraints. I retained the split into two patterns; one for arm and another for thumb2. I thought this was cleaner. Okay for trunk? 2016-02-28 Michael Collison PR target/70008 * config/arm/arm.md (*subsi3_carryin): Change predicate to arm_rhs_operand to be consistent with constraints. Only allow pattern for TARGET_ARM. * config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern. On 02/29/2016 08:29 AM, Richard Earnshaw (lists) wrote: > On 29/02/16 11:21, Michael Collison wrote: >> >> On 2/29/2016 4:06 AM, Kyrill Tkachov wrote: >>> Hi Michael, >>> >>> On 29/02/16 04:47, Michael Collison wrote: >>>> This patches address PR 70008, where a reverse subtract with carry >>>> instruction can be generated in thumb2 mode. It was tested with no >>>> regressions in arm and thumb modes on the following targets: >>>> >>>> arm-none-linux-gnueabi >>>> arm-none-linux-gnuabihf >>>> armeb-none-linux-gnuabihf >>>> arm-none-eabi >>>> >>>> Okay for trunk? >>>> >>>> 2016-02-28 Michael Collison >>>> >>>> PR target/70008 >>>> * config/arm/arm.md (*subsi3_carryin): Only match pattern if >>>> TARGET_ARM due to 'rsc' instruction alternative. >>>> * config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern. >>>> >>>> >>> The *subsi3_carrying pattern has the arch attribute: >>> (set_attr "arch" "*,a") >>> >>> That means that the second alternative that generates the RSC >>> instruction is only enabled >>> for ARM mode. Do you have a testcase where this doesn't happen and >>> this pattern generates >>> the second alternative for Thumb2? >> No I don't have a test case; i noticed the pattern when working on the >> overflow project. I did not realize >> that an attribute could affect the matching of an alternative. I will >> close the bug. >> >>> >>> Thanks, >>> Kyrill > This is all true, but there is a potential performance issue with this > pattern though, that could lead to sub-optimal code. > > The predicate accepts reg-or-int, but in ARM state only simple > 'const-ok-for-arm' immediates are permitted by the predicates, and in > thumb code, no immediates are permitted at all. This could potentially > result in sub-optimal code due to late splitting of the pattern. It > would be better if the predicate understood these limitations and > restricted immediates accordingly. > > R. > -- Michael Collison Linaro Toolchain Working Group michael.collison@linaro.org diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index e67239d..e6bcd7f 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -867,15 +867,14 @@ (define_insn "*subsi3_carryin" [(set (match_operand:SI 0 "s_register_operand" "=r,r") - (minus:SI (minus:SI (match_operand:SI 1 "reg_or_int_operand" "r,I") + (minus:SI (minus:SI (match_operand:SI 1 "arm_rhs_operand" "r,I") (match_operand:SI 2 "s_register_operand" "r,r")) (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))] - "TARGET_32BIT" + "TARGET_ARM" "@ sbc%?\\t%0, %1, %2 rsc%?\\t%0, %2, %1" [(set_attr "conds" "use") - (set_attr "arch" "*,a") (set_attr "predicable" "yes") (set_attr "predicable_short_it" "no") (set_attr "type" "adc_reg,adc_imm")] diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md index 9925365..79305c5 100644 --- a/gcc/config/arm/thumb2.md +++ b/gcc/config/arm/thumb2.md @@ -848,6 +848,20 @@ (set_attr "type" "multiple")] ) +(define_insn "*thumb2_subsi3_carryin" + [(set (match_operand:SI 0 "s_register_operand" "=r") + (minus:SI (minus:SI (match_operand:SI 1 "s_register_operand" "r") + (match_operand:SI 2 "s_register_operand" "r")) + (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))] + "TARGET_THUMB2" + "@ + sbc%?\\t%0, %1, %2" + [(set_attr "conds" "use") + (set_attr "predicable" "yes") + (set_attr "predicable_short_it" "no") + (set_attr "type" "adc_reg")] +) + (define_insn "*thumb2_cond_sub" [(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts") (minus:SI (match_operand:SI 1 "s_register_operand" "0,?Ts") -- 1.9.1