RX: Fix LT and GE comparisons

RX: Fix LT and GE comparisons

[Nick Clifton]

Hi Guys,

  I am checking in the patch below to fix the LT and GE comparisons on
  the RX target.  These need the Overflow bit to be valid in the
  processor status word.

  The patch fixes 27 failures in the gcc testsuite.

Cheers
  Nick

gcc/ChangeLog
2011-02-03  Nick Clifton  <nickc@redhat.com>

	* config/rx/predicates.md (rx_zs_comparison_operator): Remove
	lt and ge.
	* config/rx/rx.md (abssi2_flags): Use CC_ZSmode rather than
	CC_ZSOmode.
	* config/rx/rx.c (rx_print_operand): Use "lt" and "ge" suffixes
	instead of "n" and "pz".
	(flags_from_code): LT and GE tests need CC_FLAG_O as well as
	CC_FLAG_S.

Index: gcc/config/rx/predicates.md
===================================================================
--- gcc/config/rx/predicates.md	(revision 169784)
+++ gcc/config/rx/predicates.md	(working copy)
@@ -284,7 +284,7 @@
 )
 
 (define_predicate "rx_zs_comparison_operator"
-  (match_code "eq,ne,lt,ge")
+  (match_code "eq,ne")
 )
 
 ;; GT and LE omitted due to operand swap required.
Index: gcc/config/rx/rx.md
===================================================================
--- gcc/config/rx/rx.md	(revision 169784)
+++ gcc/config/rx/rx.md	(working copy)
@@ -797,7 +797,10 @@
    (set (reg CC_REG)
 	(compare (abs:SI (match_dup 1))
 		 (const_int 0)))]
-  "reload_completed && rx_match_ccmode (insn, CC_ZSOmode)"
+  ;; Note - although the ABS instruction does set the O bit in the processor
+  ;; status word, it does not do so in a way that is comparable with the CMP
+  ;; instruction.  Hence we use CC_ZSmode rather than CC_ZSOmode.
+  "reload_completed && rx_match_ccmode (insn, CC_ZSmode)"
   "@
   abs\t%0
   abs\t%1, %0"
Index: gcc/config/rx/rx.c
===================================================================
--- gcc/config/rx/rx.c	(revision 169784)
+++ gcc/config/rx/rx.c	(working copy)
@@ -450,10 +450,10 @@
 	    switch (code)
 	      {
 	      case LT:
-		ret = "n";
+		ret = "lt";
 		break;
 	      case GE:
-		ret = "pz";
+		ret = "ge";
 		break;
 	      case GT:
 		ret = "gt";
@@ -2625,7 +2625,7 @@
     {
     case LT:
     case GE:
-      return CC_FLAG_S;
+      return CC_FLAG_S | CC_FLAG_O;
     case GT:
     case LE:
       return CC_FLAG_S | CC_FLAG_O | CC_FLAG_Z;

Re: RX: Fix LT and GE comparisons

[Richard Henderson]

On 02/03/2011 03:42 AM, Nick Clifton wrote:
>  (define_predicate "rx_zs_comparison_operator"
> -  (match_code "eq,ne,lt,ge")
> +  (match_code "eq,ne")

Err... why?  One can perform X < 0 and X >= 0 with just the sign bit.

>  	      case LT:
> -		ret = "n";
> +		ret = "lt";
>  		break;
>  	      case GE:
> -		ret = "pz";
> +		ret = "ge";

One can use a slightly more complicated expression here to accommodate
the above.  C.f. the mn10300 port:

            case GE:
              /* bge is smaller than bnc.  */
              str = (have_flags & CC_FLAG_V ? "ge" : "nc");

A construct like this would allow you to use "n" when the O flag is
not present, and "lt" when a full cmp opcode is in use.

>      case LT:
>      case GE:
> -      return CC_FLAG_S;
> +      return CC_FLAG_S | CC_FLAG_O;

I suppose one could verify that Y is const0_rtx in rx_select_cc_mode before
selecting CC_FLAG_S for these codes.  I thought it was already always zero
though, thus the trivial implementation of that routine.

Can you point me to one of the tests that are fixed by this change?


r~

Re: RX: Fix LT and GE comparisons

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On 02/04/2011 10:19 AM, Richard Henderson wrote:
> I suppose one could verify that Y is const0_rtx in rx_select_cc_mode before
> selecting CC_FLAG_S for these codes.  I thought it was already always zero
> though, thus the trivial implementation of that routine.
> 
> Can you point me to one of the tests that are fixed by this change?

Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
before I ran tests last night, but see if this patch doesn't solve the 
problem you were trying to address.


r~

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 1914 bytes --]

diff --git a/gcc/config/rx/predicates.md b/gcc/config/rx/predicates.md
index 82cac42..77b3353 100644
--- a/gcc/config/rx/predicates.md
+++ b/gcc/config/rx/predicates.md
@@ -284,7 +284,7 @@
 )
 
 (define_predicate "rx_zs_comparison_operator"
-  (match_code "eq,ne")
+  (match_code "eq,ne,lt,ge")
 )
 
 ;; GT and LE omitted due to operand swap required.
diff --git a/gcc/config/rx/rx.c b/gcc/config/rx/rx.c
index 839523f..e01f453 100644
--- a/gcc/config/rx/rx.c
+++ b/gcc/config/rx/rx.c
@@ -447,13 +447,14 @@ rx_print_operand (FILE * file, rtx op, int letter)
 	  }
 	else
 	  {
+	    unsigned int flags = flags_from_mode (mode);
 	    switch (code)
 	      {
 	      case LT:
-		ret = "lt";
+		ret = (flags & CC_FLAG_O ? "lt" : "n");
 		break;
 	      case GE:
-		ret = "ge";
+		ret = (flags & CC_FLAG_O ? "ge" : "pz");
 		break;
 	      case GT:
 		ret = "gt";
@@ -482,8 +483,7 @@ rx_print_operand (FILE * file, rtx op, int letter)
 	      default:
 		gcc_unreachable ();
 	      }
-	    gcc_checking_assert ((flags_from_code (code)
-				  & ~flags_from_mode (mode)) == 0);
+	    gcc_checking_assert ((flags_from_code (code) & ~flags) == 0);
 	  }
 	fputs (ret, file);
 	break;
