On Fri, Jan 27, 2012 at 5:56 PM, Peter Bergner <[email protected]> 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;
> +}
>
>
