Am 04/16/2015 um 08:43 AM schrieb Senthil Kumar Selvaraj:
This patch fixes PR 65657.
The following artifact appears to be PR63633.
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.selva...@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.selva...@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;
IMO costs should not be used to fix wrong code or ICEs. Presumably the fix is
similar to the one used by PR63633.
Johann
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);
+}
