On Tue, Apr 05, 2022 at 10:33:14PM -0500, Peter Bergner wrote:
> On 4/5/22 5:32 PM, Segher Boessenkool wrote:
> >> +      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.

Ah, tricky.

>  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.

For future changes, likely it is best if you split the pcrel and
non-pcrel paths further.

> > 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.

Not if we know the called function is in the same object?  If we are
doing long calls anyway there isn't much point in optimising anything
anymore, but don't say "have to" or such then :-)

> > 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?

It doesn't hurt to be a bit defensive in programming.  It helps making
future changes easier, too :-)

Maybe we should have a utility function for this?  That helps preventing
microoptimisations as well :-P


Segher

Reply via email to