From patchwork Tue Apr 10 17:36:52 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Bergner X-Patchwork-Id: 151676 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]) by ozlabs.org (Postfix) with SMTP id 43897B7051 for ; Wed, 11 Apr 2012 03:38:32 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1334684312; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:Received:Message-ID:Subject:From:To: Cc:Date:In-Reply-To:References:Content-Type: Content-Transfer-Encoding:Mime-Version:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=rDT3uLZPWc5suiCX5NfSmzlvANM=; b=qqYguE2vBzwnozq xBYqKrGM8LuCdUPn8YEYj4Ws8PEmqC//qwaOn9XdTWtRsXe9QBkUVH88sUVfiCe4 fixiKGlTU2t8OPmProK4TmdQBL7yOiLdCAf6AXeOHL34KNKEsaXQ57Dl85I5hEE/ 1PUrLaDgwEqpZzU8Z3euo63Mpp0w= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:Received:Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References:Content-Type:Content-Transfer-Encoding:Mime-Version:X-Content-Scanned:x-cbid:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=BC4sm3N7keIxe0ZXAMhLDUMG8EXGRe/GKENJC1OflcXkfg1R6Ovf/J8Qsvdqm6 0VdlW9jfS13C4D6UzgFf6MvlYvyBgPZYSVrqzbuwoNrvKFhs14ebCda237Une/Ki GbyN8VdpQEvBU28Bo2mxdQGT6vD9VWPiMxajnNWZjgU3k=; Received: (qmail 21491 invoked by alias); 10 Apr 2012 17:37:59 -0000 Received: (qmail 21283 invoked by uid 22791); 10 Apr 2012 17:37:55 -0000 X-SWARE-Spam-Status: No, hits=-4.4 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_SOFTFAIL, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e37.co.us.ibm.com (HELO e37.co.us.ibm.com) (32.97.110.158) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 Apr 2012 17:37:35 +0000 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 10 Apr 2012 11:37:35 -0600 Received: from d01dlp03.pok.ibm.com (9.56.224.17) by e37.co.us.ibm.com (192.168.1.137) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 10 Apr 2012 11:37:11 -0600 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 97973C9006F for ; Tue, 10 Apr 2012 13:37:09 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q3AHbAvU165382 for ; Tue, 10 Apr 2012 13:37:10 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q3AHb7Hm002248 for ; Tue, 10 Apr 2012 13:37:07 -0400 Received: from [192.168.1.103] (vorma.rchland.ibm.com [9.10.86.174]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id q3AHawYQ001370; Tue, 10 Apr 2012 13:36:59 -0400 Message-ID: <1334079412.10401.14.camel@otta> Subject: Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares From: Peter Bergner To: Richard Guenther Cc: "gcc-patches@gcc.gnu.org" , David Edelsohn , Michael Matz Date: Tue, 10 Apr 2012 12:36:52 -0500 In-Reply-To: References: <1327683378.14063.28.camel@otta> Mime-Version: 1.0 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 12041017-7408-0000-0000-00000419325F X-IsSubscribed: yes 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 On Mon, 2012-01-30 at 10:46 +0100, Richard Guenther wrote: > On Fri, Jan 27, 2012 at 5:56 PM, Peter Bergner wrote: > > This patch fixes PR16458 by using the type expression attached to a reg > > rtx to detect its signedness and generating unsigned compares when > > appropriate. However, we continue to use signed compares for the > > special case of when we compare an unsigned reg against the constant 0. > > This is because signed and unsigned compares give the same results > > for equality and "(unsigned)x > 0)" is equivalent to "x != 0". > > Using a signed compare in this special case allows us to continue to > > generate record form instructions (ie, instructions that implicitly > > set cr0). [snip] > > This does not sound suitable for stage4. rs6000_unsigned_reg_p > looks suspiciously non-rs6000 specific, and if we really can rely > on the way it is implemented should find its way to general RTL > helper routines. Now that we're in stage1 again, I'm attaching an updated patch that moves rs6000_unsigned_reg_p into an arch independent RTL file like you wanted. I have also attached a couple of patch hunks from Michael that sets register attributes in a couple of cases we didn't before and that allows my patch to coalesce the compares in pr16458-4.c. The description of that issue is located in: http://gcc.gnu.org/ml/gcc/2012-01/msg00339.html The updated and expanded patch passes bootstrap and regtesting with no regessions on powerpc64-linux. Ok for mainline now? Peter 2012-mm-dd Peter Bergner Michael Matz gcc/ PR target/16458 * rtlanal.c (unsigned_reg_p): New function. * rtl.h (unsigned_reg_p): Prototype it. * config/rs6000/rs6000.c (rs6000_generate_compare): Use it. Update comment. * expr.c (expand_expr_real_1): Set register attributes. * stmt.c (expand_case): Likewise. gcc/testsuite/ PR target/16458 * gcc.target/powerpc/pr16458-1.c: New test. * gcc.target/powerpc/pr16458-2.c: Likewise. * gcc.target/powerpc/pr16458-3.c: Likewise. * gcc.target/powerpc/pr16458-4.c: Likewise. Index: gcc/rtlanal.c =================================================================== --- gcc/rtlanal.c (revision 186244) +++ gcc/rtlanal.c (working copy) @@ -636,6 +636,25 @@ count_occurrences (const_rtx x, const_rt } +/* Return TRUE if OP is a register or subreg of a register that + holds an unsigned quantity. Otherwise, return FALSE. */ + +bool +unsigned_reg_p (rtx op) +{ + if (REG_P (op) + && REG_EXPR (op) + && TYPE_UNSIGNED (TREE_TYPE (REG_EXPR (op)))) + return true; + + if (GET_CODE (op) == SUBREG + && SUBREG_PROMOTED_UNSIGNED_P (op)) + return true; + + return false; +} + + /* Nonzero if register REG appears somewhere within IN. Also works if REG is not a register; in this case it checks for a subexpression of IN that is Lisp "equal" to REG. */ Index: gcc/rtl.h =================================================================== --- gcc/rtl.h (revision 186244) +++ gcc/rtl.h (working copy) @@ -1909,6 +1909,7 @@ extern HOST_WIDE_INT get_integer_term (c extern rtx get_related_value (const_rtx); extern bool offset_within_block_p (const_rtx, HOST_WIDE_INT); extern void split_const (rtx, rtx *, rtx *); +extern bool unsigned_reg_p (rtx); extern int reg_mentioned_p (const_rtx, const_rtx); extern int count_occurrences (const_rtx, const_rtx, int); extern int reg_referenced_p (const_rtx, const_rtx); Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 186244) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -15570,14 +15570,11 @@ rs6000_generate_compare (rtx cmp, enum m || code == GEU || code == LEU) comp_mode = CCUNSmode; else if ((code == EQ || code == NE) - && GET_CODE (op0) == SUBREG - && GET_CODE (op1) == SUBREG - && SUBREG_PROMOTED_UNSIGNED_P (op0) - && SUBREG_PROMOTED_UNSIGNED_P (op1)) + && unsigned_reg_p (op0) + && (unsigned_reg_p (op1) + || (CONST_INT_P (op1) && INTVAL (op1) != 0))) /* These are unsigned values, perhaps there will be a later - ordering compare that can be shared with this one. - Unfortunately we cannot detect the signedness of the operands - for non-subregs. */ + ordering compare that can be shared with this one. */ comp_mode = CCUNSmode; else comp_mode = CCmode; Index: gcc/expr.c =================================================================== --- gcc/expr.c (revision 186244) +++ gcc/expr.c (working copy) @@ -9015,8 +9015,13 @@ expand_expr_real_1 (tree exp, rtx target && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp))) g = SSA_NAME_DEF_STMT (exp); if (g) - return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode, - modifier, NULL); + { + rtx r = expand_expr_real (gimple_assign_rhs_to_tree (g), target, + tmode, modifier, NULL); + if (REG_P (r) && !REG_EXPR (r)) + set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r); + return r; + } ssa_name = exp; decl_rtl = get_rtx_for_ssa_name (ssa_name); Index: gcc/stmt.c =================================================================== --- gcc/stmt.c (revision 186244) +++ gcc/stmt.c (working copy) @@ -2381,7 +2381,11 @@ expand_case (gimple stmt) do_pending_stack_adjust (); if (MEM_P (index)) - index = copy_to_reg (index); + { + index = copy_to_reg (index); + if (TREE_CODE (index_expr) == SSA_NAME) + set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (index_expr), index); + } /* We generate a binary decision tree to select the appropriate target code. This is done as follows: Index: gcc/testsuite/gcc.target/powerpc/pr16458-1.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr16458-1.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr16458-1.c (revision 0) @@ -0,0 +1,18 @@ +/* Test cse'ing of unsigned compares. */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +/* { dg-final { scan-assembler-not "cmpw" } } */ +/* { dg-final { scan-assembler-times "cmplw" 1 } } */ + +unsigned int a, b; + +int +foo (void) +{ + if (a == b) return 1; + if (a > b) return 2; + if (a < b) return 3; + if (a != b) return 4; + return 0; +} Index: gcc/testsuite/gcc.target/powerpc/pr16458-2.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr16458-2.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr16458-2.c (revision 0) @@ -0,0 +1,18 @@ +/* Test cse'ing of unsigned compares. */ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +/* { dg-final { scan-assembler-not "cmpw" } } */ +/* { dg-final { scan-assembler-times "cmplw" 1 } } */ + +unsigned int *a, *b; + +int +foo (void) +{ + if (*a == *b) return 1; + if (*a > *b) return 2; + if (*a < *b) return 3; + if (*a != *b) return 4; + return 0; +} Index: gcc/testsuite/gcc.target/powerpc/pr16458-3.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr16458-3.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr16458-3.c (revision 0) @@ -0,0 +1,41 @@ +/* Test cse'ing of unsigned compares. */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-jump-tables" } */ + +/* { dg-final { scan-assembler-not "cmpwi" } } */ +/* { dg-final { scan-assembler-times "cmplwi" 5 } } */ + +extern int case0 (void); +extern int case1 (void); +extern int case2 (void); +extern int case3 (void); +extern int case4 (void); + +enum CASE_VALUES +{ + CASE0 = 1, + CASE1, + CASE2, + CASE3, + CASE4 +}; + +int +foo (enum CASE_VALUES index) +{ + switch (index) + { + case CASE0: + return case0 (); + case CASE1: + return case1 (); + case CASE2: + return case2 (); + case CASE3: + return case3 (); + case CASE4: + return case4 (); + } + + return 0; +} Index: gcc/testsuite/gcc.target/powerpc/pr16458-4.c =================================================================== --- gcc/testsuite/gcc.target/powerpc/pr16458-4.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr16458-4.c (revision 0) @@ -0,0 +1,44 @@ +/* Test cse'ing of unsigned compares. */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-jump-tables" } */ + +/* The following tests fail due to an issue in expand not + attaching an type expression information on *index's reg rtx. */ + +/* { dg-final { scan-assembler-not "cmpwi" } } */ +/* { dg-final { scan-assembler-times "cmplwi" 5 } } */ + +extern int case0 (void); +extern int case1 (void); +extern int case2 (void); +extern int case3 (void); +extern int case4 (void); + +enum CASE_VALUES +{ + CASE0 = 1, + CASE1, + CASE2, + CASE3, + CASE4 +}; + +int +foo (enum CASE_VALUES *index) +{ + switch (*index) + { + case CASE0: + return case0 (); + case CASE1: + return case1 (); + case CASE2: + return case2 (); + case CASE3: + return case3 (); + case CASE4: + return case4 (); + } + + return 0; +}