public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Fix PR16458, eliminate redudant compares
@ 2012-01-27 16:58 Peter Bergner
  2012-01-30  9:49 ` Richard Guenther
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Bergner @ 2012-01-27 16:58 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Edelsohn

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).

I'll note that for the moment, I have XFAILed pr16458-4.c, since this
test case isn't working yet, but it is due to a problem in expand not
attaching any type expression information on *index's reg rtx like it
does for *a and *b in pr16458-2.c.  We're tracking that down for 4.8.

This has passed bootstrap and regtesting with no regressions.
Ok for mainline?

Peter

gcc/
	PR target/16458
	* config/rs6000/rs6000.c (rs6000_unsigned_reg_p): New function.
	(rs6000_generate_compare): Use it.

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/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 183628)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -15588,6 +15588,22 @@ rs6000_reverse_condition (enum machine_m
     return reverse_condition (code);
 }
 
+static bool
+rs6000_unsigned_reg_p (rtx op)
+{
+  enum rtx_code code = GET_CODE (op);
+
+  if (code == REG
+      && REG_EXPR (op)
+      && TYPE_UNSIGNED (TREE_TYPE (REG_EXPR (op))))
+    return true;
+
+  if (code == SUBREG && SUBREG_PROMOTED_UNSIGNED_P (op))
+    return true;
+
+  return false;
+}
+
 /* Generate a compare for CODE.  Return a brand-new rtx that
    represents the result of the compare.  */
 
@@ -15606,14 +15622,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))
+	   && rs6000_unsigned_reg_p (op0)
+	   && (rs6000_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/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 = 0,
+  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" { xfail *-*-* } } } */
+/* { dg-final { scan-assembler-times "cmplwi" 5 { xfail *-*-* } } } */
+
+extern int case0 (void);
+extern int case1 (void);
+extern int case2 (void);
+extern int case3 (void);
+extern int case4 (void);
+
+enum CASE_VALUES
+{
+  CASE0 = 0,
+  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;
+}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-01-27 16:58 [PATCH, rs6000] Fix PR16458, eliminate redudant compares Peter Bergner
@ 2012-01-30  9:49 ` Richard Guenther
  2012-01-31 14:56   ` Peter Bergner
  2012-04-10 17:37   ` Peter Bergner
  0 siblings, 2 replies; 7+ messages in thread
From: Richard Guenther @ 2012-01-30  9:49 UTC (permalink / raw)
  To: Peter Bergner; +Cc: gcc-patches, David Edelsohn

On Fri, Jan 27, 2012 at 5:56 PM, Peter Bergner <bergner@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).
>
> I'll note that for the moment, I have XFAILed pr16458-4.c, since this
> test case isn't working yet, but it is due to a problem in expand not
> attaching any type expression information on *index's reg rtx like it
> does for *a and *b in pr16458-2.c.  We're tracking that down for 4.8.
>
> This has passed bootstrap and regtesting with no regressions.
> Ok for mainline?

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.  The question is - do we ever coalesce signed
and unsigned variables to the same pseudo?

IIRC avr people recently have come across the same idea.

Richard.

