diff mbox

[AArch64] Use builtins for fpcr/fpsr

Message ID DB6PR0801MB20535E76ABF67E12E616C0F083F20@DB6PR0801MB2053.eurprd08.prod.outlook.com
State New, archived
Headers

Commit Message

Wilco Dijkstra Feb. 9, 2018, 3:03 p.m. UTC 
  ping


From: Wilco Dijkstra
Sent: 19 January 2018 13:18
To: Szabolcs Nagy; libc-alpha@sourceware.org
Cc: nd
Subject: Re: [PATCH][AArch64] Use builtins for fpcr/fpsr
  

Szabolcs Nagy wrote:

> this will have to wait for the next release, but please
> increase the gcc prereq to 6.0 because i see ice on gcc-5:

aarch64-none-linux-gnu-gcc ../sysdeps/aarch64/fpu/fesetenv.c -c [..]
../sysdeps/aarch64/fpu/fesetenv.c: In function '__fesetenv':
../sysdeps/aarch64/fpu/fesetenv.c:75:1: error: unrecognizable insn:
 }
 ^
(insn 23 22 4 6 (unspec_volatile [
            (mem:SI (plus:DI (reg/v/f:DI 85 [ envp ])
                    (const_int 4 [0x4])) [2 envp_8(D)->__fpsr+0 S4 A32])
        ] UNSPECV_SET_FPSR) ../sysdeps/aarch64/fpu/fesetenv.c:41 -1
     (nil))
../sysdeps/aarch64/fpu/fesetenv.c:75:1: internal compiler error: in extract_insn, at recog.c:2343

Looks like it's merging a MEM into a register operand. Since GCC5 is no longer
supported I've updated it to GCC6:

Since GCC has support for accessing FPSR/FPCR, use them when possible
so that the asm instructions can be removed eventually.  Although GCC 5
supports the builtins, it has an optimization bug, so use them from GCC 6
onwards.

GLIBC build and test OK.

ChangeLog:
2018-01-19  Wilco Dijkstra  <wdijkstr@arm.com>

        * sysdeps/aarch64/fpu/fpu_control.h: Use builtins for accessing FPCR/FPSR.

Comments

Szabolcs Nagy Feb. 9, 2018, 4:54 p.m. UTC | #1 
On 09/02/18 15:03, Wilco Dijkstra wrote:
> Since GCC has support for accessing FPSR/FPCR, use them when possible
> so that the asm instructions can be removed eventually.  Although GCC 5
> supports the builtins, it has an optimization bug, so use them from GCC 6
> onwards.
> 
> GLIBC build and test OK.
> 
> ChangeLog:
> 2018-01-19  Wilco Dijkstra  <wdijkstr@arm.com>
> 
>          * sysdeps/aarch64/fpu/fpu_control.h: Use builtins for accessing FPCR/FPSR.
> 

ok to commit.
Joseph Myers Feb. 9, 2018, 10:34 p.m. UTC | #2 
I'm seeing:

In file included from ../include/fpu_control.h:2:0,
                 from test-fpucw.c:19:
../sysdeps/aarch64/fpu/fpu_control.h:24:5: error: "__GNUC_PREREQ" is not defined, evaluates to 0 [-Werror=undef]
 #if __GNUC_PREREQ (6,0)
     ^~~~~~~~~~~~~
../sysdeps/aarch64/fpu/fpu_control.h:24:19: error: missing binary operator before token "("
 #if __GNUC_PREREQ (6,0)
                   ^

fpu_control.h is missing an include of features.h, as needed to use 
__GNUC_PREREQ.
Wilco Dijkstra Feb. 10, 2018, 3:56 p.m. UTC | #3 
Joseph Myers wrote:
  
In file included from ../include/fpu_control.h:2:0,
                 from test-fpucw.c:19:
../sysdeps/aarch64/fpu/fpu_control.h:24:5: error: "__GNUC_PREREQ" is not defined, evaluates to 0 [-Werror=undef]

> fpu_control.h is missing an include of features.h, as needed to use 
> __GNUC_PREREQ.

Yes, I added the include but forgot to merge it with the patch. Now committed.

Wilco
Joseph Myers Feb. 12, 2018, 2:02 p.m. UTC | #4 
On Sat, 10 Feb 2018, Wilco Dijkstra wrote:

> Joseph Myers wrote:
>   
> In file included from ../include/fpu_control.h:2:0,
>                  from test-fpucw.c:19:
> ../sysdeps/aarch64/fpu/fpu_control.h:24:5: error: "__GNUC_PREREQ" is not defined, evaluates to 0 [-Werror=undef]
> 
> > fpu_control.h is missing an include of features.h, as needed to use 
> > __GNUC_PREREQ.
> 
> Yes, I added the include but forgot to merge it with the patch. Now committed.

Normal practice in glibc installed headers (and more generally in glibc) 
is to use #include <>, not #include "" as in that change.
diff mbox

Patch

diff --git a/sysdeps/aarch64/fpu/fpu_control.h b/sysdeps/aarch64/fpu/fpu_control.h
index 570e3dca78adbbccdc21ca879c582e1e09196f2d..d0cc5afc9faf42249a45b7f6b24a374f944998fd 100644
--- a/sysdeps/aarch64/fpu/fpu_control.h
+++ b/sysdeps/aarch64/fpu/fpu_control.h
@@ -21,17 +21,24 @@ 
 
 /* Macros for accessing the FPCR and FPSR.  */
 
-#define _FPU_GETCW(fpcr) \
+#if __GNUC_PREREQ (6,0)
+# define _FPU_GETCW(fpcr) (fpcr = __builtin_aarch64_get_fpcr ())
+# define _FPU_SETCW(fpcr) __builtin_aarch64_set_fpcr (fpcr)
+# define _FPU_GETFPSR(fpsr) (fpsr = __builtin_aarch64_get_fpsr ())
+# define _FPU_SETFPSR(fpsr) __builtin_aarch64_set_fpsr (fpsr)
+#else
+# define _FPU_GETCW(fpcr) \
   __asm__ __volatile__ ("mrs   %0, fpcr" : "=r" (fpcr))
 
-#define _FPU_SETCW(fpcr) \
+# define _FPU_SETCW(fpcr) \
   __asm__ __volatile__ ("msr   fpcr, %0" : : "r" (fpcr))
 
-#define _FPU_GETFPSR(fpsr) \
+# define _FPU_GETFPSR(fpsr) \
   __asm__ __volatile__ ("mrs   %0, fpsr" : "=r" (fpsr))
 
-#define _FPU_SETFPSR(fpsr) \
+# define _FPU_SETFPSR(fpsr) \
   __asm__ __volatile__ ("msr   fpsr, %0" : : "r" (fpsr))
+#endif
 
 /* Reserved bits should be preserved when modifying register
    contents. These two masks indicate which bits in each of FPCR and