This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RTL, ColdFire 24/63] Add support for a MODE_INDEX_REG_CLASS macro
On Tue, 2007-02-27 at 16:50 +0000, Richard Sandiford wrote:
> Jeffrey Law <law@redhat.com> writes:
> > On Thu, 2007-02-22 at 19:08 -0800, Mark Mitchell wrote:
> >> I'm not quite sure if I know exactly whose court the ball is in at this
> >> point. If I understand correctly, Jeff is still unhappy with the patch,
> >> and Richard still thinks it's the right thing.
> > It's in my court. Current todo is to look at what Bernd did and why
> > and ponder how it plays with what Richard wants to do.
> >
> > I'd also like to see (and this is a new request) some code which
> > triggers these problems. I have a couple theories that I'd like to
> > poke at, but without triggering code that's going to be tough to do.
>
> OK. I can't remember off-hand which testcases failed without this,
> so here's a testcase specifically for it:
>
> --------------------------------------------------------------------
> void foo (float *x);
> float bar (void)
> {
> float x[0x5000];
> foo (x);
> return x[0x4000] + x[0];
> }
> --------------------------------------------------------------------
>
> Compile with -O2 -fomit-frame-pointer -mcpu=5485.
So after poking at this for a few minutes with the debugger, I'm really
interested in why you think fixing double_reg_address_ok to be indexed
by the mode of the memory reference is insufficient to fix this problem.
Basically we have this after lreg:
(insn 24 19 30 2 (set (reg/i:SF 0 %d0 [ <result> ])
(plus:SF (mem/s:SF (plus:SI (reg/f:SI 14 %a6)
(const_int -16384 [0xffffc000])) [3 x+65536 S4 A16])
(mem/s:SF (reg/f:SI 31) [3 x+0 S4 A16]))) 157 {addsf3_cf}
(nil)
(expr_list:REG_DEAD (reg/f:SI 31)
(nil)))
Which is a valid insn. Register elimination changes the memory
reference to something like:
(mem:SF (plus:SI (%sp) (const_int 65540))
Which is invalid because the offset is too big.
find_reloads_address comes along and realizes the address is
invalid and reloads the invalid displacement into an index register
creating the (invalid) reg+reg addressing mode via this code:
if (double_reg_address_ok)
{
/* Unshare the sum as well. */
*loc = ad = copy_rtx (ad);
/* Reload the displacement into an index reg.
We assume the frame pointer or arg pointer is a base reg.
*/
find_reloads_address_part (XEXP (ad, 1), &XEXP (ad, 1),
INDEX_REG_CLASS, GET_MODE (ad),
opnum,
type, ind_levels);
return 0;
}
The problem, ISTM, is that reload assumes that if reg+reg addressing
is valid for QImode (per the double_reg_address_ok computation) then
reg+reg addressing must be valid for all modes which is clearly
incorrect for coldfire (as well as the PA). If we twiddle reload
to made double_reg_address_ok be mode dependent, then instead of
reloading the displacement into a register and creating a bogus
addressing mode, we'll instead reload the entire sum into a register
and use register indirect which is clearly valid. Note you'll still
get full use of indexed addressing in cases where your G_I_L_A
allows it.
I haven't really tested the attached patch except to verify that
the provided testcase no longer aborts and that the reloads we
generate for the problem insn look correct. However, I feel its a
much more correct solution than what you're trying to do.
Thoughts/Comments?
Index: postreload.c
===================================================================
--- postreload.c (revision 122379)
+++ postreload.c (working copy)
@@ -703,12 +703,15 @@
int last_label_ruid;
int min_labelno, n_labels;
HARD_REG_SET ever_live_at_start, *label_live;
+ enum machine_mode mode;
/* If reg+reg can be used in offsetable memory addresses, the main chunk of
reload has already used it where appropriate, so there is no use in
trying to generate it now. */
- if (double_reg_address_ok && INDEX_REG_CLASS != NO_REGS)
- return;
+ for (mode = VOIDmode; (int) mode < NUM_MACHINE_MODES;
+ mode = (enum machine_mode) ((int) mode + 1))
+ if (double_reg_address_ok[mode] && INDEX_REG_CLASS != NO_REGS)
+ return;
/* To avoid wasting too much time later searching for an index register,
determine the minimum and maximum index register numbers. */
Index: reload.c
===================================================================
--- reload.c (revision 122379)
+++ reload.c (working copy)
@@ -4965,7 +4965,7 @@
loc = &XEXP (*loc, 0);
}
- if (double_reg_address_ok)
+ if (double_reg_address_ok[mode])
{
/* Unshare the sum as well. */
*loc = ad = copy_rtx (ad);
@@ -5008,8 +5008,8 @@
find_reloads_subreg_address.
If we decide to do something, it must be that `double_reg_address_ok'
- is true. We generate a reload of the base register + constant and
- rework the sum so that the reload register will be added to the index.
+ is true for MODE. We generate a reload of the base register + constant
+ and rework the sum so that the reload register will be added to the index.
This is safe because we know the address isn't shared.
We check for the base register as both the first and second operand of
Index: reload.h
===================================================================
--- reload.h (revision 122379)
+++ reload.h (working copy)
@@ -189,7 +189,7 @@
extern char indirect_symref_ok;
/* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid. */
-extern char double_reg_address_ok;
+extern char double_reg_address_ok[MAX_MACHINE_MODE];
extern int num_not_at_initial_offset;
Index: reload1.c
===================================================================
--- reload1.c (revision 122379)
+++ reload1.c (working copy)
@@ -244,8 +244,9 @@
which these are valid is the same as spill_indirect_levels, above. */
char indirect_symref_ok;
-/* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid. */
-char double_reg_address_ok;
+/* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid,
+ indxed by the mode of the memory reference. */
+char double_reg_address_ok[MAX_MACHINE_MODE];
/* Record the stack slot for each spilled hard register. */
static rtx spill_stack_slot[FIRST_PSEUDO_REGISTER];
@@ -453,6 +454,7 @@
init_reload (void)
{
int i;
+ enum machine_mode mode;
/* Often (MEM (REG n)) is still valid even if (REG n) is put on the stack.
Set spill_indirect_levels to the number of levels such addressing is
@@ -477,24 +479,25 @@
tem = gen_rtx_MEM (Pmode, gen_rtx_SYMBOL_REF (Pmode, "foo"));
indirect_symref_ok = memory_address_p (QImode, tem);
- /* See if reg+reg is a valid (and offsettable) address. */
+ /* See if reg+reg is a valid (and offsettable) address for each mode. */
+ for (mode = VOIDmode; (int) mode < NUM_MACHINE_MODES;
+ mode = (enum machine_mode) ((int) mode + 1))
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ {
+ tem = gen_rtx_PLUS (Pmode,
+ gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
+ gen_rtx_REG (Pmode, i));
- for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- {
- tem = gen_rtx_PLUS (Pmode,
- gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM),
- gen_rtx_REG (Pmode, i));
+ /* This way, we make sure that reg+reg is an offsettable address. */
+ tem = plus_constant (tem, GET_MODE_SIZE (mode));
- /* This way, we make sure that reg+reg is an offsettable address. */
- tem = plus_constant (tem, 4);
+ if (memory_address_p (mode, tem))
+ {
+ double_reg_address_ok[mode] = 1;
+ break;
+ }
+ }
- if (memory_address_p (QImode, tem))
- {
- double_reg_address_ok = 1;
- break;
- }
- }
-
/* Initialize obstack for our rtl allocation. */
gcc_obstack_init (&reload_obstack);
reload_startobj = obstack_alloc (&reload_obstack, 0);