> Peter
>
> gcc/
>        PR target/16458
>        * config/rs6000/rs6000.c (rs6000_unsigned_reg_p): New function.
>        (rs6000_generate_compare): Use it.
>
> 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/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 183628)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -15588,6 +15588,22 @@ rs6000_reverse_condition (enum machine_m
>     return reverse_condition (code);
>  }
>
> +static bool
> +rs6000_unsigned_reg_p (rtx op)
> +{
> +  enum rtx_code code = GET_CODE (op);
> +
> +  if (code == REG
> +      && REG_EXPR (op)
> +      && TYPE_UNSIGNED (TREE_TYPE (REG_EXPR (op))))
> +    return true;
> +
> +  if (code == SUBREG && SUBREG_PROMOTED_UNSIGNED_P (op))
> +    return true;
> +
> +  return false;
> +}
> +
>  /* Generate a compare for CODE.  Return a brand-new rtx that
>    represents the result of the compare.  */
>
> @@ -15606,14 +15622,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))
> +          && rs6000_unsigned_reg_p (op0)
> +          && (rs6000_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/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 = 0,
> +  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" { xfail *-*-* } } } */
> +/* { dg-final { scan-assembler-times "cmplwi" 5 { xfail *-*-* } } } */
> +
> +extern int case0 (void);
> +extern int case1 (void);
> +extern int case2 (void);
> +extern int case3 (void);
> +extern int case4 (void);
> +
> +enum CASE_VALUES
> +{
> +  CASE0 = 0,
> +  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;
> +}
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-01-30  9:49 ` Richard Guenther
@ 2012-01-31 14:56   ` Peter Bergner
  2012-04-10 17:37   ` Peter Bergner
  1 sibling, 0 replies; 7+ messages in thread
From: Peter Bergner @ 2012-01-31 14:56 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, David Edelsohn, William J. Schmidt

On Mon, 2012-01-30 at 10:46 +0100, Richard Guenther wrote:
> On Fri, Jan 27, 2012 at 5:56 PM, Peter Bergner <bergner@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).


> > I'll note that for the moment, I have XFAILed pr16458-4.c, since this
> > test case isn't working yet, but it is due to a problem in expand not
> > attaching any type expression information on *index's reg rtx like it
> > does for *a and *b in pr16458-2.c.  We're tracking that down for 4.8.

I asked Bill to have a look at this one and he emailed the gcc mailing
list regarding the issue.  It looks like micha has a working answer:

  http://gcc.gnu.org/ml/gcc/2012-01/msg00349.html



> > This has passed bootstrap and regtesting with no regressions.
> > Ok for mainline?
> 
> This does not sound suitable for stage4.  

Since this has been around for a while, I'm fine with waiting until
stage1.


> 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.

If other ports like avr can make use of it too, I have no problems
moving it somewhere else.


> The question is - do we ever coalesce signed
> and unsigned variables to the same pseudo?

What is it you are worried about?  The patch is only attempting
to emit unsigned compares for operands we know to be unsigned.
The fact that the compares are equality compares means we can
use either signed or unsigned compares.  It's just that the signed
versions don't CSE with any following non-equality unsigned compares
with the same unsigned operands.


> IIRC avr people recently have come across the same idea.

Do you have a pointer for this?


Peter



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-01-30  9:49 ` Richard Guenther
  2012-01-31 14:56   ` Peter Bergner
@ 2012-04-10 17:37   ` Peter Bergner
  2012-04-10 18:51     ` David Edelsohn
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Bergner @ 2012-04-10 17:37 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, David Edelsohn, Michael Matz

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




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-04-10 17:37   ` Peter Bergner
@ 2012-04-10 18:51     ` David Edelsohn
  2012-04-11  8:43       ` Richard Guenther
  0 siblings, 1 reply; 7+ messages in thread
From: David Edelsohn @ 2012-04-10 18:51 UTC (permalink / raw)
  To: Peter Bergner; +Cc: Richard Guenther, gcc-patches, Michael Matz

On Tue, Apr 10, 2012 at 1:36 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:

> 2012-mm-dd  Peter Bergner  <bergner@vnet.ibm.com>
>            Michael Matz  <matz@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.

The rs6000 and testsuite parts of the patch are okay with me.

Thanks, David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-04-10 18:51     ` David Edelsohn
@ 2012-04-11  8:43       ` Richard Guenther
  2012-04-11 11:55         ` Peter Bergner
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Guenther @ 2012-04-11  8:43 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Peter Bergner, gcc-patches, Michael Matz

On Tue, Apr 10, 2012 at 8:50 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> On Tue, Apr 10, 2012 at 1:36 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
>
>> 2012-mm-dd  Peter Bergner  <bergner@vnet.ibm.com>
>>            Michael Matz  <matz@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.
>
> The rs6000 and testsuite parts of the patch are okay with me.

The rest is ok as well.

Thanks,
Richard.

> Thanks, David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, rs6000] Fix PR16458, eliminate redudant compares
  2012-04-11  8:43       ` Richard Guenther
@ 2012-04-11 11:55         ` Peter Bergner
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Bergner @ 2012-04-11 11:55 UTC (permalink / raw)
  To: Richard Guenther; +Cc: David Edelsohn, gcc-patches, Michael Matz

On Wed, 2012-04-11 at 10:43 +0200, Richard Guenther wrote:
> On Tue, Apr 10, 2012 at 8:50 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> > On Tue, Apr 10, 2012 at 1:36 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> >
> >> 2012-mm-dd  Peter Bergner  <bergner@vnet.ibm.com>
> >>            Michael Matz  <matz@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.
> >
> > The rs6000 and testsuite parts of the patch are okay with me.
> 
> The rest is ok as well.

Thanks, I have committed this as revision 186312.

Peter



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-04-11 11:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-27 16:58 [PATCH, rs6000] Fix PR16458, eliminate redudant compares Peter Bergner
2012-01-30  9:49 ` Richard Guenther
2012-01-31 14:56   ` Peter Bergner
2012-04-10 17:37   ` Peter Bergner
2012-04-10 18:51     ` David Edelsohn
2012-04-11  8:43       ` Richard Guenther
2012-04-11 11:55         ` Peter Bergner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).