This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Reload inheritance and REG_CANNOT_CHANGE_MODE_P
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Mon, 03 May 2004 10:20:56 +0100
- Subject: Reload inheritance and REG_CANNOT_CHANGE_MODE_P
The attached testcase fails on 32-bit little-endian MIPS targets due to
a reload inheritance bug. Usually, if a reload is from or to a group of
N hard registers, and the spill register is also a group of N registers,
reload will allow the values of individual registers to be inherited.
For example, if we have a reload from (reg:DI x) to (reg:DI y), where
both x and y are 32-bit registers, the inheritance code will record that
(reg:SI x+1) == (reg:SI y+1).
Unfortunately, as indicated by REG_CANNOT_CHANGE_MODE_P, some classes
of register can't be narrowed in this way. The problem is that the
inheritance code isn't checking R_C_C_M_P.
On little-endian MIPS targets, the hi/lo register classes are included
in CANNOT_CHANGE_MODE_CLASS because register hi has a lower number
than register lo (the opposite of the usual little-endian ordering).
This is something of a long-standing wart, but other targets suffer
from similar problems, and C_C_M_C has been the established way
of dealing with them.
In the testcase, we have:
(set (reg:DI t1)
(mult:DI (zero_extend:SI (reg:SI x))
(zero_extend:SI (reg:SI x))))
(set (reg:SI t2)
(asm_operands:SI ("mflo %0") ... [(subreg:SI (reg:DI t1) 4)]))
t1 gets allocated $4, so the first instruction has an output reload
from (reg:DI hi) to (reg:DI $4). The inheritance code records the
DImode reload, and also says that (reg:SI lo) contains the value of
(reg:SI $5). This is incorrect, since (reg:SI $5) is the high part
of (reg:DI $4), but (reg:SI lo) is the low part of (reg:DI hi).
The second instruction has an input reload from (reg:SI $5) to
(reg:SI lo), and this reload gets deleted by inheriting the value
of (reg:SI lo) from the first instruction.
The fix is simply to check R_C_C_M_P for both the source and target
registers. Patch bootstrapped and regression tested on mips-sgi-irix6.5,
mips64-linux-gnu, mips64el-linux-gnu and i686-pc-linux-gnu. OK to install?
Patch
* reload1.c (inherit_piecemeal_p): New function.
(emit_reload_insns): When reloading a group of hard registers, use
inherit_piecemeal_p to decide whether the values of individual hard
registers can be inherited.
testsuite/
* gcc.dg/torture/mips-hilo-2.c: New test.
Index: reload1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.433
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.433 reload1.c
--- reload1.c 13 Apr 2004 23:27:43 -0000 1.433
+++ reload1.c 3 May 2004 08:46:37 -0000
@@ -417,6 +417,7 @@ static void emit_output_reload_insns (st
int);
static void do_input_reload (struct insn_chain *, struct reload *, int);
static void do_output_reload (struct insn_chain *, struct reload *, int);
+static bool inherit_piecemeal_p (int, int);
static void emit_reload_insns (struct insn_chain *);
static void delete_output_reload (rtx, int, int);
static void delete_address_reloads (rtx, rtx);
@@ -6956,6 +6957,27 @@ do_output_reload (struct insn_chain *cha
emit_output_reload_insns (chain, rld + j, j);
}
+/* Reload number R reloads from or to a group of hard registers starting at
+ register REGNO. Return true if it can be treated for inheritance purposes
+ like a group of reloads, each one reloading a single hard register.
+ The caller has already checked that the spill register and REGNO use
+ the same number of registers to store the reload value. */
+
+static bool
+inherit_piecemeal_p (int r, int regno)
+{
+#ifdef CANNOT_CHANGE_MODE_CLASS
+ return (!REG_CANNOT_CHANGE_MODE_P (reload_spill_index[r],
+ GET_MODE (rld[r].reg_rtx),
+ reg_raw_mode[reload_spill_index[r]])
+ && !REG_CANNOT_CHANGE_MODE_P (regno,
+ GET_MODE (rld[r].reg_rtx),
+ reg_raw_mode[regno]));
+#else
+ return true;
+#endif
+}
+
/* Output insns to reload values in and out of the chosen reload regs. */
static void
@@ -7137,11 +7159,16 @@ emit_reload_insns (struct insn_chain *ch
int nnr = (nregno >= FIRST_PSEUDO_REGISTER ? 1
: hard_regno_nregs[nregno]
[GET_MODE (rld[r].reg_rtx)]);
+ bool piecemeal;
spill_reg_store[i] = new_spill_reg_store[i];
spill_reg_stored_to[i] = out;
reg_last_reload_reg[nregno] = rld[r].reg_rtx;
+ piecemeal = (nregno < FIRST_PSEUDO_REGISTER
+ && nr == nnr
+ && inherit_piecemeal_p (r, nregno));
+
/* If NREGNO is a hard register, it may occupy more than
one register. If it does, say what is in the
rest of the registers assuming that both registers
@@ -7151,7 +7178,7 @@ emit_reload_insns (struct insn_chain *ch
if (nregno < FIRST_PSEUDO_REGISTER)
for (k = 1; k < nnr; k++)
reg_last_reload_reg[nregno + k]
- = (nr == nnr
+ = (piecemeal
? regno_reg_rtx[REGNO (rld[r].reg_rtx) + k]
: 0);
@@ -7160,7 +7187,7 @@ emit_reload_insns (struct insn_chain *ch
{
CLEAR_HARD_REG_BIT (reg_reloaded_dead, i + k);
reg_reloaded_contents[i + k]
- = (nregno >= FIRST_PSEUDO_REGISTER || nr != nnr
+ = (nregno >= FIRST_PSEUDO_REGISTER || !piecemeal
? nregno
: nregno + k);
reg_reloaded_insn[i + k] = insn;
@@ -7185,6 +7212,7 @@ emit_reload_insns (struct insn_chain *ch
int nregno;
int nnr;
rtx in;
+ bool piecemeal;
if (GET_CODE (rld[r].in) == REG
&& REGNO (rld[r].in) >= FIRST_PSEUDO_REGISTER)
@@ -7201,10 +7229,14 @@ emit_reload_insns (struct insn_chain *ch
reg_last_reload_reg[nregno] = rld[r].reg_rtx;
+ piecemeal = (nregno < FIRST_PSEUDO_REGISTER
+ && nr == nnr
+ && inherit_piecemeal_p (r, nregno));
+
if (nregno < FIRST_PSEUDO_REGISTER)
for (k = 1; k < nnr; k++)
reg_last_reload_reg[nregno + k]
- = (nr == nnr
+ = (piecemeal
? regno_reg_rtx[REGNO (rld[r].reg_rtx) + k]
: 0);
@@ -7220,7 +7252,7 @@ emit_reload_insns (struct insn_chain *ch
{
CLEAR_HARD_REG_BIT (reg_reloaded_dead, i + k);
reg_reloaded_contents[i + k]
- = (nregno >= FIRST_PSEUDO_REGISTER || nr != nnr
+ = (nregno >= FIRST_PSEUDO_REGISTER || !piecemeal
? nregno
: nregno + k);
reg_reloaded_insn[i + k] = insn;
--- /dev/null Fri Apr 23 00:21:55 2004
+++ testsuite/gcc.dg/torture/mips-hilo-2.c Mon May 3 09:43:57 2004
@@ -0,0 +1,24 @@
+/* Due to a reload inheritance bug, the asm statement in f() would be passed
+ the low part of u.ll on little-endian 32-bit targets. */
+/* { dg-do run { target mips*-*-* } } */
+
+unsigned int g;
+
+unsigned long long f (unsigned int x)
+{
+ union { unsigned long long ll; unsigned int parts[2]; } u;
+
+ u.ll = ((unsigned long long) x * x);
+ asm ("mflo\t%0" : "=r" (g) : "l" (u.parts[1]));
+ return u.ll;
+}
+
+int main ()
+{
+ union { unsigned long long ll; unsigned int parts[2]; } u;
+
+ u.ll = f (0x12345678);
+ if (g != u.parts[1])
+ abort ();
+ exit (0);
+}