diff mbox

[avr] Slightly better memory accesses on avr_tiny

Message ID 5d4553e1-571d-e469-e33d-a73631350f80@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay July 19, 2016, 10:31 a.m. UTC 
This patch tries to improve the bloated code we are currently generating for 
AVR_TINY.  It's mostly about printing the memory loads and stores and more 
usage of reg_unused_after to print shorter instruction sequences in some cases.

Ok for trunk?

I also played around with PLUS in legitimate_address_p and legitimize_address 
and got better code, but the problem with such changes is that almost all tests 
for such small devices are failing and no reasonable portion of the testsuite 
will pass.

I don't even know if anybody is using avr_tiny + avr-gcc or if users are 
resorting to assembler.

Johann


gcc/
	(avr_legitimize_address) [AVR_TINY]: Force constant addresses
	outside [0,0xc0] into a register.
	(avr_out_movhi_r_mr_reg_no_disp_tiny): Pass insn.  And handle
	cases where the base address register is unused after.
	(avr_out_movhi_r_mr_reg_disp_tiny): Same.
	(avr_out_movhi_mr_r_reg_disp_tiny): Same.
	(avr_out_store_psi_reg_disp_tiny): Same.

gcc/testsuite/
	* gcc.target/avr/torture/get-mem.c: New test.
	* gcc.target/avr/torture/set-mem.c: New test.

Comments

Denis Chertykov July 19, 2016, 4:53 p.m. UTC | #1 
2016-07-19 13:31 GMT+03:00 Georg-Johann Lay <avr@gjlay.de>:
> This patch tries to improve the bloated code we are currently generating for
> AVR_TINY.  It's mostly about printing the memory loads and stores and more
> usage of reg_unused_after to print shorter instruction sequences in some
> cases.
>
> Ok for trunk?
>
> I also played around with PLUS in legitimate_address_p and
> legitimize_address and got better code, but the problem with such changes is
> that almost all tests for such small devices are failing and no reasonable
> portion of the testsuite will pass.
>
> I don't even know if anybody is using avr_tiny + avr-gcc or if users are
> resorting to assembler.
>

Keep Calm and Carry On
;-)

> Johann
>
>
> gcc/
>         (avr_legitimize_address) [AVR_TINY]: Force constant addresses
>         outside [0,0xc0] into a register.
>         (avr_out_movhi_r_mr_reg_no_disp_tiny): Pass insn.  And handle
>         cases where the base address register is unused after.
>         (avr_out_movhi_r_mr_reg_disp_tiny): Same.
>         (avr_out_movhi_mr_r_reg_disp_tiny): Same.
>         (avr_out_store_psi_reg_disp_tiny): Same.
>
> gcc/testsuite/
>         * gcc.target/avr/torture/get-mem.c: New test.
>         * gcc.target/avr/torture/set-mem.c: New test.

