Re: [RS6000] make PLT loads volatile
Hi! On Sat, Mar 14, 2020 at 09:30:02AM +1030, Alan Modra wrote: > On Fri, Mar 13, 2020 at 10:40:38AM -0500, Segher Boessenkool wrote: > > > Using a call-saved register to cache a load out of the PLT looks > > > really silly > > > > Who said anything about using call-saved registers? GCC will usually > > make a stack slot for this, and only use a non-volatile register when > > that is profitable. (I know it is a bit too aggressive with it, but > > that is a generic problem). > > Using a stack slot comes about due to hoisting then running out of > call-saved registers in the loop. Score another reason not to hoist > PLT loads. LRA can do this directly, without ever using call-saved registers. There are some other passes that can do rematerialisation as well. Not enough yet, but GCC does *not* use non-volatile registers to save values, unless it thinks that is cheaper (which it currently thinks too often, but that is a separate problem). > > > when the inline PLT call is turned back into a direct > > > call by the linker. > > > > Ah, so yeah, for direct calls we do not want this. I was thinking this > > was about indirect calls (via a bctrl that is), dunno how I got that > > misperception. Sorry. > > > > What is this like for indirect calls (at C level)? Does your patch do > > anything to those? > > No effect at all. To put your mind at rest on this point you can > verify quite easily by noticing that UNSPECV_PLT* is only generated in > rs6000_longcall_ref, and calls to that function are conditional on > GET_CODE (func_desc) == SYMBOL_REF. Could you please send a new patch (could be the same patch even) that is easier to review for me? With things like all of the above info in the message (describing the setting, the problem, and the solution). Or should I read the original message another ten times until it clicks? It certainly is possible this matter just is hard to grasp :-/ Thanks in advance, Segher
Re: [RS6000] make PLT loads volatile
On Fri, Mar 13, 2020 at 10:40:38AM -0500, Segher Boessenkool wrote: > Hi! > > On Fri, Mar 13, 2020 at 10:06:01AM +1030, Alan Modra wrote: > > On Thu, Mar 12, 2020 at 11:57:17AM -0500, Segher Boessenkool wrote: > > > On Thu, Mar 12, 2020 at 01:18:50PM +1030, Alan Modra wrote: > > > > With lazy PLT resolution the first load of a PLT entry may be a value > > > > pointing at a resolver stub. gcc's loop processing can result in the > > > > PLT load in inline PLT calls being hoisted out of a loop in the > > > > mistaken idea that this is an optimisation. It isn't. If the value > > > > hoisted was that for a resolver stub then every call to that function > > > > in the loop will go via the resolver, slowing things down quite > > > > dramatically. > > > > > > > > The PLT really is volatile, so teach gcc about that. > > > > > > It would be nice if we could keep it cached after it has been resolved > > > once, this has potential for regressing performance if we don't? And > > > LD_BIND_NOW should keep working just as fast as it is now, too? > > > > Using a call-saved register to cache a load out of the PLT looks > > really silly > > Who said anything about using call-saved registers? GCC will usually > make a stack slot for this, and only use a non-volatile register when > that is profitable. (I know it is a bit too aggressive with it, but > that is a generic problem). Using a stack slot comes about due to hoisting then running out of call-saved registers in the loop. Score another reason not to hoist PLT loads. > > when the inline PLT call is turned back into a direct > > call by the linker. > > Ah, so yeah, for direct calls we do not want this. I was thinking this > was about indirect calls (via a bctrl that is), dunno how I got that > misperception. Sorry. > > What is this like for indirect calls (at C level)? Does your patch do > anything to those? No effect at all. To put your mind at rest on this point you can verify quite easily by noticing that UNSPECV_PLT* is only generated in rs6000_longcall_ref, and calls to that function are conditional on GET_CODE (func_desc) == SYMBOL_REF. -- Alan Modra Australia Development Lab, IBM
Re: [RS6000] make PLT loads volatile
Hi! On Fri, Mar 13, 2020 at 10:06:01AM +1030, Alan Modra wrote: > On Thu, Mar 12, 2020 at 11:57:17AM -0500, Segher Boessenkool wrote: > > On Thu, Mar 12, 2020 at 01:18:50PM +1030, Alan Modra wrote: > > > With lazy PLT resolution the first load of a PLT entry may be a value > > > pointing at a resolver stub. gcc's loop processing can result in the > > > PLT load in inline PLT calls being hoisted out of a loop in the > > > mistaken idea that this is an optimisation. It isn't. If the value > > > hoisted was that for a resolver stub then every call to that function > > > in the loop will go via the resolver, slowing things down quite > > > dramatically. > > > > > > The PLT really is volatile, so teach gcc about that. > > > > It would be nice if we could keep it cached after it has been resolved > > once, this has potential for regressing performance if we don't? And > > LD_BIND_NOW should keep working just as fast as it is now, too? > > Using a call-saved register to cache a load out of the PLT looks > really silly Who said anything about using call-saved registers? GCC will usually make a stack slot for this, and only use a non-volatile register when that is profitable. (I know it is a bit too aggressive with it, but that is a generic problem). > when the inline PLT call is turned back into a direct > call by the linker. Ah, so yeah, for direct calls we do not want this. I was thinking this was about indirect calls (via a bctrl that is), dunno how I got that misperception. Sorry. What is this like for indirect calls (at C level)? Does your patch do anything to those? Segher
Re: [RS6000] make PLT loads volatile
On Thu, Mar 12, 2020 at 11:57:17AM -0500, Segher Boessenkool wrote: > Hi! > > On Thu, Mar 12, 2020 at 01:18:50PM +1030, Alan Modra wrote: > > With lazy PLT resolution the first load of a PLT entry may be a value > > pointing at a resolver stub. gcc's loop processing can result in the > > PLT load in inline PLT calls being hoisted out of a loop in the > > mistaken idea that this is an optimisation. It isn't. If the value > > hoisted was that for a resolver stub then every call to that function > > in the loop will go via the resolver, slowing things down quite > > dramatically. > > > > The PLT really is volatile, so teach gcc about that. > > It would be nice if we could keep it cached after it has been resolved > once, this has potential for regressing performance if we don't? And > LD_BIND_NOW should keep working just as fast as it is now, too? Using a call-saved register to cache a load out of the PLT looks really silly when the inline PLT call is turned back into a direct call by the linker. You end up with an unnecessary save and restore of the register, plus copies from the register to r12. What's the chance of someone reporting that as a gcc "bug"? :-) Then there's the possibility that shortening the number of instructions between two calls of a small function runs into stalls. How can we teach gcc about these unknowns? ie. How to weight use of a call-saved register to cache PLT loads against other possible uses of that register in a loop? It's quite likely not a good use, even when gcc knows the PLT entry has been resolved.. Which means some gcc infrastructure would be needed to do this sensibly and without the necessary infrastructure, I think gcc hoisting a PLT load out of a loop should never be done. -- Alan Modra Australia Development Lab, IBM
Re: [RS6000] make PLT loads volatile
Hi! On Thu, Mar 12, 2020 at 01:18:50PM +1030, Alan Modra wrote: > With lazy PLT resolution the first load of a PLT entry may be a value > pointing at a resolver stub. gcc's loop processing can result in the > PLT load in inline PLT calls being hoisted out of a loop in the > mistaken idea that this is an optimisation. It isn't. If the value > hoisted was that for a resolver stub then every call to that function > in the loop will go via the resolver, slowing things down quite > dramatically. > > The PLT really is volatile, so teach gcc about that. It would be nice if we could keep it cached after it has been resolved once, this has potential for regressing performance if we don't? And LD_BIND_NOW should keep working just as fast as it is now, too? Segher
