Message ID | DB6PR0801MB20530C40FDB2D3561E6728BC83100@DB6PR0801MB2053.eurprd08.prod.outlook.com |
---|---|
State | New |
Headers | show |
Series | [AArch64] Use builtins for fpcr/fpsr | expand |
On 09/01/18 11:38, Wilco Dijkstra wrote: > Since GCC 5 has builtin support for accessing FPSR/FPCR, use them when possible > so that the asm instructions can be removed eventually. > > GLIBC build and test OK. > 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
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. 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
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. 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
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.
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.
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
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 --git a/sysdeps/aarch64/fpu/fpu_control.h b/sysdeps/aarch64/fpu/fpu_control.h index 570e3dca78adbbccdc21ca879c582e1e09196f2d..d7ed28e3c1d7bd4a89087604f80268c36c890c80 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 (5,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