diff mbox

[avr] Fix PR61055: Wrong branch instruction after INC, DEC.

Message ID 536B80F2.8020703@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay May 8, 2014, 1:04 p.m. UTC 
Some instructions like INC, DEC, NEG currently set cc0 to set_zn which is not 
the whole story because they also set the V flag.  This leads to a wrong branch 
instruction in the remainder as cc0 is used.  Fix is to do same as clobber cc0. 
  For the matter of clarity, I added a new cc0 alternative set_vzn for that case.

Moreover, ADIW sets cc0 to set_czn rather than set_zn.  This is the same as the 
action of a single ADD and like ADIW was modeled the old days (before 
avr_out_plus_1 was introduced to print the output).

No new regressions.

Ok to apply?

Johann


gcc/config/avr
	PR target/61055
	* config/avr/avr.md (cc): Add new attribute set_vzn.
	(addqi3, addqq3, adduqq3, subqi3, subqq3, subuqq3, negqi2) [cc]:
	Set cc insn attribute to set_vzn instead of set_zn for alternatives
	with INC, DEC or NEG.
	* config/avr/avr.c (avr_notice_update_cc): Handle SET_VZN.
	(avr_out_plus_1): ADIW sets cc0 to CC_SET_CZN.
	INC, DEC and ADD+ADC set cc0 to CC_CLOBBER.

gcc/testsuite/
	PR target/61055
	* gcc.target/avr/torture/pr61055.c: New test.

Comments

Denis Chertykov May 9, 2014, 10:47 a.m. UTC | #1 
2014-05-08 17:04 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> Some instructions like INC, DEC, NEG currently set cc0 to set_zn which is
> not the whole story because they also set the V flag.  This leads to a wrong
> branch instruction in the remainder as cc0 is used.  Fix is to do same as
> clobber cc0.  For the matter of clarity, I added a new cc0 alternative
> set_vzn for that case.
>
> Moreover, ADIW sets cc0 to set_czn rather than set_zn.  This is the same as
> the action of a single ADD and like ADIW was modeled the old days (before
> avr_out_plus_1 was introduced to print the output).
>
> No new regressions.
>
> Ok to apply?
>
> Johann
>
>
> gcc/config/avr
>         PR target/61055
>         * config/avr/avr.md (cc): Add new attribute set_vzn.
>         (addqi3, addqq3, adduqq3, subqi3, subqq3, subuqq3, negqi2) [cc]:
>         Set cc insn attribute to set_vzn instead of set_zn for alternatives
>         with INC, DEC or NEG.
>         * config/avr/avr.c (avr_notice_update_cc): Handle SET_VZN.
>         (avr_out_plus_1): ADIW sets cc0 to CC_SET_CZN.
>         INC, DEC and ADD+ADC set cc0 to CC_CLOBBER.
>
> gcc/testsuite/
>         PR target/61055
>         * gcc.target/avr/torture/pr61055.c: New test.

Please apply.

Denis.
diff mbox

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 210209)
+++ config/avr/avr.c	(working copy)
@@ -2359,6 +2359,12 @@  avr_notice_update_cc (rtx body ATTRIBUTE
         }
       break;
 
