On Mon, 2012-01-30 at 10:46 +0100, Richard Guenther wrote: > On Fri, Jan 27, 2012 at 5:56 PM, Peter Bergner <berg...@vnet.ibm.com> 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 <berg...@vnet.ibm.com> Michael Matz <m...@suse.de> 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; +}