Hi,
this is a regression present on the 4.6 branch at -O2 for the SPARC, but the
underlying issue is presumably latent everywhere. It's reload inheritance so
the opinion of reload specialists is welcome.
We have a couple of insns with 2 reloads each:
Reloads for insn # 84
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 30 %fp)
(const_int -6140
[0xffffffffffffe804]))
GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine
reload_in_reg: (plus:SI (reg/f:SI 30 %fp)
(const_int -6140
[0xffffffffffffe804]))
reload_reg_rtx: (reg:SI 11 %o3)
Reload 1: reload_in (SI) = (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 2)
reload_in_reg: (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
reload_reg_rtx: (reg:SI 11 %o3)
Reloads for insn # 85
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 30 %fp)
(const_int -6140
[0xffffffffffffe804]))
GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 0), can't combine
reload_in_reg: (plus:SI (reg/f:SI 30 %fp)
(const_int -6140
[0xffffffffffffe804]))
reload_reg_rtx: (reg:SI 11 %o3)
Reload 1: reload_in (SI) = (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 0)
reload_in_reg: (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
reload_reg_rtx: (reg:SI 11 %o3)
Reload 1 of insn #85 inherits the reload reg from reload 1 of insn #84, the
bug being that the same reload reg is also used for reload 0 of insn #85.
This is supposed to work like so: the inheritance code in choose_reload_regs
calls free_for_value_p with IGNORE_ADDRESS_RELOADS set to 1 to bypass the
conflict with reload 0 because there is special code in the same function to
discard reload 0 in this case:
/* If we can inherit a RELOAD_FOR_INPUT, or can use a
reload_override_in, then we do not need its related
RELOAD_FOR_INPUT_ADDRESS / RELOAD_FOR_INPADDR_ADDRESS reloads;
likewise for other reload types.
We handle this by removing a reload when its only replacement
is mentioned in reload_in of the reload we are going to inherit.
A special case are auto_inc expressions; even if the input is
inherited, we still need the address for the output. We can
recognize them because they have RELOAD_OUT set to RELOAD_IN.
If we succeeded removing some reload and we are doing a preliminary
pass just to remove such reloads, make another pass, since the
removal of one reload might allow us to inherit another one. */
else if (rld[r].in
&& rld[r].out != rld[r].in
&& remove_address_replacements (rld[r].in) && pass)
pass = 2;
But it is called with rld[r].in equal to:
(gdb) p debug_rtx(in_rtx)
(reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
and the following replacement for reload 0:
(gdb) p debug_rtx(*replacements[0].where)
(plus:SI (reg/f:SI 30 %fp)
(const_int -6140 [0xffffffffffffe804]))
so loc_mentioned_in_p returns false and reload 0 isn't discarded.
The problem is that reload 0 is pushed because of SECONDARY_MEMORY_NEEDED,
i.e. (reg:SI 40 %f8) is spilled to memory at the address
(gdb) p debug_rtx(secondary_memlocs_elim[11][0])
(mem/c:SI (plus:SI (reg/f:SI 30 %fp)
(const_int -6140 [0xffffffffffffe804])) [0 S4 A32])
and replacements[0].where above points to within this MEM instead of IN_RTX.
The attached patch fixes the problem (on the 4.6 branch) by also invoking
remove_address_replacements on the result of get_secondary_mem if a secondary
memory is needed. The condition to detect it has been copied from push_reload
on the 4.6 branch, but it has already slightly changed on the mainline and I'm
not sure how to adjust it.
Bootstrapped/regtested on x86_64-suse-linux. Does that look plausible? Do we
want to fix this on release branches as well?
2012-09-02 Eric Botcazou <ebotca...@adacore.com>
PR rtl-optimization/54290
* reload1.c (choose_reload_regs): Also take into account secondary MEMs
to remove address replacements for inherited reloads.
--
Eric Botcazou
Index: reload1.c
===================================================================
--- reload1.c (revision 190664)
+++ reload1.c (working copy)
@@ -6977,10 +6977,27 @@ choose_reload_regs (struct insn_chain *c
If we succeeded removing some reload and we are doing a preliminary
pass just to remove such reloads, make another pass, since the
removal of one reload might allow us to inherit another one. */
- else if (rld[r].in
+ else if (pass
+ && rld[r].in
&& rld[r].out != rld[r].in
- && remove_address_replacements (rld[r].in) && pass)
+ && remove_address_replacements (rld[r].in))
pass = 2;
+#ifdef SECONDARY_MEMORY_NEEDED
+ else if (pass
+ && rld[r].in
+ && rld[r].out != rld[r].in
+ && (REG_P (rld[r].in)
+ || (GET_CODE (rld[r].in) == SUBREG
+ && REG_P (SUBREG_REG (rld[r].in))))
+ && reg_or_subregno (rld[r].in) < FIRST_PSEUDO_REGISTER
+ && SECONDARY_MEMORY_NEEDED
+ (REGNO_REG_CLASS (reg_or_subregno (rld[r].in)),
+ rld[r].rclass, rld[r].inmode)
+ && remove_address_replacements
+ (get_secondary_mem (rld[r].in, rld[r].inmode,
+ rld[r].opnum, rld[r].when_needed)))
+ pass = 2;
+#endif
}
}
