------- Additional Comments From uweigand at gcc dot gnu dot org 2004-11-24
14:50 -------
We have here the following situation before reload:
(insn 27 46 28 7 (set (reg:DI 118 [ D.1118 ])
(const_int 4294967295 [0xffffffff])) 313 {*movdi_internal32} (nil)
(nil))
where reg 118 gets assigned a stack slot, and due to the large
stack frame size this slot is not directly addressable.
Now, when reverting my patch, what happens is that LEGITIMIZE_RELOAD_ADDRESS
finds a way to construct the stack slot address, and the constant is reloaded
into a register (pair):
(insn 67 46 66 7 (set (reg:DI 2 r2)
(const_int 4294967295 [0xffffffff])) 313 {*movdi_internal32} (nil)
(nil))
(insn 66 67 27 7 (set (reg:SI 9 r9)
(plus:SI (reg/f:SI 30 r30)
(const_int 131072 [0x20000]))) 64 {*addsi3_internal1} (nil)
(nil))
(insn 27 66 28 7 (set (mem:DI (plus:SI (reg:SI 9 r9)
(const_int 40 [0x28])) [0 D.1118+0 S8 A8])
(reg:DI 2 r2)) 313 {*movdi_internal32} (nil)
(nil))
Note that insn 27 now uses the r -> o alternative of *movdi_internal32;
this is allowed only if the address is offsettable.
Now, that address is the one that was constructed by
LEGITIMIZE_RELOAD_ADDRESS. Unfortunately, reload common code has
no way to actually look at what LEGITIMIZE_RELOAD_ADDRESS does,
so that it could decide whether or not the address constructed
is in fact an offsettable one or not.
Before my 2004-08-22 patch, reload simply always assumed the address
is offsettable, due to an apparent bug in LEGITIMIZE_RELOAD_ADDRESS
handling: reload assumed that L_R_A would always completely replace
the address by a single base register (which of course implies the
address is offsettable). This bug caused problems on s390.
My patch removed this erroneous assumption that L_R_A always completly
replaces the address; as we don't know anything further we then have
to make the conservative assumption that addresses constructed by L_R_A
are never offsettable. Thus reload doesn't accept the r -> o alternative
of *movdi_internal32 any more; the one it chooses instead is f -> m.
This implies reloading the constant into a floating point register,
and that's what reload goes ahead and does:
(insn 67 46 66 7 (set (reg:DI 32 f0)
(const_int 4294967295 [0xffffffff])) 313 {*movdi_internal32} (nil)
(nil))
(insn 66 67 27 7 (set (reg:SI 2 r2)
(plus:SI (reg/f:SI 30 r30)
(const_int 131072 [0x20000]))) 64 {*addsi3_internal1} (nil)
(nil))
(insn 27 66 28 7 (set (mem:DI (plus:SI (reg:SI 2 r2)
(const_int 40 [0x28])) [0 D.1118+0 S8 A8])
(reg:DI 32 f0)) 313 {*movdi_internal32} (nil)
(nil))
However, the insn 67 emitted thus is not actually implemented by
any of the alternatives or splitters; thus the crash later on.
This would appear to be a latent bug in the rs6000 back end.
Reload insns generated during the gen_reload phase *must* be
implemented by the backend. I'm not familiar enough with the
platform to suggested the best way to do so; one obvious option
would be force the constant to memory using either from within
the movdi expander or via a secondary input reload.
The next question is whether the new code, if implemented
correctly, is better or worse than the old code -- again this
is a rs6000 back-end issue.
However, one middle-end question remains: while it is obviously
wrong for reload to assume that L_R_A always results in a simple
base register, at least on rs6000 is appears to be the case that
L_R_A always results in an *offsettable* address. If this is true,
we might be missing optimizations by not exploiting that knowledge
in reload any more.
One way might be to extend the L_R_A interface in a way that would
allow the back end to inform the middle end about properties of
the address is has constructed; I'm not sure how this interface
should look in detail.
The other option would be to simply go back to having the middle end
assume the L_R_A constructed addresses are always offsettable. This
could be implemented by something like the following patch:
Index: reload.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload.c,v
retrieving revision 1.258
diff -c -p -r1.258 reload.c
*** reload.c 9 Nov 2004 17:29:02 -0000 1.258
--- reload.c 24 Nov 2004 14:49:04 -0000
*************** find_reloads (rtx insn, int replace, int
*** 3189,3196 ****
&& ((ind_levels ? offsettable_memref_p (operand)
: offsettable_nonstrict_memref_p (operand))
/* A reloaded address is offsettable because it is now
! just a simple register indirect. */
! || address_reloaded[i] == 1))
|| (REG_P (operand)
&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
&& reg_renumber[REGNO (operand)] < 0
--- 3189,3198 ----
&& ((ind_levels ? offsettable_memref_p (operand)
: offsettable_nonstrict_memref_p (operand))
/* A reloaded address is offsettable because it is now
! just a simple register indirect. Addresses built
! via LEGITIMIZE_RELOAD_ADDRESS must always be
! offsettable as well. */
! || address_reloaded[i]))
|| (REG_P (operand)
&& REGNO (operand) >= FIRST_PSEUDO_REGISTER
&& reg_renumber[REGNO (operand)] < 0
(This requirement should then be documented in the L_R_A docs as well.)
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18641
