From patchwork Tue Sep 2 15:34:06 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 385197 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 7BFEC1401AA for ; Wed, 3 Sep 2014 01:34:24 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=dGyoENRokvWix4mXy e+lOMOurC4oZT2Qg4djnzKYuolBUPgPZ7fOPSWtwdb2TTjp2lTUHnp0bwFjIv9uf Uf8lVNg7Brs1Azd9HHzcNz94FyVIxfxIAwK+C/asoJJDrw5gU8M2SNdQOT1dAX/a fIgvql+wVwoT0+qh9m4fi0OHFg= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=i0zkcYGb5eLj7mkuJQ2eRuG yy3E=; b=PydVXJzojcQHgPA0GVxkc4tR0cJf/KoulCqwXy3ySXjI9yQjgHkJpnr /UW7fgkxYIggl1ROXoQBmMX1PH+922B2P0p/2fteLbCHs58HD7Jkx0vB2v3WAWgY tdp62d5f9lNmcn2MubIvkJSpxa3S1TchRSYLmX3KZ7JrD0Zln3w0= Received: (qmail 2099 invoked by alias); 2 Sep 2014 15:34:13 -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 2001 invoked by uid 89); 2 Sep 2014 15:34:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: service87.mimecast.com Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 02 Sep 2014 15:34:10 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Tue, 02 Sep 2014 16:34:07 +0100 Received: from [10.1.208.24] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 2 Sep 2014 16:34:07 +0100 Message-ID: <5405E36E.3040100@arm.com> Date: Tue, 02 Sep 2014 16:34:06 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Richard Henderson , GCC Patches CC: Richard Earnshaw , Marcus Shawcroft Subject: Re: [PATCH][AArch64] Use CC_Z and CC_NZ with csinc and similar instructions References: <53F1F068.4070708@arm.com> <53F24B7D.1010200@redhat.com> <53F35128.9010801@arm.com> <53F36C67.4080803@redhat.com> <53F376D2.8080505@redhat.com> In-Reply-To: <53F376D2.8080505@redhat.com> X-MC-Unique: 114090216340705901 X-IsSubscribed: yes Hi Richard, Sorry for the delay. On 19/08/14 17:09, Richard Henderson wrote: >> (define_special_predicate "cc_register_zero" >> (match_code "reg") >> { >> return (REGNO (op) == CC_REGNUM >> && (GET_MODE (op) == CCmode >> || GET_MODE (op) == CC_Zmode >> || GET_MODE (op) == CC_NZmode)); >> }) > ... and now that I read the backend more closely, I see "_zero" was a bad name. > > But more importantly, I see no connection between the comparison used and the > CCmode being accepted. And if we fix that, why are you restricting to just Z > and NZ? What's wrong with e.g. CFPmode? I'm not sure why restricted the modes for csinc. > In the i386 backend, we check comparison+mode correspondence like > > (match_operator 4 "ix86_carry_flag_operator" > [(match_operand 3 "flags_reg_operand") (const_int 0)]) > > I think you'll want something similar. In the case of CSINC, we can accept all > conditions, so let's start with the most general: > > (match_operator:GPI 2 "aarch64_comparison_operation" > [(reg CC_REGNUM) (const_int 0)] > > or even > > (match_operand:GPI 2 "aarch64_comparison_operation" "") > > with > > (define_predicate "aarch64_comparison_operation" > (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu," > "unordered,ordered,unlt,unle,unge,ungt") > { > if (XEXP (op, 1) != const0_rtx) > return false; > rtx op0 = XEXP (op, 0); > if (!REG_P (op0) || REGNO (op0) != CC_REGNUM) > return false; > return aarch64_get_condition_code (op) >= 0; > }) > > where aarch64_get_condition_code is > (1) exported > (2) adjusted to return "int" not "unsigned" > (3) adjusted to not abort, but return -1 for invalid combinations. > > and the two existing users of aarch64_get_condition_code are adjusted to > gcc_assert that the return value is valid. Implementing that seems to work fine. Bootstrap and testing were successful. How's this version then? Kyrill 2014-09-02 Kyrylo Tkachov * config/aarch64/predicates.md (aarch64_comparison_operation): New special predicate. * config/aarch64/aarch64.md (*csinc2_insn): Use aarch64_comparison_operation instead of matching an operator. Update operand numbers. (csinc3_insn): Likewise. (*csinv3_insn): Likewise. (*csneg3_insn): Likewise. (ffs2): Update gen_csinc3_insn callsite. * config/aarch64/aarch64.c (aarch64_get_condition_code): Export. Return -1 instead of aborting on invalid condition codes. (aarch64_print_operand): Update aarch64_get_condition_code callsites to assert that the returned condition code is valid. > > r~ > commit a70dc696b967196d6662479a44682e3f423377ac Author: Kyrylo Tkachov Date: Mon Aug 4 16:49:24 2014 +0100 [AArch64] Generalise condition code usage for csinc pattterns diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index b5335bf..d3be619 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -174,6 +174,7 @@ struct tune_params }; HOST_WIDE_INT aarch64_initial_elimination_offset (unsigned, unsigned); +int aarch64_get_condition_code (rtx); bool aarch64_bitmask_imm (HOST_WIDE_INT val, enum machine_mode); bool aarch64_cannot_change_mode_class (enum machine_mode, enum machine_mode, diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index ba45d00..809d562 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -3589,7 +3589,7 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y) return CCmode; } -static unsigned +int aarch64_get_condition_code (rtx x) { enum machine_mode mode = GET_MODE (XEXP (x, 0)); @@ -3616,7 +3616,7 @@ aarch64_get_condition_code (rtx x) case UNLE: return AARCH64_LE; case UNGT: return AARCH64_HI; case UNGE: return AARCH64_PL; - default: gcc_unreachable (); + default: return -1; } break; @@ -3633,7 +3633,7 @@ aarch64_get_condition_code (rtx x) case GTU: return AARCH64_HI; case LEU: return AARCH64_LS; case LTU: return AARCH64_CC; - default: gcc_unreachable (); + default: return -1; } break; @@ -3652,7 +3652,7 @@ aarch64_get_condition_code (rtx x) case GTU: return AARCH64_CC; case LEU: return AARCH64_CS; case LTU: return AARCH64_HI; - default: gcc_unreachable (); + default: return -1; } break; @@ -3663,7 +3663,7 @@ aarch64_get_condition_code (rtx x) case EQ: return AARCH64_EQ; case GE: return AARCH64_PL; case LT: return AARCH64_MI; - default: gcc_unreachable (); + default: return -1; } break; @@ -3672,12 +3672,12 @@ aarch64_get_condition_code (rtx x) { case NE: return AARCH64_NE; case EQ: return AARCH64_EQ; - default: gcc_unreachable (); + default: return -1; } break; default: - gcc_unreachable (); + return -1; break; } } @@ -3795,39 +3795,48 @@ aarch64_print_operand (FILE *f, rtx x, char code) break; case 'm': - /* Print a condition (eq, ne, etc). */ - - /* CONST_TRUE_RTX means always -- that's the default. */ - if (x == const_true_rtx) - return; + { + int cond_code; + /* Print a condition (eq, ne, etc). */ - if (!COMPARISON_P (x)) - { - output_operand_lossage ("invalid operand for '%%%c'", code); + /* CONST_TRUE_RTX means always -- that's the default. */ + if (x == const_true_rtx) return; - } - fputs (aarch64_condition_codes[aarch64_get_condition_code (x)], f); + if (!COMPARISON_P (x)) + { + output_operand_lossage ("invalid operand for '%%%c'", code); + return; + } + + cond_code = aarch64_get_condition_code (x); + gcc_assert (cond_code >= 0); + fputs (aarch64_condition_codes[cond_code], f); + } break; case 'M': - /* Print the inverse of a condition (eq <-> ne, etc). */ - - /* CONST_TRUE_RTX means never -- that's the default. */ - if (x == const_true_rtx) - { - fputs ("nv", f); - return; - } + { + int cond_code; + /* Print the inverse of a condition (eq <-> ne, etc). */ - if (!COMPARISON_P (x)) - { - output_operand_lossage ("invalid operand for '%%%c'", code); - return; - } + /* CONST_TRUE_RTX means never -- that's the default. */ + if (x == const_true_rtx) + { + fputs ("nv", f); + return; + } - fputs (aarch64_condition_codes[AARCH64_INVERSE_CONDITION_CODE - (aarch64_get_condition_code (x))], f); + if (!COMPARISON_P (x)) + { + output_operand_lossage ("invalid operand for '%%%c'", code); + return; + } + cond_code = aarch64_get_condition_code (x); + gcc_assert (cond_code >= 0); + fputs (aarch64_condition_codes[AARCH64_INVERSE_CONDITION_CODE + (cond_code)], f); + } break; case 'b': diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 3c51fd3..2e1394d 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -2598,9 +2598,8 @@ (define_insn "aarch64_" (define_insn "*csinc2_insn" [(set (match_operand:GPI 0 "register_operand" "=r") - (plus:GPI (match_operator:GPI 2 "aarch64_comparison_operator" - [(match_operand:CC 3 "cc_register" "") (const_int 0)]) - (match_operand:GPI 1 "register_operand" "r")))] + (plus:GPI (match_operand 2 "aarch64_comparison_operation" "") + (match_operand:GPI 1 "register_operand" "r")))] "" "csinc\\t%0, %1, %1, %M2" [(set_attr "type" "csel")] @@ -2609,37 +2608,34 @@ (define_insn "*csinc2_insn" (define_insn "csinc3_insn" [(set (match_operand:GPI 0 "register_operand" "=r") (if_then_else:GPI - (match_operator:GPI 1 "aarch64_comparison_operator" - [(match_operand:CC 2 "cc_register" "") (const_int 0)]) - (plus:GPI (match_operand:GPI 3 "register_operand" "r") + (match_operand 1 "aarch64_comparison_operation" "") + (plus:GPI (match_operand:GPI 2 "register_operand" "r") (const_int 1)) - (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))] + (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))] "" - "csinc\\t%0, %4, %3, %M1" + "csinc\\t%0, %3, %2, %M1" [(set_attr "type" "csel")] ) (define_insn "*csinv3_insn" [(set (match_operand:GPI 0 "register_operand" "=r") (if_then_else:GPI - (match_operator:GPI 1 "aarch64_comparison_operator" - [(match_operand:CC 2 "cc_register" "") (const_int 0)]) - (not:GPI (match_operand:GPI 3 "register_operand" "r")) - (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))] + (match_operand 1 "aarch64_comparison_operation" "") + (not:GPI (match_operand:GPI 2 "register_operand" "r")) + (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))] "" - "csinv\\t%0, %4, %3, %M1" + "csinv\\t%0, %3, %2, %M1" [(set_attr "type" "csel")] ) (define_insn "*csneg3_insn" [(set (match_operand:GPI 0 "register_operand" "=r") (if_then_else:GPI - (match_operator:GPI 1 "aarch64_comparison_operator" - [(match_operand:CC 2 "cc_register" "") (const_int 0)]) - (neg:GPI (match_operand:GPI 3 "register_operand" "r")) - (match_operand:GPI 4 "aarch64_reg_or_zero" "rZ")))] + (match_operand 1 "aarch64_comparison_operation" "") + (neg:GPI (match_operand:GPI 2 "register_operand" "r")) + (match_operand:GPI 3 "aarch64_reg_or_zero" "rZ")))] "" - "csneg\\t%0, %4, %3, %M1" + "csneg\\t%0, %3, %2, %M1" [(set_attr "type" "csel")] ) @@ -2896,7 +2892,7 @@ (define_expand "ffs2" emit_insn (gen_rbit2 (operands[0], operands[1])); emit_insn (gen_clz2 (operands[0], operands[0])); - emit_insn (gen_csinc3_insn (operands[0], x, ccreg, operands[0], const0_rtx)); + emit_insn (gen_csinc3_insn (operands[0], x, operands[0], const0_rtx)); DONE; } ) diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md index 3dd83ca..c1510ca 100644 --- a/gcc/config/aarch64/predicates.md +++ b/gcc/config/aarch64/predicates.md @@ -202,6 +202,18 @@ (define_predicate "aarch64_reg_or_imm" (define_special_predicate "aarch64_comparison_operator" (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt")) +(define_special_predicate "aarch64_comparison_operation" + (match_code "eq,ne,le,lt,ge,gt,geu,gtu,leu,ltu,unordered,ordered,unlt,unle,unge,ungt") +{ + if (XEXP (op, 1) != const0_rtx) + return false; + rtx op0 = XEXP (op, 0); + if (!REG_P (op0) || REGNO (op0) != CC_REGNUM) + return false; + return aarch64_get_condition_code (op) >= 0; +}) + + ;; True if the operand is memory reference suitable for a load/store exclusive. (define_predicate "aarch64_sync_memory_operand" (and (match_operand 0 "memory_operand")