This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] Add volatiles for x86-64 bits/mathinline.h
On Thu, May 3, 2012 at 8:15 AM, Andreas Jaeger <firstname.lastname@example.org> 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
One minor bike-shed below...
> 2012-05-02 ?Andreas Jaeger ?<email@example.com>
> ? ? ? ?[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.