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.
>
> Thanks,
> Richard
>
>> 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]))
