* [patch,avr] Slightly better memory accesses on avr_tiny
@ 2016-07-19 10:31 Georg-Johann Lay
2016-07-19 16:53 ` Denis Chertykov
0 siblings, 1 reply; 2+ messages in thread
From: Georg-Johann Lay @ 2016-07-19 10:31 UTC (permalink / raw)
To: gcc-patches
Cc: Denis Chertykov, Selvaraj, Senthil_Kumar, Senthil Kumar Selvaraj
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
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.
[-- Attachment #2: tiny-mem.diff --]
[-- Type: text/x-patch, Size: 12524 bytes --]
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;
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [patch,avr] Slightly better memory accesses on avr_tiny
2016-07-19 10:31 [patch,avr] Slightly better memory accesses on avr_tiny Georg-Johann Lay
@ 2016-07-19 16:53 ` Denis Chertykov
0 siblings, 0 replies; 2+ messages in thread
From: Denis Chertykov @ 2016-07-19 16:53 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches, Selvaraj, Senthil_Kumar
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-07-19 16:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-19 10:31 [patch,avr] Slightly better memory accesses on avr_tiny Georg-Johann Lay
2016-07-19 16:53 ` Denis Chertykov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).