diff mbox

[AVR] : Fix cc0 and loading const_int/const_double

Message ID 4ECA5C04.2030600@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Nov. 21, 2011, 2:11 p.m. UTC 
After updating my local copy I get new runtime FAILs in the test suite because
of the following sequence, e.g. from gcc.c-torture/execute/990527-1.c:

	sbiw r28,1	 ;  12	addhi3_clobber/1	[length = 1]
	ldi r24,lo8(9)	 ;  24	*movhi/5	[length = 2]
	clr r25
	brne .L3	 ;  16	branch	[length = 1]

The trouble is that *movhi promises that it does not change cc0. Thus, the
correct code is:

	sbiw r28,1	 ;  12	addhi3_clobber/1	[length = 1]
	ldi r24,lo8(9)	 ;  24	*movhi/5	[length = 2]
	ldi r25,0
	brne .L3	 ;  16	branch	[length = 1]

The patch fixes this and adjusts cc attribute of involved insns.

When loading 0, cc=none is more useful than cc=set_zn because the compiler
knows what it loads, and cc=none preserves cc0 from preceding code.

Tests pass fine again.

Ok to apply?

Johann

	* config/avr/avr.c (output_reload_in_const): Loading a byte with 0
	must not affect cc0.
	* config/avr/avr.md (*movhi, *movpsi, *movsi, *movsf): Zero to any
	register does not change cc0. Same for any constant to ld-register.

Comments

Denis Chertykov Nov. 21, 2011, 5:26 p.m. UTC | #1 
2011/11/21 Georg-Johann Lay <avr@gjlay.de>:
> After updating my local copy I get new runtime FAILs in the test suite because
> of the following sequence, e.g. from gcc.c-torture/execute/990527-1.c:
>
>        sbiw r28,1       ;  12  addhi3_clobber/1        [length = 1]
>        ldi r24,lo8(9)   ;  24  *movhi/5        [length = 2]
>        clr r25
>        brne .L3         ;  16  branch  [length = 1]
>
> The trouble is that *movhi promises that it does not change cc0. Thus, the
> correct code is:
>
>        sbiw r28,1       ;  12  addhi3_clobber/1        [length = 1]
>        ldi r24,lo8(9)   ;  24  *movhi/5        [length = 2]
>        ldi r25,0
>        brne .L3         ;  16  branch  [length = 1]
>
> The patch fixes this and adjusts cc attribute of involved insns.
>
> When loading 0, cc=none is more useful than cc=set_zn because the compiler
> knows what it loads, and cc=none preserves cc0 from preceding code.
>
> Tests pass fine again.
>
> Ok to apply?
>
> Johann
>
>        * config/avr/avr.c (output_reload_in_const): Loading a byte with 0
>        must not affect cc0.
>        * config/avr/avr.md (*movhi, *movpsi, *movsi, *movsf): Zero to any
>        register does not change cc0. Same for any constant to ld-register.
>
>

Approved.

Denis.
diff mbox

Patch

Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 181554)
+++ config/avr/avr.md	(working copy)
@@ -649,7 +649,7 @@  (define_insn "*movhi"
   }
   [(set_attr "length" "2,2,6,7,2,6,5,2")
    (set_attr "adjust_len" "mov16")
-   (set_attr "cc" "none,clobber,clobber,clobber,none,clobber,none,none")])
+   (set_attr "cc" "none,none,clobber,clobber,none,clobber,none,none")])
 
 (define_peephole2 ; movw
   [(set (match_operand:QI 0 "even_register_operand" "")
@@ -752,7 +752,7 @@  (define_insn "*movpsi"
   }
   [(set_attr "length" "3,3,8,9,4,10")
    (set_attr "adjust_len" "mov24")
-   (set_attr "cc" "none,set_zn,clobber,clobber,clobber,clobber")])
+   (set_attr "cc" "none,none,clobber,clobber,none,clobber")])
   
 ;;==========================================================================
 ;; move double word (32 bit)
@@ -793,7 +793,7 @@  (define_insn "*movsi"
   }
   [(set_attr "length" "4,4,8,9,4,10")
    (set_attr "adjust_len" "mov32")
-   (set_attr "cc" "none,set_zn,clobber,clobber,clobber,clobber")])
+   (set_attr "cc" "none,none,clobber,clobber,none,clobber")])
 
 ;; fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
 ;; move floating point numbers (32 bit)
@@ -809,7 +809,7 @@  (define_insn "*movsf"
   }
   [(set_attr "length" "4,4,8,9,4,10")
    (set_attr "adjust_len" "mov32")
-   (set_attr "cc" "none,set_zn,clobber,clobber,clobber,clobber")])
+   (set_attr "cc" "none,none,clobber,clobber,none,clobber")])
 
 (define_peephole2 ; *reload_insf
   [(match_scratch:QI 2 "d")
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 181554)
+++ config/avr/avr.c	(working copy)
@@ -8836,7 +8836,13 @@  avr_regno_mode_code_ok_for_base_p (int r
    LEN != NULL: set *LEN to the length of the instruction sequence
                 (in words) printed with LEN = NULL.
    If CLEAR_P is true, OP[0] had been cleard to Zero already.
-   If CLEAR_P is false, nothing is known about OP[0].  */
+   If CLEAR_P is false, nothing is known about OP[0].
+
+   The effect on cc0 is as follows:
+
+   Load 0 to any register          : NONE
+   Load ld register with any value : NONE
+   Anything else:                  : CLOBBER  */
 
 static void
 output_reload_in_const (rtx *op, rtx clobber_reg, int *len, bool clear_p)
@@ -8914,7 +8920,7 @@  output_reload_in_const (rtx *op, rtx clo
           xop[2] = clobber_reg;
 
           if (n >= 2 + (avr_current_arch->n_segments > 1))
-            avr_asm_len ("clr %0", xop, len, 1);
+            avr_asm_len ("mov %0,__zero_reg__", xop, len, 1);
           else
             avr_asm_len (asm_code[n][ldreg_p], xop, len, ldreg_p ? 1 : 2);
           continue;
@@ -8946,14 +8952,13 @@  output_reload_in_const (rtx *op, rtx clo
             }
         }
 
-      /* Use CLR to zero a value so that cc0 is set as expected
-         for zero.  */
+      /* Don't use CLR so that cc0 is set as expected.  */
       
       if (ival[n] == 0)
         {
           if (!clear_p)
-            avr_asm_len ("clr %0", &xdest[n], len, 1);
-          
+            avr_asm_len (ldreg_p ? "ldi %0,0" : "mov %0,__zero_reg__",
+                         &xdest[n], len, 1);
           continue;
         }