| Message ID | 20170322104630.z7g7nk6sv2xdzhad@e107464-lin.cambridge.arm.com |
|---|---|
| State | New |
| Headers | show |
On 22/03/2017 10:46:30, Prakhar Bahuguna wrote: > The GCC documentation in section 6.60.8 ARM Floating Point Status and Control > Intrinsics states that the FPSCR register can be read and written to using the > intrinsics __builtin_arm_get_fpscr and __builtin_arm_set_fpscr. However, these > are misnamed within GCC itself and these intrinsic names are not recognised. > This patch corrects the intrinsic names to match the documentation, and adds > tests to verify these intrinsics generate the correct instructions. > > Testing done: Ran regression tests on arm-none-eabi for Cortex-M4. > > 2017-03-09 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > gcc/ChangeLog: > > * gcc/config/arm/arm-builtins.c (arm_init_builtins): Rename > __builtin_arm_ldfscr to __builtin_arm_get_fpscr, and rename > __builtin_arm_stfscr to __builtin_arm_set_fpscr. > * gcc/testsuite/gcc.target/arm/fpscr.c: New file. > > Okay for stage 1? > > -- > > Prakhar Bahuguna Bumping, could I get a review for this please? Thanks, -- Prakhar Bahuguna
Hi Prakhar, Sorry for the delay, On 22/03/17 10:46, Prakhar Bahuguna wrote: > The GCC documentation in section 6.60.8 ARM Floating Point Status and Control > Intrinsics states that the FPSCR register can be read and written to using the > intrinsics __builtin_arm_get_fpscr and __builtin_arm_set_fpscr. However, these > are misnamed within GCC itself and these intrinsic names are not recognised. > This patch corrects the intrinsic names to match the documentation, and adds > tests to verify these intrinsics generate the correct instructions. > > Testing done: Ran regression tests on arm-none-eabi for Cortex-M4. > > 2017-03-09 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > gcc/ChangeLog: > > * gcc/config/arm/arm-builtins.c (arm_init_builtins): Rename > __builtin_arm_ldfscr to __builtin_arm_get_fpscr, and rename > __builtin_arm_stfscr to __builtin_arm_set_fpscr. > * gcc/testsuite/gcc.target/arm/fpscr.c: New file. > > Okay for stage 1? I see that the mistake was in not addressing one of the review comments in: https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01832.html properly in the patch that added these functions :( This is ok for stage 1 if a bootstrap and test on arm-none-linux-gnueabihf works fine I don't think we want to maintain the __builtin_arm_[ld,st]fscr names for backwards compatibility as they were not documented and are __builtin_arm* functions that we don't guarantee to maintain. Thanks, Kyrill > -- > > Prakhar Bahuguna
Hi, We have decided to apply the following patch to the embedded-6-branch to fix naming of an ARM intrinsic. ChangeLog entry is as follows: 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> gcc/ * gcc/config/arm/arm-builtins.c (arm_init_builtins): Rename __builtin_arm_ldfscr to __builtin_arm_get_fpscr, and rename __builtin_arm_stfscr to __builtin_arm_set_fpscr. gcc/testsuite/ * gcc.target/arm/fpscr.c: New file. Best regards, Thomas Hi Prakhar, Sorry for the delay, On 22/03/17 10:46, Prakhar Bahuguna wrote: > The GCC documentation in section 6.60.8 ARM Floating Point Status and Control > Intrinsics states that the FPSCR register can be read and written to using the > intrinsics __builtin_arm_get_fpscr and __builtin_arm_set_fpscr. However, these > are misnamed within GCC itself and these intrinsic names are not recognised. > This patch corrects the intrinsic names to match the documentation, and adds > tests to verify these intrinsics generate the correct instructions. > > Testing done: Ran regression tests on arm-none-eabi for Cortex-M4. > > 2017-03-09 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > gcc/ChangeLog: > > * gcc/config/arm/arm-builtins.c (arm_init_builtins): Rename > __builtin_arm_ldfscr to __builtin_arm_get_fpscr, and rename > __builtin_arm_stfscr to __builtin_arm_set_fpscr. > * gcc/testsuite/gcc.target/arm/fpscr.c: New file. > > Okay for stage 1? I see that the mistake was in not addressing one of the review comments in: https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01832.html properly in the patch that added these functions :( This is ok for stage 1 if a bootstrap and test on arm-none-linux-gnueabihf works fine I don't think we want to maintain the __builtin_arm_[ld,st]fscr names for backwards compatibility as they were not documented and are __builtin_arm* functions that we don't guarantee to maintain. Thanks, Kyrill > -- > > Prakhar Bahuguna
diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index ca622519b7d..aef05d0127f 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -1860,10 +1860,10 @@ arm_init_builtins (void) = build_function_type_list (unsigned_type_node, NULL); arm_builtin_decls[ARM_BUILTIN_GET_FPSCR] - = add_builtin_function ("__builtin_arm_ldfscr", ftype_get_fpscr, + = add_builtin_function ("__builtin_arm_get_fpscr", ftype_get_fpscr, ARM_BUILTIN_GET_FPSCR, BUILT_IN_MD, NULL, NULL_TREE); arm_builtin_decls[ARM_BUILTIN_SET_FPSCR] - = add_builtin_function ("__builtin_arm_stfscr", ftype_set_fpscr, + = add_builtin_function ("__builtin_arm_set_fpscr", ftype_set_fpscr, ARM_BUILTIN_SET_FPSCR, BUILT_IN_MD, NULL, NULL_TREE); } diff --git a/gcc/testsuite/gcc.target/arm/fpscr.c b/gcc/testsuite/gcc.target/arm/fpscr.c new file mode 100644 index 00000000000..7b4d71d72d8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/fpscr.c @@ -0,0 +1,16 @@ +/* Test the fpscr builtins. */ + +/* { dg-do compile } */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ +/* { dg-add-options arm_fp } */ + +void +test_fpscr () +{ + volatile unsigned int status = __builtin_arm_get_fpscr (); + __builtin_arm_set_fpscr (status); +} + +/* { dg-final { scan-assembler "mrc\tp10, 7, r\[0-9\]+, cr1, cr0, 0" } } */ +/* { dg-final { scan-assembler "mcr\tp10, 7, r\[0-9\]+, cr1, cr0, 0" } } */
The GCC documentation in section 6.60.8 ARM Floating Point Status and Control Intrinsics states that the FPSCR register can be read and written to using the intrinsics __builtin_arm_get_fpscr and __builtin_arm_set_fpscr. However, these are misnamed within GCC itself and these intrinsic names are not recognised. This patch corrects the intrinsic names to match the documentation, and adds tests to verify these intrinsics generate the correct instructions. Testing done: Ran regression tests on arm-none-eabi for Cortex-M4. 2017-03-09 Prakhar Bahuguna <prakhar.bahuguna@arm.com> gcc/ChangeLog: * gcc/config/arm/arm-builtins.c (arm_init_builtins): Rename __builtin_arm_ldfscr to __builtin_arm_get_fpscr, and rename __builtin_arm_stfscr to __builtin_arm_set_fpscr. * gcc/testsuite/gcc.target/arm/fpscr.c: New file. Okay for stage 1? -- Prakhar Bahuguna From 8359732084b5b5585d14b7fbdf70d3cfa4c6dda2 Mon Sep 17 00:00:00 2001 From: Prakhar Bahuguna <prakhar.bahuguna@arm.com> Date: Wed, 8 Mar 2017 16:29:09 +0000 Subject: [PATCH] Rename FPSCR builtins to correct names --- gcc/config/arm/arm-builtins.c | 4 ++-- gcc/testsuite/gcc.target/arm/fpscr.c | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/fpscr.c