Approved.
diff mbox

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 238425)
+++ config/avr/avr.c	(working copy)
@@ -1922,6 +1922,16 @@  avr_legitimize_address (rtx x, rtx oldx,
 
   x = oldx;
 
+  if (AVR_TINY)
+    {
+      if (CONSTANT_ADDRESS_P (x)
+          && !(CONST_INT_P (x)
+               && IN_RANGE (INTVAL (x), 0, 0xc0 - GET_MODE_SIZE (mode))))
+        {
+          x = force_reg (Pmode, x);
+        }
+    }
+
   if (GET_CODE (oldx) == PLUS
       && REG_P (XEXP (oldx, 0)))
     {
@@ -3510,7 +3520,7 @@  out_movqi_r_mr (rtx_insn *insn, rtx op[]
 /* Same as movhi_r_mr, but TINY does not have ADIW, SBIW and LDD */
 
 static const char*
-avr_out_movhi_r_mr_reg_no_disp_tiny (rtx op[], int *plen)
+avr_out_movhi_r_mr_reg_no_disp_tiny (rtx_insn *insn, rtx op[], int *plen)
 {
   rtx dest = op[0];
   rtx src = op[1];
@@ -3524,17 +3534,20 @@  avr_out_movhi_r_mr_reg_no_disp_tiny (rtx
 			"ld %B0,%1"          CR_TAB
 			"mov %A0,__tmp_reg__", op, plen, -3);
 
-  return avr_asm_len ("ld %A0,%1"             CR_TAB
-                      TINY_ADIW (%E1, %F1, 1) CR_TAB
-                      "ld %B0,%1"             CR_TAB
-                      TINY_SBIW (%E1, %F1, 1), op, plen, -6);
+  avr_asm_len ("ld %A0,%1+"                  CR_TAB
+               "ld %B0,%1", op, plen, -2);
+
+  if (!reg_unused_after (insn, base))
+    avr_asm_len (TINY_SBIW (%E1, %F1, 1), op, plen, 2);
+
+  return "";
 }
 
 
 /* Same as movhi_r_mr, but TINY does not have ADIW, SBIW and LDD */
 
 static const char*
-avr_out_movhi_r_mr_reg_disp_tiny (rtx op[], int *plen)
+avr_out_movhi_r_mr_reg_disp_tiny (rtx_insn *insn, rtx op[], int *plen)
 {
   rtx dest = op[0];
   rtx src = op[1];
@@ -3552,10 +3565,14 @@  avr_out_movhi_r_mr_reg_disp_tiny (rtx op
     }
   else
     {
-      return avr_asm_len (TINY_ADIW (%I1, %J1, %o1) CR_TAB
-                          "ld %A0,%b1+"             CR_TAB
-                          "ld %B0,%b1"              CR_TAB
-                          TINY_SBIW (%I1, %J1, %o1+1), op, plen, -6);
+      avr_asm_len (TINY_ADIW (%I1, %J1, %o1) CR_TAB
+                   "ld %A0,%b1+"             CR_TAB
+                   "ld %B0,%b1", op, plen, -4);
+
+      if (!reg_unused_after (insn, XEXP (base, 0)))
+        avr_asm_len (TINY_SBIW (%I1, %J1, %o1+1), op, plen, 2);
+
+      return "";
     }
 }
 
@@ -3603,7 +3620,7 @@  out_movhi_r_mr (rtx_insn *insn, rtx op[]
   if (reg_base > 0)
     {
       if (AVR_TINY)
-        return avr_out_movhi_r_mr_reg_no_disp_tiny (op, plen);
+        return avr_out_movhi_r_mr_reg_no_disp_tiny (insn, op, plen);
 
       if (reg_dest == reg_base)         /* R = (R) */
         return avr_asm_len ("ld __tmp_reg__,%1+" CR_TAB
@@ -3628,7 +3645,7 @@  out_movhi_r_mr (rtx_insn *insn, rtx op[]
       int reg_base = true_regnum (XEXP (base, 0));
 
       if (AVR_TINY)
-        return avr_out_movhi_r_mr_reg_disp_tiny (op, plen);
+        return avr_out_movhi_r_mr_reg_disp_tiny (insn, op, plen);
 
       if (disp > MAX_LD_OFFSET (GET_MODE (src)))
         {
@@ -4377,8 +4394,8 @@  avr_out_load_psi_reg_no_disp_tiny (rtx_i
 		   "ld %B0,%1+"  CR_TAB
 		   "ld %C0,%1", op, plen, -3);
 
-      if (reg_dest != reg_base - 2 &&
-          !reg_unused_after (insn, base))
+      if (reg_dest != reg_base - 2
+          && !reg_unused_after (insn, base))
         {
           avr_asm_len (TINY_SBIW (%E1, %F1, 2), op, plen, 2);
         }
@@ -4408,13 +4425,13 @@  avr_out_load_psi_reg_disp_tiny (rtx_insn
   else
     {
       avr_asm_len (TINY_ADIW (%I1, %J1, %o1)   CR_TAB
-                          "ld %A0,%b1+"              CR_TAB
-                          "ld %B0,%b1+"              CR_TAB
-                          "ld %C0,%b1", op, plen, -5);
+                   "ld %A0,%b1+"               CR_TAB
+                   "ld %B0,%b1+"               CR_TAB
+                   "ld %C0,%b1", op, plen, -5);
 
-      if (reg_dest != (reg_base - 2)
+      if (reg_dest != reg_base - 2
           && !reg_unused_after (insn, XEXP (base, 0)))
-          avr_asm_len (TINY_SBIW (%I1, %J1, %o1+2), op, plen, 2);
+        avr_asm_len (TINY_SBIW (%I1, %J1, %o1+2), op, plen, 2);
 
       return "";
     }
@@ -4599,7 +4616,7 @@  avr_out_store_psi_reg_no_disp_tiny (rtx_
 }
 
 static const char*
-avr_out_store_psi_reg_disp_tiny (rtx *op, int *plen)
+avr_out_store_psi_reg_disp_tiny (rtx_insn *insn, rtx *op, int *plen)
 {
   rtx dest = op[0];
   rtx src = op[1];
@@ -4608,31 +4625,29 @@  avr_out_store_psi_reg_disp_tiny (rtx *op
   int reg_src = true_regnum (src);
 
   if (reg_src == reg_base)
-    {
-      return avr_asm_len ("mov __tmp_reg__,%A1"          CR_TAB
-                          "mov __zero_reg__,%B1"         CR_TAB
-                          TINY_ADIW (%I0, %J0, %o0)      CR_TAB
-                          "st %b0+,__tmp_reg__"          CR_TAB
-                          "st %b0+,__zero_reg__"         CR_TAB
-                          "st %b0,%C1"                   CR_TAB
-                          "clr __zero_reg__"             CR_TAB
-                          TINY_SBIW (%I0, %J0, %o0+2), op, plen, -10);
-    }
+    avr_asm_len ("mov __tmp_reg__,%A1"          CR_TAB
+                 "mov __zero_reg__,%B1"         CR_TAB
+                 TINY_ADIW (%I0, %J0, %o0)      CR_TAB
+                 "st %b0+,__tmp_reg__"          CR_TAB
+                 "st %b0+,__zero_reg__"         CR_TAB
+                 "st %b0,%C1"                   CR_TAB
+                 "clr __zero_reg__", op, plen, -8);
   else if (reg_src == reg_base - 2)
-    {
-      return avr_asm_len ("mov __tmp_reg__,%C1"          CR_TAB
-                          TINY_ADIW (%I0, %J0, %o0)      CR_TAB
-                          "st %b0+,%A1"                  CR_TAB
-                          "st %b0+,%B1"                  CR_TAB
-                          "st %b0,__tmp_reg__"           CR_TAB
-                          TINY_SBIW (%I0, %J0, %o0+2), op, plen, -8);
-    }
+    avr_asm_len ("mov __tmp_reg__,%C1"          CR_TAB
+                 TINY_ADIW (%I0, %J0, %o0)      CR_TAB
+                 "st %b0+,%A1"                  CR_TAB
+                 "st %b0+,%B1"                  CR_TAB
+                 "st %b0,__tmp_reg__", op, plen, -6);
+  else
+    avr_asm_len (TINY_ADIW (%I0, %J0, %o0)      CR_TAB
+                 "st %b0+,%A1"                  CR_TAB
+                 "st %b0+,%B1"                  CR_TAB
+                 "st %b0,%C1", op, plen, -5);
+
+  if (!reg_unused_after (insn, XEXP (base, 0)))
+    avr_asm_len (TINY_SBIW (%I0, %J0, %o0+2), op, plen, 2);
 
-  return avr_asm_len (TINY_ADIW (%I0, %J0, %o0)      CR_TAB
-                          "st %b0+,%A1"                  CR_TAB
-                          "st %b0+,%B1"                  CR_TAB
-                          "st %b0,%C1"                   CR_TAB
-                          TINY_SBIW (%I0, %J0, %o0+2), op, plen, -7);
+  return "";
 }
 
 /* Handle store of 24-bit type from register or zero to memory.  */
@@ -4681,7 +4696,7 @@  avr_out_store_psi (rtx_insn *insn, rtx *
       int disp = INTVAL (XEXP (base, 1));
 
       if (AVR_TINY)
-        return avr_out_store_psi_reg_disp_tiny (op, plen);
+        return avr_out_store_psi_reg_disp_tiny (insn, op, plen);
 
       reg_base = REGNO (XEXP (base, 0));
 
@@ -4815,7 +4830,7 @@  avr_out_movqi_mr_r_reg_disp_tiny (rtx_in
     else
     {
       avr_asm_len (TINY_ADIW (%I0, %J0, %o0) CR_TAB
-          "st %b0,%1" , op, plen, -3);
+                   "st %b0,%1", op, plen, -3);
     }
 
   if (!reg_unused_after (insn, XEXP (x,0)))
@@ -5039,7 +5054,7 @@  avr_out_movhi_mr_r_reg_no_disp_tiny (rtx
 }
 
 static const char*
-avr_out_movhi_mr_r_reg_disp_tiny (rtx op[], int *plen)
+avr_out_movhi_mr_r_reg_disp_tiny (rtx_insn *insn, rtx op[], int *plen)
 {
   rtx dest = op[0];
   rtx src = op[1];
@@ -5047,19 +5062,22 @@  avr_out_movhi_mr_r_reg_disp_tiny (rtx op
   int reg_base = REGNO (XEXP (base, 0));
   int reg_src = true_regnum (src);
 
-  return reg_src == reg_base
-        ? avr_asm_len ("mov __tmp_reg__,%A1"          CR_TAB
-                       "mov __zero_reg__,%B1"         CR_TAB
-                       TINY_ADIW (%I0, %J0, %o0+1)    CR_TAB
-                       "st %b0,__zero_reg__"          CR_TAB
-                       "st -%b0,__tmp_reg__"          CR_TAB
-                       "clr __zero_reg__"             CR_TAB
-                       TINY_SBIW (%I0, %J0, %o0), op, plen, -9)
-
-        : avr_asm_len (TINY_ADIW (%I0, %J0, %o0+1) CR_TAB
-                       "st %b0,%B1"                CR_TAB
-                       "st -%b0,%A1"               CR_TAB
-                       TINY_SBIW (%I0, %J0, %o0), op, plen, -6);
+  if (reg_src == reg_base)
+    avr_asm_len ("mov __tmp_reg__,%A1"          CR_TAB
+                 "mov __zero_reg__,%B1"         CR_TAB
+                 TINY_ADIW (%I0, %J0, %o0+1)    CR_TAB
+                 "st %b0,__zero_reg__"          CR_TAB
+                 "st -%b0,__tmp_reg__"          CR_TAB
+                 "clr __zero_reg__", op, plen, -7);
+  else
+    avr_asm_len (TINY_ADIW (%I0, %J0, %o0+1) CR_TAB
+                 "st %b0,%B1"                CR_TAB
+                 "st -%b0,%A1", op, plen, -4);
+
+  if (!reg_unused_after (insn, XEXP (base, 0)))
+    avr_asm_len (TINY_SBIW (%I0, %J0, %o0), op, plen, 2);
+
+  return "";
 }
 
 static const char*
@@ -5136,7 +5154,7 @@  out_movhi_mr_r (rtx_insn *insn, rtx op[]
       int disp = INTVAL (XEXP (base, 1));
 
       if (AVR_TINY)
-        return avr_out_movhi_mr_r_reg_disp_tiny (op, plen);
+        return avr_out_movhi_mr_r_reg_disp_tiny (insn, op, plen);
 
       reg_base = REGNO (XEXP (base, 0));
       if (disp > MAX_LD_OFFSET (GET_MODE (dest)))
Index: testsuite/gcc.target/avr/torture/get-mem.c
===================================================================
--- testsuite/gcc.target/avr/torture/get-mem.c	(nonexistent)
+++ testsuite/gcc.target/avr/torture/get-mem.c	(working copy)
@@ -0,0 +1,52 @@ 
+/* { dg-do run } */
+
+#define NI __attribute__((noinline, noclone))
+
+typedef __INT8_TYPE__ s8;
+typedef __INT16_TYPE__ s16;
+typedef __int24 s24;
+typedef __INT32_TYPE__ s32;
+
+static const s8 arr8[] = { 12, 23, 34 };
+static const s16 arr16[] = { 123, 234, 345 };
+static const s24 arr24[] = { 1234, 2345, 3456 };
+static const s32 arr32[] = { 12345, 23456, 34567 };
+
+NI s8  add8  (const s8  *p) { return p[0] + p[1] + p[2]; }
+NI s16 add16 (const s16 *p) { return p[0] + p[1] + p[2]; }
+NI s24 add24 (const s24 *p) { return p[0] + p[1] + p[2]; }
+NI s32 add32 (const s32 *p) { return p[0] + p[1] + p[2]; }
+
+void test8 (void)
+{
+  if (add8 (arr8) != arr8[0] + arr8[1] + arr8[2])
+    __builtin_abort();
+}
+
+void test16 (void)
+{
+  if (add16 (arr16) != arr16[0] + arr16[1] + arr16[2])
+    __builtin_abort();
+}
+
+void test24 (void)
+{
+  if (add24 (arr24) != arr24[0] + arr24[1] + arr24[2])
+    __builtin_abort();
+}
+
+void test32 (void)
+{
+  if (add32 (arr32) != arr32[0] + arr32[1] + arr32[2])
+    __builtin_abort();
+}
+
+int main (void)
+{
+  test8();
+  test16();
+  test24();
+  test32();
+
+  return 0;
+}
Index: testsuite/gcc.target/avr/torture/set-mem.c
===================================================================
--- testsuite/gcc.target/avr/torture/set-mem.c	(nonexistent)
+++ testsuite/gcc.target/avr/torture/set-mem.c	(working copy)
@@ -0,0 +1,55 @@ 
+/* { dg-do run } */
+
+#define NI __attribute__((noinline, noclone))
+
+typedef __INT8_TYPE__ s8;
+typedef __INT16_TYPE__ s16;
+typedef __int24 s24;
+typedef __INT32_TYPE__ s32;
+
+static s8 arr8[3];
+static s16 arr16[3];
+static s24 arr24[3];
+static s32 arr32[3];
+
+NI void set8  (s8  *p) { p[0] = -123; p[1]  = -23; p[2]  = -34; }
+NI void set16 (s16 *p) { p[0] = -123; p[1] = -234; p[2] = -345; }
+NI void set24 (s24 *p) { p[0] = -123; p[1] = -234; p[2] = -345; }
+NI void set32 (s32 *p) { p[0] = -123; p[1] = -234; p[2] = -345; }
+
+void test8 (void)
+{
+  set8 (arr8);
+  if (arr8[0] != -123 || arr8[1] != -23 || arr8[2] != -34)
+    __builtin_abort();
+}
+
+void test16 (void)
+{
+  set16 (arr16);
+  if (arr16[0] != -123 || arr16[1] != -234 || arr16[2] != -345)
+    __builtin_abort();
+}
+
+void test24 (void)
+{
+  set24 (arr24);
+  if (arr24[0] != -123 || arr24[1] != -234 || arr24[2] != -345)
+    __builtin_abort();
+}
+
+void test32 (void)
+{
+  set32 (arr32);
+  if (arr32[0] != -123 || arr32[1] != -234 || arr32[2] != -345)
+    __builtin_abort();
+}
+
+int main (void)
+{
+  test8();
+  test16();
+  test24();
+  test32();
+  return 0;
+}