This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch,avr] Fix PR 65657 - read from __memx address space tramples arguments to function call


This patch fixes PR 65657.

When cfgexpand.c expands a function call, it first figures out the
registers in which the arguments will go, followed by expansion of the
arguments themselves (right to left). It later emits mov insns to set 
the precomputed registers with the expanded RTXes.

If one of the arguments is a __memx char pointer dereference, the mov
eventually expands to gen_xload<mode>_A (for certain cases), which 
clobbers R22, R21 and Z.  This causes problems if one of those 
clobbered registers was precomputed to hold another argument.

In general, call expansion does not appear to take clobbers into account - 
when it checks for argument overlap, the RTX (args[i].value) is only a MEM 
in QImode for the memx deref - the clobber shows up when it eventually 
calls emit_move_insn, at which point, it is too late.

This does not happen for a int pointer dereference - turns out that 
precompute_register_parameters does a copy_to_mode_reg if the
cost of args[i].value is more than COSTS_N_INSNS(1) i.e., it creates a
pseudo and later assigns the pseudo to the arg register. This is done
before any moves to arg registers is done, so other arguments are not
overwritten.

Doing the same thing - providing a better cost estimate for a MEM rtx in
the non-default address space, makes this problem go away, and that is
what this patch does. Regression testing does not show any new failures.

The fix does seem tangential to the core problem - not sure if there is
a better way to fix this?

If not, could someone commit this please? I don't have commit access.

Regards
Senthil

gcc/ChangeLog

2015-04-16  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* config/avr/avr.c (avr_rtx_costs_1): Increase cost for
	MEM rtx in non-default address space.

gcc/testsuite/ChangeLog

2015-04-16  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	* gcc.target/avr/pr65657.c: New.

diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index 68d5ddc..c9b8678 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -9959,7 +9959,11 @@ avr_rtx_costs_1 (rtx x, int codearg, int outer_code ATTRIBUTE_UNUSED,
       return true;
 
     case MEM:
-      *total = COSTS_N_INSNS (GET_MODE_SIZE (mode));
+      /* MEM rtx with non-default address space is more
+         expensive. Not expressing that results in reg
+         clobber during expand (PR 65657). */
+      *total = COSTS_N_INSNS (GET_MODE_SIZE (mode)
+                  + (MEM_ADDR_SPACE(x) == ADDR_SPACE_RAM ? 0 : 5));
       return true;
 
     case NEG:
diff --git gcc/testsuite/gcc.target/avr/pr65657.c gcc/testsuite/gcc.target/avr/pr65657.c
new file mode 100644
index 0000000..d908fe3
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr65657.c
@@ -0,0 +1,39 @@
+/* { dg-do run } */
+/* { dg-options "-Os" } */
+
+/* When cfgexpand expands the call to foo, it
+   assigns registers R22:R23 to 0xABCD and R24
+   to *x. Because x is a __memx address space
+   pointer, the dereference results in an RTL
+   insn that clobbers R22 among other 
+   registers (xload<mode>_A).
+
+   Call expansion does not take this into account
+   and ends up clobbering R22 set previously to 
+   hold part of the second argument.
+*/
+
+#include <stdlib.h>
+
+void __attribute__((noinline))
+    __attribute__((noclone))
+foo (char c, volatile unsigned int d)
+{
+    if (d != 0xABCD)
+      abort();
+    if (c != 'A')
+        abort();
+}
+
+void __attribute__((noinline))
+    __attribute__((noclone))
+readx(const char __memx *x)
+{
+    foo (*x, 0xABCD);
+}
+
+const char __memx arr[] = { 'A' };
+int main()
+{
+   readx (arr); 
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]