* 3.1 BUG: reload inheritance / frame pointer elimination
@ 2002-04-08 13:02 Ulrich Weigand
0 siblings, 0 replies; only message in thread
From: Ulrich Weigand @ 2002-04-08 13:02 UTC (permalink / raw)
To: gcc
Hello,
I've found a somewhat weird problem in choose_reload_regs
which causes incorrect code to be generated for a test case
from a proprietary test suite (on s390-ibm-linux).
I'm not completely sure I've pinpointed the bug correctly,
as it involves an unfortunate interaction of various parts
of reload (reload inheritance, delete_output_reload, frame
pointer elimination).
Anyway, in my test case the situation before reload is
like this:
(insn 6832 2453 6833 (set (reg:SI 3101)
(ashift:SI (reg/v:SI 164)
(const_int 1 [0x1]))) 236 {ashlsi3} (insn_list 288 (nil))
(expr_list:REG_EQUAL (mult:SI (reg/v:SI 164)
(const_int 2 [0x2]))
(nil)))
(insn 6833 6832 2448 (parallel[
(set (reg:SI 3101)
(plus:SI (reg:SI 3101)
(reg/v:SI 164)))
(clobber (reg:CC 33 %cc))
] ) 121 {addsi3} (insn_list 6832 (insn_list 288 (nil)))
(expr_list:REG_DEAD (reg/v:SI 164)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(expr_list:REG_EQUAL (mult:SI (reg/v:SI 164)
(const_int 3 [0x3]))
(nil)))))
[snip insns not touching reg 3101]
(insn 6835 2508 6838 (set (reg:SI 3101)
(ashift:SI (reg:SI 3101)
(const_int 5 [0x5]))) 236 {ashlsi3} (insn_list 6833 (nil))
(nil))
(insn 6838 6835 2505 (parallel[
(set (reg:SI 3101)
(plus:SI (reg:SI 3101)
(const_int 80 [0x50])))
(clobber (reg:CC 33 %cc))
] ) 121 {addsi3} (insn_list 6835 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
These insns are reloaded into:
(insn 8976 2453 6832 (set (reg:SI 11 %r11)
(mem/u/f:SI (symbol_ref/u:SI ("*.LC14")) [13 S4 A32])) 56 {*movsi}
(nil)
(nil))
{NOTE: reg 164 was in fact constant, and this constant is in the
literal pool at .LC14, so this is fine.}
(insn 6832 8976 8985 (set (reg:SI 11 %r11)
(ashift:SI (reg:SI 11 %r11)
(const_int 1 [0x1]))) 236 {ashlsi3} (insn_list 288 (nil))
(expr_list:REG_EQUAL (mult:SI (reg/v:SI 164)
(const_int 2 [0x2]))
(nil)))
(insn 8985 6832 6833 (set (reg:SI 5 %r5)
(reg:SI 11 %r11)) 56 {*movsi} (nil)
(nil))
{NOTE: Somewhat weird, but not actually wrong ...}
(insn 6833 8985 8990 (parallel[
(set (reg:SI 5 %r5)
(plus:SI (reg:SI 5 %r5)
(const_int 10 [0xa])))
(clobber (reg:CC 33 %cc))
] ) 121 {addsi3} (insn_list 6832 (insn_list 288 (nil)))
(expr_list:REG_EQUAL (mult:SI (reg/v:SI 164)
(const_int 3 [0x3]))
(nil)))
{NOTE: there was initally an insn 8988 generated, performing
the output reload of %r5 to the stack slot at %r15+6892;
this offset is stored in the literal pool at .LC62.
This was later on deleted by delete_output_reload}.
[snip]
(insn 8997 2508 9000 (set (reg:SI 8 %r8)
(mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4 A32])) 56 {*movsi}
(nil)
(nil))
(insn 9000 8997 6835 (set (reg:SI 11 %r11)
(mem:SI (plus:SI (reg/f:SI 15 %r15)
(reg:SI 8 %r8)) [56 S4 A8])) 56 {*movsi} (nil)
(nil))
{NOTE: however, here the code assumes that reg 3101 now
lives at its stack slot, which is wrong; the output
reload was deleted!}
(insn 6835 9000 9009 (set (reg:SI 11 %r11)
(ashift:SI (reg:SI 11 %r11)
(const_int 5 [0x5]))) 236 {ashlsi3} (insn_list 6833 (nil))
(nil))
(insn 9009 6835 6838 (set (reg:SI 5 %r5)
(reg:SI 11 %r11)) 56 {*movsi} (nil)
(nil))
{NOTE: Again this weirdness ...]
(insn 6838 9009 9006 (parallel[
(set (reg:SI 5 %r5)
(plus:SI (reg:SI 5 %r5)
(const_int 80 [0x50])))
(clobber (reg:CC 33 %cc))
] ) 121 {addsi3} (insn_list 6835 (nil))
(nil))
{NOTE: during the processing of the input reloads of *this* insn,
the output reload for 6833 is deleted.}
(insn 9006 6838 9012 (set (reg:SI 3 %r3)
(mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4 A32])) 56 {*movsi}
(nil)
(nil))
(insn 9012 9006 9014 (set (mem:SI (plus:SI (reg/f:SI 15 %r15)
(reg:SI 3 %r3)) [56 S4 A8])
(reg:SI 5 %r5)) 56 {*movsi} (nil)
(nil))
debug_reload output:
Reloads for insn # 6832
Reload 0: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4
A32])
ADDR_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
reload_reg_rtx: (reg:SI 8 %r8)
Reload 1: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC14")) [13 S4
A32])
reload_out (SI) = (mem:SI (plus:SI (reg/f:SI 15 %r15)
(const_int 6892
[0x1aec])) [56 S4 A8])
GENERAL_REGS, RELOAD_OTHER (opnum = 0), can't combine
reload_in_reg: (reg/v:SI 164)
reload_out_reg: (reg:SI 3101)
reload_reg_rtx: (reg:SI 11 %r11)
Reloads for insn # 6833
Reload 0: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4
A32])
ADDR_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
reload_reg_rtx: (reg:SI 3 %r3)
Reload 1: ADDR_REGS, RELOAD_FOR_OTHER_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
Reload 2: reload_in (SI) = (reg:SI 11 %r11)
reload_out (SI) = (mem:SI (plus:SI (reg/f:SI 15 %r15)
(const_int 6892
[0x1aec])) [56 S4 A8])
GENERAL_REGS, RELOAD_OTHER (opnum = 0), can't combine
reload_in_reg: (reg:SI 3101)
reload_out_reg: (reg:SI 3101)
reload_reg_rtx: (reg:SI 5 %r5)
[snip]
Reloads for insn # 6835
Reload 0: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4
A32])
ADDR_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
reload_reg_rtx: (reg:SI 8 %r8)
Reload 1: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4
A32])
ADDR_REGS, RELOAD_FOR_OTHER_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
reload_reg_rtx: (reg:SI 8 %r8)
Reload 2: reload_in (SI) = (mem:SI (plus:SI (reg/f:SI 15 %r15)
(const_int 6892
[0x1aec])) [56 S4 A8])
reload_out (SI) = (mem:SI (plus:SI (reg/f:SI 15 %r15)
(const_int 6892
[0x1aec])) [56 S4 A8])
GENERAL_REGS, RELOAD_OTHER (opnum = 0), can't combine
reload_in_reg: (reg:SI 3101)
reload_out_reg: (reg:SI 3101)
reload_reg_rtx: (reg:SI 11 %r11)
Reloads for insn # 6838
Reload 0: reload_in (SI) = (mem/u/f:SI (symbol_ref/u:SI ("*.LC62")) [13 S4
A32])
ADDR_REGS, RELOAD_FOR_OUTPUT_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
reload_reg_rtx: (reg:SI 3 %r3)
Reload 1: ADDR_REGS, RELOAD_FOR_OTHER_ADDRESS (opnum = 0), can't combine
reload_in_reg: (const_int 6892 [0x1aec])
Reload 2: reload_in (SI) = (reg:SI 11 %r11)
reload_out (SI) = (mem:SI (plus:SI (reg/f:SI 15 %r15)
(const_int 6892
[0x1aec])) [56 S4 A8])
GENERAL_REGS, RELOAD_OTHER (opnum = 0), can't combine
reload_in_reg: (reg:SI 3101)
reload_out_reg: (reg:SI 3101)
reload_reg_rtx: (reg:SI 5 %r5)
Apparently, the problem is that during the input reload
processing for 6838, the output reload of 6833 is deleted,
ignoring the fact that there is 6835 in between, which
assumes the output reload has taken place.
The cause for this appears to be the peculiar reload
inheritance that is used in insn 6838;
choose_reload_regs has noticed that the value of reg 3101
is already present in reg 11, but chose not to use reg 11
as inherited reload register, but only as reload_override_in;
copying the value to reg 5.
The reason for this is that reg 11 happens to be the
HARD_FRAME_POINTER_REGNUM; however, as the frame pointer
was eliminated, it is in fact used as normal register here.
However, this (part of a) check in choose_reload_regs
/* Don't clobber the frame pointer. */
|| (i == HARD_FRAME_POINTER_REGNUM
&& rld[r].out)
refuses to inherit reg 11, no matter whether this is
actually the frame pointer or it was eliminated.
Now, in do_input_reload for 6838, the following check hits:
if (optimize
&& (reload_inherited[j] || reload_override_in[j])
&& rl->reg_rtx
&& GET_CODE (rl->reg_rtx) == REG
&& spill_reg_store[REGNO (rl->reg_rtx)] != 0
&& (dead_or_set_p (insn,
spill_reg_stored_to[REGNO (rl->reg_rtx)])
|| rtx_equal_p (spill_reg_stored_to[REGNO (rl->reg_rtx)],
rl->out_reg)))
delete_output_reload (insn, j, REGNO (rl->reg_rtx));
because reload_override_in is set, and reg_rtx (reg 5)
was in fact last used as spill register for reg 3101
(in insn 6833!).
Thus, delete_output_reload is called an ends up deleting
the output reload for 6833 ...
The question is now, who's at fault? Special-casing the
hard frame pointer even when it is not used in any special
way seems wrong to me, so I've changed that check to
/* Don't clobber the frame pointer. */
|| (i == HARD_FRAME_POINTER_REGNUM
&& frame_pointer_needed
&& rld[r].out)
With this change, correct code is generated and the
test case passes. Do you agree this is the right way?
(I'll run a bootstrap/regtest cycle later ...)
However, I'm not sure the checks on delete_output_reload
are strict enough. Is it really not necessary to verify
that no insn *between* the spill_reg_store insn and the
current insn in do_input_reload accessed the pseudo in
question?
Thanks for any feedback on this problem,
Ulrich
--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2002-04-08 19:57 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-04-08 13:02 3.1 BUG: reload inheritance / frame pointer elimination Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).