+    case CC_SET_VZN:
+      /* Insn like INC, DEC, NEG that set Z,N,V.  We currently don't make use
+         of this combination, cf. also PR61055.  */
+      CC_STATUS_INIT;
+      break;
+
     case CC_SET_CZN:
       /* Insn sets the Z,N,C flags of CC to recog_operand[0].
          The V flag may or may not be known but that's ok because
@@ -6290,7 +6296,7 @@  avr_out_plus_1 (rtx *xop, int *plen, enu
 
   if (REG_P (xop[2]))
     {
-      *pcc = MINUS == code ? (int) CC_SET_CZN : (int) CC_SET_N;
+      *pcc = MINUS == code ? (int) CC_SET_CZN : (int) CC_CLOBBER;
 
       for (i = 0; i < n_bytes; i++)
         {
@@ -6399,7 +6405,7 @@  avr_out_plus_1 (rtx *xop, int *plen, enu
                                op, plen, 1);
 
                   if (n_bytes == 2 && PLUS == code)
-                    *pcc = CC_SET_ZN;
+                    *pcc = CC_SET_CZN;
                 }
 
               i++;
@@ -6422,6 +6428,7 @@  avr_out_plus_1 (rtx *xop, int *plen, enu
         {
           avr_asm_len ((code == PLUS) ^ (val8 == 1) ? "dec %0" : "inc %0",
                        op, plen, 1);
+          *pcc = CC_CLOBBER;
           break;
         }
 
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 210209)
+++ config/avr/avr.md	(working copy)
@@ -90,7 +90,7 @@  (define_c_enum "unspecv"
 (include "constraints.md")
 
 ;; Condition code settings.
-(define_attr "cc" "none,set_czn,set_zn,set_n,compare,clobber,
+(define_attr "cc" "none,set_czn,set_zn,set_vzn,set_n,compare,clobber,
                    plus,ldi"
   (const_string "none"))
 
@@ -1098,7 +1098,7 @@  (define_insn "add<mode>3"
 	inc %0\;inc %0
 	dec %0\;dec %0"
   [(set_attr "length" "1,1,1,1,2,2")
-   (set_attr "cc" "set_czn,set_czn,set_zn,set_zn,set_zn,set_zn")])
+   (set_attr "cc" "set_czn,set_czn,set_vzn,set_vzn,set_vzn,set_vzn")])
 
 ;; "addhi3"
 ;; "addhq3" "adduhq3"
@@ -1369,7 +1369,7 @@  (define_insn "sub<mode>3"
 	dec %0\;dec %0
 	inc %0\;inc %0"
   [(set_attr "length" "1,1,1,1,2,2")
-   (set_attr "cc" "set_czn,set_czn,set_zn,set_zn,set_zn,set_zn")])
+   (set_attr "cc" "set_czn,set_czn,set_vzn,set_vzn,set_vzn,set_vzn")])
 
 ;; "subhi3"
 ;; "subhq3" "subuhq3"
@@ -3992,7 +3992,7 @@  (define_insn "negqi2"
   ""
   "neg %0"
   [(set_attr "length" "1")
-   (set_attr "cc" "set_zn")])
+   (set_attr "cc" "set_vzn")])
 
 (define_insn "*negqihi2"
   [(set (match_operand:HI 0 "register_operand"                        "=r")
Index: testsuite/gcc.target/avr/torture/pr61055.c
===================================================================
--- testsuite/gcc.target/avr/torture/pr61055.c	(revision 0)
+++ testsuite/gcc.target/avr/torture/pr61055.c	(revision 0)
@@ -0,0 +1,88 @@ 
+/* { dg-do run } */
+/* { dg-options { -fno-peephole2 } } */
+
+#include <stdlib.h>
+
+typedef __UINT16_TYPE__ uint16_t;
+typedef __INT16_TYPE__  int16_t;
+typedef __UINT8_TYPE__  uint8_t;
+
+uint8_t __attribute__((noinline,noclone))
+fun_inc (uint8_t c0)
+{
+  register uint8_t c asm ("r15") = c0;
+
+  /* Force target value into R15 (lower register)  */
+  asm ("" : "+l" (c));
+
+  c++;
+  if (c >= 0x80)
+    c = 0;
+  
+  asm ("" : "+l" (c));
+
+  return c;
+}
+
+uint8_t __attribute__((noinline,noclone))
+fun_dec (uint8_t c0)
+{
+  register uint8_t c asm ("r15") = c0;
+
+  /* Force target value into R15 (lower register)  */
+  asm ("" : "+l" (c));
+
+  c--;
+  if (c < 0x80)
+    c = 0;
+  
+  asm ("" : "+l" (c));
+
+  return c;
+}
+
+
+uint8_t __attribute__((noinline,noclone))
+fun_neg (uint8_t c0)
+{
+  register uint8_t c asm ("r15") = c0;
+
+  c = -c;
+  if (c >= 0x80)
+    c = 0;
+
+  return c;
+}
+
+uint16_t __attribute__((noinline,noclone))
+fun_adiw (uint16_t c0)
+{
+  register uint16_t c asm ("r24") = c0;
+
+  /* Force target value into R24 (for ADIW) */
+  asm ("" : "+r" (c));
+
+  c += 2;
+  if (c >= 0x8000)
+    c = 0;
+
+  asm ("" : "+r" (c));
+  
+  return c;
+}
+
+
+int main()
+{
+  if (fun_inc (0x7f) != 0)
+    abort();
+  
+  if (fun_neg (0x80) != 0)
+    abort();
+  
+  if (fun_adiw (0x7ffe) != 0)
+    abort();
+
+  exit (0);
+  return 0;
+}