[PATCH][fr30] Fix PR 34174 movdi mem->reg wrong-code bug
Rask Ingemann Lambertsen
rask@sygehus.dk
Fri Nov 23 22:16:00 GMT 2007
The bug is in fr30_move_double()'s mem->reg case. This code is amazing.
It detects when it is necessary to avoid clobbering the register holding the
memory address, then starts out by clobbering it anyway. And the
reverse/!reverse trickery is applied twice and therefore cancels itself out.
It has been broken this way ever since it was added seven and a half years
ago. The broken instruction sequence looks like this
ld @rN, r1
mov rN, r2
addn 4, r2
ld @r2, r2
where rN holds the address. If N == 1, it breaks. This patch changes it to
mov rN, r2
ld @rN, r1
addn 4, r2
ld @r2, r2
which fixes the bug. I've tested it by building a cross compiler for
fr30-unknown-elf and running the testsuite using the simulator from GDB 5.2.
There were no new failures. Ok for trunk?
Note that I expect the testcase to fail on the AVR because of PR
target/27386 where this testcase originates. Should I XFAIL it on the AVR?
Mark, would it be OK to fix this for 4.2.3 also? While clearly not a
regression, it is a wrong-code bug which is easy to trigger and causes
374 testsuite failures, all execution failures:
=== gcc Summary ===
-# of expected passes 43562
-# of unexpected failures 509
+# of expected passes 43901
+# of unexpected failures 182
# of unexpected successes 3
# of expected failures 84
# of unresolved testcases 71
=== g++ Summary ===
-# of expected passes 16296
-# of unexpected failures 118
+# of expected passes 16343
+# of unexpected failures 71
# of unexpected successes 2
# of expected failures 81
# of unresolved testcases 51
:ADDPATCH fr30:
23-11-2007 Rask Ingemann Lambertsen <rask@sygehus.dk>
PR target/34174
* config/fr30/fr30.c (fr30_move_double): Sanitize mem->reg case. Copy
the address before it is clobbered.
testsuite/
* gcc.dg/torture/pr34174-1.c: New.
Index: gcc/config/fr30/fr30.c
===================================================================
--- gcc/config/fr30/fr30.c (revision 130319)
+++ gcc/config/fr30/fr30.c (working copy)
@@ -828,47 +828,23 @@ fr30_move_double (rtx * operands)
{
rtx addr = XEXP (src, 0);
int dregno = REGNO (dest);
- rtx dest0;
- rtx dest1;
+ rtx dest0 = operand_subword (dest, 0, TRUE, mode);;
+ rtx dest1 = operand_subword (dest, 1, TRUE, mode);;
rtx new_mem;
- /* If the high-address word is used in the address, we
- must load it last. Otherwise, load it first. */
- int reverse = (refers_to_regno_p (dregno, dregno + 1, addr, 0) != 0);
-
gcc_assert (GET_CODE (addr) == REG);
- dest0 = operand_subword (dest, reverse, TRUE, mode);
- dest1 = operand_subword (dest, !reverse, TRUE, mode);
-
- if (reverse)
- {
- emit_insn (gen_rtx_SET (VOIDmode, dest1,
- adjust_address (src, SImode, 0)));
- emit_insn (gen_rtx_SET (SImode, dest0,
- gen_rtx_REG (SImode, REGNO (addr))));
- emit_insn (gen_rtx_SET (SImode, dest0,
- plus_constant (dest0, UNITS_PER_WORD)));
-
- new_mem = gen_rtx_MEM (SImode, dest0);
- MEM_COPY_ATTRIBUTES (new_mem, src);
-
- emit_insn (gen_rtx_SET (VOIDmode, dest0, new_mem));
- }
- else
- {
- emit_insn (gen_rtx_SET (VOIDmode, dest0,
- adjust_address (src, SImode, 0)));
- emit_insn (gen_rtx_SET (SImode, dest1,
- gen_rtx_REG (SImode, REGNO (addr))));
- emit_insn (gen_rtx_SET (SImode, dest1,
- plus_constant (dest1, UNITS_PER_WORD)));
+ /* Copy the address before clobbering it. See PR 34174. */
+ emit_insn (gen_rtx_SET (SImode, dest1, addr));
+ emit_insn (gen_rtx_SET (VOIDmode, dest0,
+ adjust_address (src, SImode, 0)));
+ emit_insn (gen_rtx_SET (SImode, dest1,
+ plus_constant (dest1, UNITS_PER_WORD)));
- new_mem = gen_rtx_MEM (SImode, dest1);
- MEM_COPY_ATTRIBUTES (new_mem, src);
+ new_mem = gen_rtx_MEM (SImode, dest1);
+ MEM_COPY_ATTRIBUTES (new_mem, src);
- emit_insn (gen_rtx_SET (VOIDmode, dest1, new_mem));
- }
+ emit_insn (gen_rtx_SET (VOIDmode, dest1, new_mem));
}
else if (src_code == CONST_INT || src_code == CONST_DOUBLE)
{
Index: gcc/testsuite/gcc.dg/torture/pr34174-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr34174-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr34174-1.c (revision 0)
@@ -0,0 +1,44 @@
+/* { dg-do run } */
+/* Based on PR target/27386 testcase by Joerg Wunsch. */
+
+extern void abort (void);
+extern void exit (int);
+
+#if __INT_MAX__ >= 9223372036854775807LL
+typedef unsigned int uint64_t;
+#elif __LONG_MAX__ >= 9223372036854775807LL
+typedef unsigned long int uint64_t;
+#elif __LONG_LONG_MAX__ >= 9223372036854775807LL
+typedef unsigned long long int uint64_t;
+#else
+int
+main (void)
+{
+ exit (0);
+}
+#endif
+
+uint64_t a, b, c;
+
+int
+foo (uint64_t x, uint64_t y, uint64_t z, int i)
+{
+ a = x;
+ b = y;
+ c = z;
+ return 2 * i;
+}
+
+int
+main (void)
+{
+ if (foo (1234512345123ull, 3456734567345ull, 7897897897897ull, 42) != 84)
+ abort ();
+ if (a != 1234512345123ull)
+ abort ();
+ if (b != 3456734567345ull)
+ abort ();
+ if (c != 7897897897897ull)
+ abort ();
+ exit (0);
+}
--
Rask Ingemann Lambertsen
Danish law requires addresses in e-mail to be logged and stored for a year
More information about the Gcc-patches
mailing list