On Tue, Jun 28, 2011 at 7:24 AM, Ulrich Weigand <uweig...@de.ibm.com> wrote:
> H.J. Lu wrote:
>> > find_replacement never checks subreg:
>> >
>> > Breakpoint 3, find_replacement (loc=3D0x7ffff068ab00)
>> > =A0 =A0at /export/gnu/import/git/gcc-x32/gcc/reload.c:6411
>> > 6411 =A0 =A0 =A0 =A0 =A0if (reloadreg && r->where =3D=3D loc)
>> > (reg:DI 0 ax)
>> > (reg/v/f:DI 182 [ b ])
>> > (gdb) call debug_rtx (*loc)
>> > (subreg:SI (reg/v/f:DI 182 [ b ]) 0)
>> > (gdb)
>> >
>>
>> This seems to work. Does it make any senses?
>
> Ah, I see. This was supposed to be handled via the SUBREG_LOC
> member of the replacement struct. Unfortunately, it turns out
> that this is no longer reliably set these days ...
>
> At first I was concerned that this might also cause problems at
> the other location where replacements are processed, subst_reloads.
> However, it turns out that code in subst_reloads is dead these days
> anyway, as the reloadreg is *always* a REG, and never a SUBREG.
>
> Once that code (and similar code in find_replacement that tries
> to handle SUBREG reloadregs) is removed, the only remaining user
> of the SUBREG_LOC field is actually find_replacement. But here
> we're doing a recursive descent through an RTL anyway, so we
> always know we're replacing inside a SUBREG.
>
> This makes the whole SUBREG_LOC field obsolete.
>
> The patch below implements those changes (untested so far).
> Can you verify that this works for you as well?
>
> Thanks,
> Ulrich
>
>
> ChangeLog:
>
> * reload.c (struct replacement): Remove SUBREG_LOC member.
> (push_reload): Do not set it.
> (push_replacement): Likewise.
> (subst_reload): Remove dead code.
> (copy_replacements): Remove assertion.
> (copy_replacements_1): Do not handle SUBREG_LOC.
> (move_replacements): Likewise.
> (find_replacement): Remove dead code. Detect subregs via
> recursive descent instead of via SUBREG_LOC.
>
> Index: gcc/reload.c
> ===================================================================
> *** gcc/reload.c (revision 175580)
> --- gcc/reload.c (working copy)
> *************** static int replace_reloads;
> *** 158,165 ****
> struct replacement
> {
> rtx *where; /* Location to store in */
> - rtx *subreg_loc; /* Location of SUBREG if WHERE is inside
> - a SUBREG; 0 otherwise. */
> int what; /* which reload this is for */
> enum machine_mode mode; /* mode it must have */
> };
> --- 158,163 ----
> *************** push_reload (rtx in, rtx out, rtx *inloc
> *** 1496,1502 ****
> {
> struct replacement *r = &replacements[n_replacements++];
> r->what = i;
> - r->subreg_loc = in_subreg_loc;
> r->where = inloc;
> r->mode = inmode;
> }
> --- 1494,1499 ----
> *************** push_reload (rtx in, rtx out, rtx *inloc
> *** 1505,1511 ****
> struct replacement *r = &replacements[n_replacements++];
> r->what = i;
> r->where = outloc;
> - r->subreg_loc = out_subreg_loc;
> r->mode = outmode;
> }
> }
> --- 1502,1507 ----
> *************** push_replacement (rtx *loc, int reloadnu
> *** 1634,1640 ****
> struct replacement *r = &replacements[n_replacements++];
> r->what = reloadnum;
> r->where = loc;
> - r->subreg_loc = 0;
> r->mode = mode;
> }
> }
> --- 1630,1635 ----
> *************** subst_reloads (rtx insn)
> *** 6287,6319 ****
> if (GET_MODE (reloadreg) != r->mode && r->mode != VOIDmode)
> reloadreg = reload_adjust_reg_for_mode (reloadreg, r->mode);
>
> ! /* If we are putting this into a SUBREG and RELOADREG is a
> ! SUBREG, we would be making nested SUBREGs, so we have to fix
> ! this up. Note that r->where == &SUBREG_REG (*r->subreg_loc). */
> !
> ! if (r->subreg_loc != 0 && GET_CODE (reloadreg) == SUBREG)
> ! {
> ! if (GET_MODE (*r->subreg_loc)
> ! == GET_MODE (SUBREG_REG (reloadreg)))
> ! *r->subreg_loc = SUBREG_REG (reloadreg);
> ! else
> ! {
> ! int final_offset =
> ! SUBREG_BYTE (*r->subreg_loc) + SUBREG_BYTE (reloadreg);
> !
> ! /* When working with SUBREGs the rule is that the byte
> ! offset must be a multiple of the SUBREG's mode. */
> ! final_offset = (final_offset /
> ! GET_MODE_SIZE (GET_MODE (*r->subreg_loc)));
> ! final_offset = (final_offset *
> ! GET_MODE_SIZE (GET_MODE (*r->subreg_loc)));
> !
> ! *r->where = SUBREG_REG (reloadreg);
> ! SUBREG_BYTE (*r->subreg_loc) = final_offset;
> ! }
> ! }
> ! else
> ! *r->where = reloadreg;
> }
> /* If reload got no reg and isn't optional, something's wrong. */
> else
> --- 6282,6288 ----
> if (GET_MODE (reloadreg) != r->mode && r->mode != VOIDmode)
> reloadreg = reload_adjust_reg_for_mode (reloadreg, r->mode);
>
> ! *r->where = reloadreg;
> }
> /* If reload got no reg and isn't optional, something's wrong. */
> else
> *************** subst_reloads (rtx insn)
> *** 6327,6336 ****
> void
> copy_replacements (rtx x, rtx y)
> {
> - /* We can't support X being a SUBREG because we might then need to know
> its
> - location if something inside it was replaced. */
> - gcc_assert (GET_CODE (x) != SUBREG);
> -
> copy_replacements_1 (&x, &y, n_replacements);
> }
>
> --- 6296,6301 ----
> *************** copy_replacements_1 (rtx *px, rtx *py, i
> *** 6344,6367 ****
> const char *fmt;
>
> for (j = 0; j < orig_replacements; j++)
> ! {
> ! if (replacements[j].subreg_loc == px)
> ! {
> ! r = &replacements[n_replacements++];
> ! r->where = replacements[j].where;
> ! r->subreg_loc = py;
> ! r->what = replacements[j].what;
> ! r->mode = replacements[j].mode;
> ! }
> ! else if (replacements[j].where == px)
> ! {
> ! r = &replacements[n_replacements++];
> ! r->where = py;
> ! r->subreg_loc = 0;
> ! r->what = replacements[j].what;
> ! r->mode = replacements[j].mode;
> ! }
> ! }
>
> x = *px;
> y = *py;
> --- 6309,6321 ----
> const char *fmt;
>
> for (j = 0; j < orig_replacements; j++)
> ! if (replacements[j].where == px)
> ! {
> ! r = &replacements[n_replacements++];
> ! r->where = py;
> ! r->what = replacements[j].what;
> ! r->mode = replacements[j].mode;
> ! }
>
> x = *px;
> y = *py;
> *************** move_replacements (rtx *x, rtx *y)
> *** 6387,6399 ****
> int i;
>
> for (i = 0; i < n_replacements; i++)
> ! if (replacements[i].subreg_loc == x)
> ! replacements[i].subreg_loc = y;
> ! else if (replacements[i].where == x)
> ! {
> ! replacements[i].where = y;
> ! replacements[i].subreg_loc = 0;
> ! }
> }
>
> /* If LOC was scheduled to be replaced by something, return the replacement.
> --- 6341,6348 ----
> int i;
>
> for (i = 0; i < n_replacements; i++)
> ! if (replacements[i].where == x)
> ! replacements[i].where = y;
> }
>
> /* If LOC was scheduled to be replaced by something, return the replacement.
> *************** find_replacement (rtx *loc)
> *** 6415,6446 ****
>
> return reloadreg;
> }
> ! else if (reloadreg && r->subreg_loc == loc)
> {
> ! /* RELOADREG must be either a REG or a SUBREG.
> !
> ! ??? Is it actually still ever a SUBREG? If so, why? */
> !
> ! if (REG_P (reloadreg))
> ! return gen_rtx_REG (GET_MODE (*loc),
> ! (REGNO (reloadreg) +
> ! subreg_regno_offset (REGNO (SUBREG_REG
> (*loc)),
> GET_MODE (SUBREG_REG
> (*loc)),
> SUBREG_BYTE (*loc),
> GET_MODE (*loc))));
> - else if (GET_MODE (reloadreg) == GET_MODE (*loc))
> - return reloadreg;
> - else
> - {
> - int final_offset = SUBREG_BYTE (reloadreg) + SUBREG_BYTE (*loc);
> -
> - /* When working with SUBREGs the rule is that the byte
> - offset must be a multiple of the SUBREG's mode. */
> - final_offset = (final_offset / GET_MODE_SIZE (GET_MODE (*loc)));
> - final_offset = (final_offset * GET_MODE_SIZE (GET_MODE (*loc)));
> - return gen_rtx_SUBREG (GET_MODE (*loc), SUBREG_REG (reloadreg),
> - final_offset);
> - }
> }
> }
>
> --- 6364,6378 ----
>
> return reloadreg;
> }
> ! else if (reloadreg && GET_CODE (*loc) == SUBREG
> ! && r->where == &SUBREG_REG (*loc))
> {
> ! return gen_rtx_REG (GET_MODE (*loc),
> ! (REGNO (reloadreg)
> ! + subreg_regno_offset (REGNO (SUBREG_REG
> (*loc)),
> GET_MODE (SUBREG_REG
> (*loc)),
> SUBREG_BYTE (*loc),
> GET_MODE (*loc))));
> }
> }
>
>
> --
> Dr. Ulrich Weigand
> GNU Toolchain for Linux on System z and Cell BE
> ulrich.weig...@de.ibm.com
>
it doesn't work;
allocation.f: In function 'allocation':
allocation.f:1048:0: internal compiler error: in subreg_get_info, at
rtlanal.c:3235
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
(gdb) bt
#0 fancy_abort (file=0xe7d920 "/export/gnu/import/git/gcc-x32/gcc/rtlanal.c",
line=3235, function=0xe7d8c0 "subreg_get_info")
at /export/gnu/import/git/gcc-x32/gcc/diagnostic.c:892
#1 0x0000000000888c8e in subreg_get_info (xregno=<optimized out>,
xmode=<optimized out>, offset=0, ymode=<optimized out>,
info=<optimized out>) at /export/gnu/import/git/gcc-x32/gcc/rtlanal.c:3235
#2 0x0000000000888d4c in subreg_regno_offset (xregno=<optimized out>,
xmode=<optimized out>, offset=<optimized out>, ymode=<optimized out>)
at /export/gnu/import/git/gcc-x32/gcc/rtlanal.c:3387
#3 0x0000000000868546 in find_replacement (loc=0x7ffff063ee80)
at /export/gnu/import/git/gcc-x32/gcc/reload.c:6372
#4 0x00000000008685b8 in find_replacement (loc=0x7ffff063ba48)
at /export/gnu/import/git/gcc-x32/gcc/reload.c:6385
#5 0x0000000000877182 in gen_reload (out=0x7ffff062be80, in=0x7ffff063ba40,
opnum=0, type=RELOAD_FOR_OPERAND_ADDRESS)
at /export/gnu/import/git/gcc-x32/gcc/reload1.c:8599
since subreg_regno_offset only works on hard registers.
--
H.J.
