> From: Richard Sandiford <richard.sandif...@arm.com>
> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > 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?
>
> But doesn't the comment
(the second, patched comment; removed in the patch)
> mean that we have/had no way of getting
> a *class* that is the intersection of preferred_class[i] and
> this_alternative[i], to store as the new
> this_alternative[i]?
Yes, that's likely what's going on in the (second) comment
and code; needing a s/intersection/a class for the
intersection/, but the *first* comment is pretty clear that
the intent is exactly to "override" this_alternative[i]: "If
not (a subclass), but it intersects that class, use the
preferred class instead". But of course, that doesn't
actually have to make sense as code! And indeed it doesn't,
as you say.
> If the alternatives were register sets rather than classes,
> I think the intended effect would be:
>
> this_alternative[i] &= preferred_class[i];
>
> (i.e. AND_HARD_REG_SET in old money).
>
> It looks like the patch would instead make this_alternative[i] include
> registers that the alternative doesn't actually accept, which feels odd.
Perhaps I put too much trust in the sanity of old comments.
How about I actually commit this one instead? Better get it
right before reload is removed.
8< ------- >8
"reload: Adjust comment in find_reloads about subset, not intersection"
gcc:
* reload.cc (find_reloads): Align comment with code where
considering the intersection of register classes then tweaking the
regclass for the current alternative or rejecting it.
---
gcc/reload.cc | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/gcc/reload.cc b/gcc/reload.cc
index 664082a533d9..3ed901e39447 100644
--- a/gcc/reload.cc
+++ b/gcc/reload.cc
@@ -3635,9 +3635,11 @@ find_reloads (rtx_insn *insn, int replace, int
ind_levels, int live_known,
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
+ but it intersects that class, we'd like to use the
+ intersection, but the best we can do is to use the
+ preferred class, if it is instead a subset of the
+ class we want in this alternative. If we can't use
+ it, 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
@@ -3664,9 +3666,10 @@ find_reloads (rtx_insn *insn, int replace, int
ind_levels, int live_known,
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
+ /* Since we don't have a way of forming a register
+ class for 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]))
--
2.30.2
