I'm not seriously submitting this patch for approval. I just thought
it'd be interesting to some people, at least those maintaining ports
still using reload; I know it's reload and major ports don't really
care about that anymore. TL;DR: scroll down for the mystery part.
The *benevolent* effects of this bug had me confused for a while: I
thought I'd see neutral or good effects of nominally *improving* the
register-class topology of the CRIS port such that there's "a class
for the union of two classes whenever some instruction allows both
classes". Essentially, IRA picked (still does, not committed yet) two
classes GENNONACR_REGS and SPECIAL_REGS and SPEC_GENNONACR_REGS, their
union, but the move-pattern alternatives corresponding to
GENNONACR_REGS are actually "r" (GENERAL_REGS), which also contains
the ACR register. I want to get rid of all things related to ACR and
simplify the port, as ACR is an artefact of a no-longer-supported CRIS
variant (see commit r11-220 which also mentions this remaining
artefact).
But, when improving the topology, instead of still making good use of
registers from SPECIAL_REGS where the reload pass matters, registers
from GENERAL_REGS are always preferred. Though, for coremark, this
only happens in two printf-formatting functions of newlib; _vfprintf_r
and _vfiprintf_r (same code actually, and it's likely a bug that both
instances get included).
Anyway, skipping to the point: the effect of the bug is that
alternatives corresponding to classes that are *not subclasses* of the
preferred class (the union), are demoted by 4 (or actually: 2 + 2 *
pref_or_nothing[i]), instead of those register alternatives where the
register-class *intersection is empty*, as is the intent according to
my reading of the comment. For CRIS, this means that GENERAL_REGS get
demoted (as ACR is present there but not in SPEC_GENNONACR_REGS) and
then SPECIAL_REGS are preferred, which is *sometimes* better as those
registers are "free". They're not used in regular code and outside
their special purposes, basically just usable for moving around data;
perfect for plain single-use spill-reloads. Net result: for CRIS the
bug causes a slightly smaller coremark binary but the same execution
time.
With the CRIS register-class topology fixed and the bug no longer
having effect, I have to adjust the port such that SPECIAL_REGS are
preferred when that helps (some heuristics required), as the (current)
order of alternatives for the *movsi_internal pattern causes
preference of GENERAL_REGS over SPECIAL_REGS when the cost is the
same. IOW, a flaw in the port masked by a bug in reload.
The mystery isn't so much that there's code mismatching comments or
intent, but that this code has been there "forever". There has been a
function reg_classes_intersect_p, in gcc since r0-54, even *before*
there was reload.c; r0-361, so why the "we don't have a way of forming
the intersection" in the actual patch and why wasn't this fixed
(perhaps not even noticed) when reload was state-of-the-art?
(I know the file has been renamed but again this isn't an patch
intended to be applied. The context is adjusted to include the
comment. The ChangeLog entry is the effect of a habitual twitch. 8-]
)
gcc:
* reload.c (find_reloads): Call reg_classes_intersect_p, not
reg_class_subset_p.
---
gcc/reload.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/gcc/reload.c b/gcc/reload.c
index 4c55ca58a5f7..36e887c6b57e 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -3616,43 +3616,39 @@ find_reloads (rtx_insn *insn, int replace, int
ind_levels, int live_known,
/* If this operand is a pseudo register that didn't get
a hard reg and this alternative accepts some
register, see if the class that we want is a subset
of the preferred class for this register. If not,
but it intersects that class, use the preferred class
instead. If it does not intersect the preferred
class, show that usage of this alternative should be
discouraged; it will be discouraged more still if the
register is `preferred or nothing'. We do this
because it increases the chance of reusing our spill
register in a later insn and avoiding a pair of
memory stores and loads.
Don't bother with this if this alternative will
accept this operand.
Don't do this for a multiword operand, since it is
only a small win and has the risk of requiring more
spill registers, which could cause a large loss.
Don't do this if the preferred class has only one
register because we might otherwise exhaust the
class. */
if (! win && ! did_match
&& this_alternative[i] != NO_REGS
&& known_le (GET_MODE_SIZE (operand_mode[i]), UNITS_PER_WORD)
&& reg_class_size [(int) preferred_class[i]] > 0
&& ! small_register_class_p (preferred_class[i]))
{
if (! reg_class_subset_p (this_alternative[i],
preferred_class[i]))
{
- /* Since we don't have a way of forming the intersection,
- we just do something special if the preferred class
- is a subset of the class we have; that's the most
- common case anyway. */
- if (reg_class_subset_p (preferred_class[i],
- this_alternative[i]))
+ if (reg_classes_intersect_p (preferred_class[i],
+ this_alternative[i]))
this_alternative[i] = preferred_class[i];
else
reject += (2 + 2 * pref_or_nothing[i]);
}
--
2.30.2
brgds, H-P
