This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add volatiles for x86-64 bits/mathinline.h


On Thu, May 3, 2012 at 8:15 AM, Andreas Jaeger <aj@suse.de> wrote:
>> AIUI the issue is that these asms depend on the state of the fpsr, but
>> the compiler doesn't know that and so eliminates a second instance
>> after an fpsr change as redundant with the first. ?So what would be
>> really right is to tell the compiler that the fpsr is an input to these
>> asms and an output of the mode-changing code.
>
> Yes, indeed - but that's not possible with GCC.
>
> Here's a patch with comments. Ok to commit ?

I reviewed the gcc bug, the original patch, Roland's comments, and the
new patch.

One minor bike-shed below...

> Andreas
>
> 2012-05-02 ?Andreas Jaeger ?<aj@suse.de>
>
> ? ? ? ?[BZ #14053]
> ? ? ? ?* sysdeps/x86_64/fpu/bits/mathinline.h (lrintf): Add __volatile to asm.
> ? ? ? ?(lrint): Likewise.
> ? ? ? ?(llrintf): Likewise.
> ? ? ? ?(llrint): Likewise.
> ? ? ? ?(rint): Likewise.
> ? ? ? ?(rintf): Likewise.
> ? ? ? ?(nearbyint): Likewise.
> ? ? ? ?(nearbyintf): Likewise.
>
> diff --git a/sysdeps/x86_64/fpu/bits/mathinline.h b/sysdeps/x86_64/fpu/bits/mathinline.h
> index c072f16..a0c2bc1 100644
> --- a/sysdeps/x86_64/fpu/bits/mathinline.h
> +++ b/sysdeps/x86_64/fpu/bits/mathinline.h
> @@ -79,7 +79,11 @@ __MATH_INLINE long int
> ?__NTH (lrintf (float __x))
> ?{
> ? long int __res;
> - ?__asm ("cvtss2si %1, %0" : "=r" (__res) : "xm" (__x));
> + ?/* Mark as volatile since the result is dependend on the state of
> + ? ? the SSE control register (the rounding mode). Otherwise GCC might
> + ? ? remove these assembler instructions since it does not know about
> + ? ? the rounding mode change. ?*/

"the rounding mode change and can't currently be told.  */"

It's not our fault. If this ever changes then a future maintainer will
know why we did this and adjust the code. I know you plan to fix this
tomorrow but tomorrow may never come. It's not morbid, it's just the
reality of life :-)

OK with that change.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]