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

[Carlos O'Donell]

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.