On 4/5/22 5:32 PM, Segher Boessenkool wrote:
> On Tue, Apr 05, 2022 at 05:06:50PM -0500, Peter Bergner wrote:
>>
>> + /* Handle longcall attributes. */
>> + if ((INTVAL (cookie) & CALL_LONG) != 0
>> + && GET_CODE (func_desc) == SYMBOL_REF)
>
> Don't say "!= 0" here please.
>
> if (INTVAL (cookie) & CALL_LONG && SYMBOL_REF_P (func_desc))
Yes, cut & paste. I'll change it.
>> + {
>> + /* Only PCREL can do a sibling call to a longcall function
>> + because we don't need to restore the TOC register. */
>
> Don't say "Only", it is the "New" syndrome: it is out of date before you
> know it :-( You can just leave it away here.
Ok, I can drop "Only" and keep the rest the same.
>> + gcc_assert (rs6000_pcrel_p ());
>> + func_desc = rs6000_longcall_ref (func_desc, tlsarg);
>> + }
>> + else
>> + gcc_assert (INTVAL (cookie) == 0);
>
> So in the old code the cookie could *only* contain the CALL_LONG flag,
> now it can contain any others as long as it has that flag as well.
> Please fix.
No, the old code only allowed INTVAL (cookie) == 0, which means no
attributes are allowed. The new code now allows the CALL_LONG attribute
iff the function is a SYMBOL_REF. This is only allowed for pcrel calls
though. I debated on whether to do a gcc_assert on rs6000_pcrel_p() or
fold the rs6000_pcrel_p() into the if () and let the original assert
on INTVAL (cookie) == 0 catch the illegal uses. It's up to you on
what you prefer.
> Not every LONG_CALL needs a TOC restore though?
I believe if the function we're calling has the CALL_LONG attribute
set, we have to assume that the TOC needs to be restored. Now whether
the actual callee's TOC at run time is different or not from the callers
is another question. We're just forced to assume they are different.
> I won't ask you to make it work in those cases of course, but change
> the comment to not be as misleading? Taking the "Only" out is good
> already I think :-)
Yes, as I said above, I will drop "Only" from the comment.
> You probably should have the same condition here as actually doing a
> longcall as well, something involving SYMBOL_REF_FUNCTION_P?
I believe if we're here in rs6000_sibcall_aix() and func_desc is a
SYMBOL_REF, then it also must be SYMBOL_REF_FUNCTION_P, correct?
Otherwise, why would we be attempting to do a sibcall to it?
Peter