@@ -2625,7 +2625,7 @@ flags_from_code (enum rtx_code code)
     {
     case LT:
     case GE:
-      return CC_FLAG_S | CC_FLAG_O;
+      return CC_FLAG_S;
     case GT:
     case LE:
       return CC_FLAG_S | CC_FLAG_O | CC_FLAG_Z;
@@ -2666,11 +2666,14 @@ rx_cc_modes_compatible (enum machine_mode m1, enum machine_mode m2)
 /* Return the minimal CC mode needed to implement (CMP_CODE X Y).  */
 
 enum machine_mode
-rx_select_cc_mode (enum rtx_code cmp_code, rtx x, rtx y ATTRIBUTE_UNUSED)
+rx_select_cc_mode (enum rtx_code cmp_code, rtx x, rtx y)
 {
   if (GET_MODE_CLASS (GET_MODE (x)) == MODE_FLOAT)
     return CC_Fmode;
 
+  if (y != const0_rtx)
+    return CCmode;
+
   return mode_from_flags (flags_from_code (cmp_code));
 }
 

Re: RX: Fix LT and GE comparisons

[Nick Clifton]

Hi Richard,

> Can you point me to one of the tests that are fixed by this change?

  PASS: gcc.c-torture/execute/20050104-1.c execution,  -O0

And at higher optimization levels.  There is also:

  gcc.c-torture/execute/cmpsi-2.c
  gcc.c-torture/execute/int-compare.c
  gcc.c-torture/execute/pr28651.c

I am about to apply your patch and rerun the testsuite to see what 
effect it has.

Cheers
   Nick

Re: RX: Fix LT and GE comparisons

[Nick Clifton]

Hi Richard,


> Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
> before I ran tests last night, but see if this patch doesn't solve the
> problem you were trying to address.

Hmm, it does fix the same testcases that my patch fixed.  But it also 
appears to introduce a new failure:

   gcc/testsuite/gcc.dg/tree-ssa/pr42327.c: In function 'foo':
   gcc/testsuite/gcc.dg/tree-ssa/pr42327.c:4:6: internal compiler error:
    Segmentation fault

However this appears to be a memory corruption in the omega code, which 
is unlikely to be connected with RX comparison instructions.  So I think 
that maybe it is just a case that a new bug has been introduced into the 
gcc sources between the time that I ran my pre-patch tests and today. 
I'll try reverting your patch and my patch and then running the 
testsuite again to make absolutely sure.

Cheers
   Nick

Re: RX: Fix LT and GE comparisons

[Nick Clifton]

Hi Richard,

> Irritatingly, I didn't copy the dejagnu magic for rx-sim onto this machine
> before I ran tests last night, but see if this patch doesn't solve the
> problem you were trying to address.

I have now rerun the gcc testsuite without your patch applied or mine, 
and unfortunately gcc.dg/tree-ssa/pr42327.c passes.  So there must be 
something in your patch that is triggering this failure.  Any ideas ?

Cheers
   Nick

Re: RX: Fix LT and GE comparisons

[Richard Henderson]

On 02/08/2011 04:32 AM, Nick Clifton wrote:
> I have now rerun the gcc testsuite without your patch applied or
> mine, and unfortunately gcc.dg/tree-ssa/pr42327.c passes.  So there
> must be something in your patch that is triggering this failure.  Any
> ideas ?

None.  I can't reproduce this with r169926 with and without my patch.


r~

Re: RX: Fix LT and GE comparisons

[Nick Clifton]

Hi Richard,

> None.  I can't reproduce this with r169926 with and without my patch.

Hmm, must be something in my local setup then.  Could you check your 
patch in please ?

Cheers
   Nick