Re: [PATCH] Save fp registers on x64 function resolution.

[Rich Felker <dalias at aerifal dot cx>]

On Fri, Jul 26, 2013 at 05:37:06PM +0200, Andreas Jaeger wrote:
> On 07/26/2013 11:15 AM, OndÅej BÃlka wrote:
> > Hi, as having to manually save xmm registers causes many issues recently
> > (memset issues, bug 15786...) this patch save xmm registers. If you
> > accept it to 2.19 further cleanups will follow. 
> 
> The question here is whether we state that the resolver and any
> functions it calls - including IFUNC resolvers - are allowed to touch
> xmm registers or not. In the past, we were fine with not touching xmm
> registers - but now with IFUNC, it's for sure safer to save xmm as well.
> But it comes with a cost.

It was never fine/safe. If a register is call-clobbered, any compiler
claiming conformance to the psABI is free to generate instructions
that clobber this register as part of its code generation. The only
"safety" before came from invalid assumptions about GCC's code
generation, assumptions which may prove false in the future and make
it impossible to build correctly-working old versions of glibc with
newer compilers in the future.

> At minimum, we should document what IFUNCs are allowed to do and what not.

This would be an impossible requirement for programmers using IFUNCs
to satisfy, unless you want to mandate that IFUNCs be written in pure
assembly, as the programmer has no control over whether the compiler
generates instructions that would clobber the registers.

> > We could also add register saving for other architectures.
> > 
> > As performance is concerned not saving registers looks like saving at
> > wrong place. It causes dl_fixup code not to use sse functions variants
> > that could have bigger slowdown than what was saved by not saving
> > registers.
> > 
> > I do not have measurements yet, it would need to add rdtsc to _dl_fixup
> > as it is and _dl_fixup with rtld-*.S, -mno-sse and other hacks removed.
> > 
> > Comments?
> 
> Regarding the code itself:
> We already do a "subq $56,%rsp" to reserve stack-space, so let's combine

I agree.

> those with the subq128 you have. This also deserves a comment why this
> is needed.

How is it not self-explanatory? The code just above is already backing
up some of the argument registers because they'll be clobbered in the
resolver. It was just failing to backup the rest of the argument
registers.

If a comment is needed, it should simply be along the lines of: "Any
register that may be used to pass arguments must be backed up before
calling the resolver, as it may contain an argument value to be passed
to the resolved function, and the resolver may clobber the register as
part of its execution."

Rich