From patchwork Mon Mar 7 03:51:07 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Collison X-Patchwork-Id: 63633 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp1230496lbc; Sun, 6 Mar 2016 19:51:30 -0800 (PST) X-Received: by 10.98.43.194 with SMTP id r185mr30572855pfr.24.1457322690620; Sun, 06 Mar 2016 19:51:30 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id fk1si2152378pad.35.2016.03.06.19.51.30 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 06 Mar 2016 19:51:30 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-422870-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-422870-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-422870-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=wGwqu+iBJugEHDSYg IkR3/G7/ba1dHfGjtmVaiuHCUsoxm5N3JqziwNUH3F4YRjxHc2w+dbl/0ykQcZON EEb8OaZyH/oQyHQAhaPK80vuLG1/9OtfNGBWkgfpr333sNbeDA7lrT8issPuIKE4 MBE09eSNOpxKylqrAge+2na5PA= 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=H8xN4k5oOabXSVQY+wuXqZf 7QN8=; b=q74lVYP/Z+UI9J6kQJ9fuKgMR9p5tocFS5UvptzjIHBZR71MG8WKi+O aSxy8tJxT2mtC5Pc5Flmqo7hRF9RHIrFn62LFXQH4cHZc0f4sjOv+Ir5Ln3lNFrP 98QepLXa2/ZqorpzSiN0yMIRdHZ5iV6Ja16w6zSSKIAZ7pbvcRIA= Received: (qmail 75465 invoked by alias); 7 Mar 2016 03:51:18 -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 75448 invoked by uid 89); 7 Mar 2016 03:51:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=144, 6, ltu, HX-Envelope-From:sk:michael, carry X-HELO: mail-pf0-f180.google.com Received: from mail-pf0-f180.google.com (HELO mail-pf0-f180.google.com) (209.85.192.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 07 Mar 2016 03:51:13 +0000 Received: by mail-pf0-f180.google.com with SMTP id 129so27539871pfw.1 for ; Sun, 06 Mar 2016 19:51:13 -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=dTsT+7tWB3W0bHpz26V/z9ReygC2NLjU01aZ19Mqfmo=; b=Xe+/no12ub50Ffk+QhkhY35FdaR1Bbt8XmjUb4QZsWUfw2hIJBYdUWuYzdeIlUelWl hsmQEn2M1GcBpF0/hkf+6JygnhjSyRrt5bznSS7L7tXhgk7dhMjH/xnSXh2SFYhyxA5F 9bxKpTQdIls3XFMEYBSZRObOQ8JnZme7u3KapNt7umiDPOcbcLCDgjBaiwchT+9hl62H 8yp7rKW9DE436oMjh+l+IgJuq1E5I2Sm8oz6SIj+u+mtHKiBMtinySBKewLj76LdhrAZ zzoGj5L2R+YahkqIqawIft9QrjsVoj+Tm2C+I7EjhLieUIS8o1Z7d7q8fWYYS6dN3R5d FUdA== X-Gm-Message-State: AD7BkJK7gUKguoopOvhp1wxZFBzQEjUmGQygseo+Oav3HZVnqlMTSqJglQAZM7SmR441DwOQ X-Received: by 10.98.68.91 with SMTP id r88mr30473627pfa.12.1457322672051; Sun, 06 Mar 2016 19:51:12 -0800 (PST) Received: from [172.4.115.222] (110-170-137-3.static.asianet.co.th. [110.170.137.3]) by smtp.googlemail.com with ESMTPSA id o185sm10610953pfo.36.2016.03.06.19.51.09 (version=TLSv1/SSLv3 cipher=OTHER); Sun, 06 Mar 2016 19:51:11 -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> <56D7E68F.4020500@linaro.org> <56D8405B.7020409@arm.com> From: Michael Collison Message-ID: <56DCFAAB.5030008@linaro.org> Date: Mon, 7 Mar 2016 10:51:07 +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: <56D8405B.7020409@arm.com> New patch that adds the predicate "reg_or_arm_rhs_operand" as suggested by Richard. Tested on all arm and thumb targets as before. Okay for trunk? 2016-03-07 Michael Collison PR target/70008 * config/arm/predicates.md (reg_or_arm_rhs_operand): New predicate that allows registers or immediates if TARGET_ARM. * config/arm/arm.md (*subsi3_carryin): Change predicate to reg_or_arm_rhs_operand to be consistent with constraints. On 03/03/2016 08:47 PM, Richard Earnshaw (lists) wrote: > On 03/03/16 07:23, Michael Collison wrote: >> 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. >> > I don't think we need two patterns. We could share this if we had a new > predicate that was called something like reg_or_arm_rhs_operand, with a > rule that's something like: > > register_operand (op) || (TARGET_ARM && arm_immediate_operand (op)) > > R. >> 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. >>> >> >> bugzilla-70008-upstream-v2.patch >> >> >> 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") >> -- 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..eb4803a 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -867,7 +867,7 @@ (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 "reg_or_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" diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md index b1cd556..8bf7c1a 100644 --- a/gcc/config/arm/predicates.md +++ b/gcc/config/arm/predicates.md @@ -144,6 +144,11 @@ (and (match_code "const_int") (match_test "INTVAL (op) == 0"))) +(define_predicate "reg_or_arm_rhs_operand" + (ior (match_operand 0 "s_register_operand") + (and (match_test "TARGET_ARM") + (match_operand 0 "arm_immediate_operand")))) + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate "arm_rhs_operand" (ior (match_operand 0 "s_register_operand") -- 1.9.1