From patchwork Tue Apr 2 16:06:14 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 233089 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 8260B2C020A for ; Wed, 3 Apr 2013 03:07:03 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=N/R4GbmLHu0hgFPo gcfTL5kGaLPJhR9kK9kgPj6e73VR1kY38wUluh18SqB70Y28XptsIRYTyqInpam/ GtEnyOrLP0CwER6n7uHtTF6eC0GgIE/beHBUxD/2P1SXqv/ONyue24nyahu9F1t0 6ifIfOyLiv8zkVBdY3iqVw8EShQ= 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:from :to:cc:references:in-reply-to:subject:date:message-id :mime-version:content-type; s=default; bh=8COWOlMxmR6bJF1iDWBbCO qRO3w=; b=QlPFaWGbbzWWmcqzQxrDiTKqjzc+/t3OjfrLLQ9ktautf3oCw8BYmg +Xa6W8PTD7QPHTPl4fFWtoC5D9OG0QiuD7tNx8LgVbHHkhUbdyou/OJWjrYi9Ek+ dhGOecm4dEhhHefAI69amUV1WyMptR93XNFpSTxHKCYDkVjIhaKB8= Received: (qmail 16002 invoked by alias); 2 Apr 2013 16:06:52 -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 15930 invoked by uid 89); 2 Apr 2013 16:06:45 -0000 X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, MSGID_MULTIPLE_AT, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 02 Apr 2013 16:06:41 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Tue, 02 Apr 2013 17:06:38 +0100 Received: from e106372vm ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.0); Tue, 2 Apr 2013 17:06:37 +0100 From: "Kyrylo Tkachov" To: "Ramana Radhakrishnan" Cc: References: <514b4cf7.c9c4440a.7a38.789dSMTPIN_ADDED_BROKEN@mx.google.com> In-Reply-To: Subject: RE: [PATCH][ARM] minmax_arithsi for non-canonical operand order with MINUS operator Date: Tue, 2 Apr 2013 17:06:14 +0100 Message-ID: <019501ce2fbc$00ea3830$02bea890$@tkachov@arm.com> MIME-Version: 1.0 X-MC-Unique: 113040217063809601 X-Virus-Found: No > From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com] > Sent: 02 April 2013 11:10 > To: Kyrylo Tkachov > Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw; Ramana Radhakrishnan > Subject: Re: [PATCH][ARM] minmax_arithsi for non-canonical operand > order with MINUS operator > > On Thu, Mar 21, 2013 at 6:09 PM, Kyrylo Tkachov > wrote: > > Hi all, > > > > This patch adds a splitter variant of the minmax_arithsi pattern for > when > > the operator > > is non-commutative (MINUS) and the ordering of the operands is not > > canonical. > > > > That is, it will trigger for: > > #define MAX(a, b) (a > b ? a : b) > > int > > foo (int a, int b, int c) > > { > > return c - MAX (a,b); > > } > > > > and will generate: > > cmp r1, r0 > > rsbge r0, r1, r2 > > rsblt r0, r0, r2 > > > > instead of the current: > > cmp r0, r1 > > movlt r0, r1 > > rsb r0, r0, r2 > > > > No regressions on arm-none-eabi. > > > > Ok for trunk? > > > > Split after reload please into cond-exec or use if_then_else instead > if you are splitting before reload - I originally thought it to be > safe when you asked me, but then have gone back and corrected myself. > > Read this thread . http://patches.linaro.org/6469/ . Hi Ramana, thanks for the review. How about this? We split after reload now. Using if_then_else got me a lot of unrecognisable instruction ICEs and delaying the split till after reload seemed like a cleaner approach. Tested arm-none-eabi on qemu. gcc/ 2013-04-02 Kyrylo Tkachov * config/arm/arm.md (minmax_arithsi_non_canon): New pattern. gcc/testsuite 2013-04-02 Kyrylo Tkachov * gcc.target/arm/minmax_minus.c: New test. > regards > Ramana > Thanks, Kyrill diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 8895080..7ff066e 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -3417,6 +3417,50 @@ (const_int 12)))] ) +; Reject the frame pointer in operand[1], since reloading this after +; it has been eliminated can cause carnage. +(define_insn_and_split "*minmax_arithsi_non_canon" + [(set (match_operand:SI 0 "s_register_operand" "=r,r") + (minus:SI + (match_operand:SI 1 "s_register_operand" "0,?r") + (match_operator:SI 4 "minmax_operator" + [(match_operand:SI 2 "s_register_operand" "r,r") + (match_operand:SI 3 "arm_rhs_operand" "rI,rI")]))) + (clobber (reg:CC CC_REGNUM))] + "TARGET_32BIT && !arm_eliminable_register (operands[1])" + "#" + "TARGET_32BIT && !arm_eliminable_register (operands[1]) && reload_completed" + [(set (reg:CC CC_REGNUM) + (compare:CC (match_dup 2) (match_dup 3))) + + (cond_exec (match_op_dup 4 [(reg:CC CC_REGNUM) (const_int 0)]) + (set (match_dup 0) + (minus:SI (match_dup 1) + (match_dup 2)))) + (cond_exec (match_op_dup 5 [(reg:CC CC_REGNUM) (const_int 0)]) + (set (match_dup 0) + (minus:SI (match_dup 1) + (match_dup 3))))] + { + enum machine_mode mode = SELECT_CC_MODE (GET_CODE (operands[1]), + operands[2], operands[3]); + enum rtx_code rc = minmax_code (operands[4]); + operands[4] = gen_rtx_fmt_ee (rc, VOIDmode, + operands[2], operands[3]); + + if (mode == CCFPmode || mode == CCFPEmode) + rc = reverse_condition_maybe_unordered (rc); + else + rc = reverse_condition (rc); + operands[5] = gen_rtx_fmt_ee (rc, SImode, operands[2], operands[3]); + } + [(set_attr "conds" "clob") + (set (attr "length") + (if_then_else (eq_attr "is_thumb" "yes") + (const_int 14) + (const_int 12)))] +) + (define_code_iterator SAT [smin smax]) (define_code_iterator SATrev [smin smax]) (define_code_attr SATlo [(smin "1") (smax "2")]) diff --git a/gcc/testsuite/gcc.target/arm/minmax_minus.c b/gcc/testsuite/gcc.target/arm/minmax_minus.c new file mode 100644 index 0000000..050c847 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/minmax_minus.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +#define MAX(a, b) (a > b ? a : b) +int +foo (int a, int b, int c) +{ + return c - MAX (a, b); +} + +/* { dg-final { scan-assembler "rsbge" } } */ +/* { dg-final { scan-assembler "rsblt" } } */