On 2/2/2022 8:16 AM, Richard Sandiford via Gcc-patches wrote:
Richard Sandiford <richard.sandif...@arm.com> writes:
Hans-Peter Nilsson <h...@axis.com> writes:
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.
:-) LGTM, but I'd like to hear Jeff's opinion.
So it would be a good idea if I used the right email address.
:-) I still got a copy as I'm subscribed to gcc-patches. I've just
been incredibly busy since the end of Jan.
I think y'all are right. Go ahead and commit the comment. I don't
think there's an active proposal to remove reload[1].cc before gcc-12,
but I would hope we can remove it shortly thereafter.
jeff
