Hi!
On Tue, Apr 05, 2022 at 05:06:50PM -0500, Peter Bergner wrote:
> Before PCREL in POWER10, we were not allowed to perform sibcalls to longcall
> functions since callee's return would skip the TOC restore in the caller.
> However, with PCREL we can now safely perform a sibling call to longcall
> functions. The problem with the current code in rs6000_sibcall_aix is that it
> asserts we do not have a longcall and fixing that, it generates a direct call
> to a PLT stub, when it should generate an indirect sibcall due to -fno-plt.
> The solution here is to check for a pcrel longcall and emit an indirect
> sibcall
> using an inline plt stub in that case.
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -25659,11 +25659,21 @@ rs6000_sibcall_aix (rtx value, rtx func_desc, rtx
> tlsarg, rtx cookie)
> rtx r12 = NULL_RTX;
> rtx func_addr = func_desc;
>
> - gcc_assert (INTVAL (cookie) == 0);
> -
> if (global_tlsarg)
> tlsarg = global_tlsarg;
>
> + /* 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))
You can put parens around that "A & B" if you don't trust the reader (or
the compiler ;-) ) to know the precedence rules
(Hrm, you just c'n'p-ed this, but :-) )
> + {
> + /* 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.
> + 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.
Not every LONG_CALL needs a TOC restore though? 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 :-)
You probably should have the same condition here as actually doing a
longcall as well, something involving SYMBOL_REF_FUNCTION_P?
Thanks,
Segher
