This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Fix E500 wrong-code bug
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Thu, 27 Sep 2007 12:12:41 +0000 (UTC)
- Subject: Fix E500 wrong-code bug
This patch (an rs6000 back-end patch plus a middle-end assertion to help
detect such bugs in future) fixes a wrong-code bug for E500. (The
original bug was observed as a libstdc++ test failure for a 4.2-based
toolchain; that failure no longer appears with trunk, but as far as I can
tell the underlying issue is still present.)
The problem was that
(insn 213 212 214 9 (set (mem/s/c:V2SI (plus:SI (reg/f:SI 115 virtual-stack-vars)
(const_int 33592 [0x8338])) [0 S8 A128])
(reg:V2SI 225)) -1 (nil)
(nil))
in the .unshare dump turns into
(insn 213 354 214 9 (set (reg:V2SI 280)
(reg:V2SI 225)) 1026 {*movv2si_internal} (nil)
(nil))
in the .vregs dump (and reg 280 isn't used after being set there). This
is storing a structure value, whose address is passed as a parameter to
the "copy" function, on the stack; the address still gets passed, but the
structure hasn't been initialized, but with the more-reduced testcase the
structure does get initialized and so the test passes.
The problem transformation above is done by
instantiate_virtual_regs_in_insn. We have a loop
/* In the general case, we expect virtual registers to appear only in
operands, and then only as either bare registers or inside memories. */
for (i = 0; i < recog_data.n_operands; ++i)
{
x = recog_data.operand[i];
switch (GET_CODE (x))
{
case MEM:
{
rtx addr = XEXP (x, 0);
bool changed = false;
for_each_rtx (&addr, instantiate_virtual_regs_in_rtx, &changed);
if (!changed)
continue;
start_sequence ();
x = replace_equiv_address (x, addr);
seq = get_insns ();
end_sequence ();
if (seq)
emit_insn_before (seq, insn);
}
break;
[...]
/* At this point, X contains the new value for the operand.
Validate the new value vs the insn predicate. Note that
asm insns will have insn_code -1 here. */
if (!safe_insn_predicate (insn_code, i, x))
{
start_sequence ();
x = force_reg (insn_data[insn_code].operand[i].mode, x);
seq = get_insns ();
end_sequence ();
if (seq)
emit_insn_before (seq, insn);
}
and the new value of x for the MEM above fails to satisfy
safe_insn_predicate, so force_reg generates the REG above. This won't
work for the output of the SET; presumably it's expected that only inputs
should ever get to force_reg here.
The MEM should have been made valid by replace_equiv_address. However,
replace_equiv_address ends up calling memory_address to create a valid
address, which calls LEGITIMIZE_ADDRESS - which changes the address and
returns one which still isn't valid for V2SImode.
Hence the patch below. I fixed rs6000_legitimize_address not to apply an
inappropriate strategy to addresses for E500 modes that have more
restricted offset ranges (and are handled further down the function) and
added an assertion to memory_address that the resulting address is valid.
Tested with no regressions with cross to powerpc-none-linux-gnuspe; also
bootstrapped with no regressions on i686-pc-linux-gnu to make sure the
assertion doesn't fail there. OK to commit?
2007-09-27 Joseph Myers <joseph@codesourcery.com>
* explow.c (memory_address): Assert that the generated address is
valid.
* config/rs6000/rs6000.c (rs6000_legitimize_address): Do not
reduce offset by units of 0x10000 for SPE vector modes or modes
used with E500 double instructions.
Index: gcc/explow.c
===================================================================
--- gcc/explow.c (revision 128824)
+++ gcc/explow.c (working copy)
@@ -497,6 +497,7 @@
done:
+ gcc_assert (memory_address_p (mode, x));
/* If we didn't change the address, we are done. Otherwise, mark
a reg as a pointer if we have REG or REG + CONST_INT. */
if (oldx == x)
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 128824)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -3381,7 +3381,10 @@
if (GET_CODE (x) == PLUS
&& GET_CODE (XEXP (x, 0)) == REG
&& GET_CODE (XEXP (x, 1)) == CONST_INT
- && (unsigned HOST_WIDE_INT) (INTVAL (XEXP (x, 1)) + 0x8000) >= 0x10000)
+ && (unsigned HOST_WIDE_INT) (INTVAL (XEXP (x, 1)) + 0x8000) >= 0x10000
+ && !(SPE_VECTOR_MODE (mode)
+ || (TARGET_E500_DOUBLE && (mode == DFmode || mode == TFmode
+ || mode == DImode))))
{
HOST_WIDE_INT high_int, low_int;
rtx sum;
--
Joseph S. Myers
joseph@codesourcery.com