diff mbox

RX: Fix LT and GE comparisons

Message ID m3ei7pe44t.fsf@redhat.com
State New
Headers show

Commit Message

Nick Clifton Feb. 3, 2011, 11:42 a.m. UTC 
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.

Comments

Richard Henderson Feb. 4, 2011, 6:19 p.m. UTC | #1 
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~
Nick Clifton Feb. 7, 2011, 3:05 p.m. UTC | #2 
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
diff mbox

Patch

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;