* [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names @ 2017-03-22 10:46 Prakhar Bahuguna 2017-04-10 12:26 ` Prakhar Bahuguna 2017-04-10 14:01 ` Kyrill Tkachov 0 siblings, 2 replies; 8+ messages in thread From: Prakhar Bahuguna @ 2017-03-22 10:46 UTC (permalink / raw) To: gcc-patches; +Cc: nd, Richard.Earnshaw, Ramana.Radhakrishnan, Kyrylo.Tkachov [-- Attachment #1: Type: text/plain, Size: 869 bytes --] 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 [-- Attachment #2: 0001-Rename-FPSCR-builtins-to-correct-names.patch --] [-- Type: text/plain, Size: 1964 bytes --] 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 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" } } */ -- 2.11.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-03-22 10:46 [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names Prakhar Bahuguna @ 2017-04-10 12:26 ` Prakhar Bahuguna 2017-04-10 14:01 ` Kyrill Tkachov 1 sibling, 0 replies; 8+ messages in thread From: Prakhar Bahuguna @ 2017-04-10 12:26 UTC (permalink / raw) To: gcc-patches; +Cc: nd, Richard.Earnshaw, Ramana.Radhakrishnan, Kyrylo.Tkachov 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-03-22 10:46 [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names Prakhar Bahuguna 2017-04-10 12:26 ` Prakhar Bahuguna @ 2017-04-10 14:01 ` Kyrill Tkachov 2017-06-20 13:49 ` [arm-embedded] " Thomas Preudhomme 2017-06-23 15:49 ` Thomas Preudhomme 1 sibling, 2 replies; 8+ messages in thread From: Kyrill Tkachov @ 2017-04-10 14:01 UTC (permalink / raw) To: Prakhar Bahuguna, gcc-patches; +Cc: nd, Richard.Earnshaw, Ramana.Radhakrishnan 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [arm-embedded] [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-04-10 14:01 ` Kyrill Tkachov @ 2017-06-20 13:49 ` Thomas Preudhomme 2017-06-23 15:49 ` Thomas Preudhomme 1 sibling, 0 replies; 8+ messages in thread From: Thomas Preudhomme @ 2017-06-20 13:49 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 544 bytes --] 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 [-- Attachment #2: Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names.eml --] [-- Type: message/rfc822, Size: 5059 bytes --] From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> To: Prakhar Bahuguna <prakhar.bahuguna@arm.com>, gcc-patches@gcc.gnu.org Cc: nd@arm.com, Richard.Earnshaw@arm.com, Ramana.Radhakrishnan@arm.com Subject: Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names Date: Mon, 10 Apr 2017 15:01:39 +0100 Message-ID: <58EB9043.8060104@foss.arm.com> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-04-10 14:01 ` Kyrill Tkachov 2017-06-20 13:49 ` [arm-embedded] " Thomas Preudhomme @ 2017-06-23 15:49 ` Thomas Preudhomme 2017-06-23 15:54 ` Kyrill Tkachov 2017-06-23 19:10 ` Christophe Lyon 1 sibling, 2 replies; 8+ messages in thread From: Thomas Preudhomme @ 2017-06-23 15:49 UTC (permalink / raw) To: Kyrill Tkachov, Prakhar Bahuguna, gcc-patches Cc: Richard.Earnshaw, Ramana.Radhakrishnan [-- Attachment #1: Type: text/plain, Size: 2408 bytes --] Hi Kyrill, On 10/04/17 15:01, Kyrill Tkachov wrote: > 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. How about a backport to GCC 5, 6 & 7? The patch applied cleanly on each of these versions and the testsuite didn't show any regression for any of the backport when run for Cortex-M7. Patches attached for reference. ChangeLog entries: *** gcc/ChangeLog *** 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> * 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/ChangeLog *** 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> gcc/testsuite/ * gcc.target/arm/fpscr.c: New file. Best regards, Thomas [-- Attachment #2: fix_fpscr_builtin_name_gcc5.patch --] [-- Type: text/x-patch, Size: 2722 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index da321440384628fb1770ff9e96377b341c61da6a..ab0e7c0167ac287b774378c3ecfb15a37d5362e7 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * 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. + 2017-06-22 Martin Liska <mliska@suse.cz> Backport from mainline diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 6f4fd9bdb9774b942f7f51145a406258a82ac1e7..edd6dac6ab73d24447e8c9f6e39c5ba22fbf9302 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -1747,10 +1747,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/ChangeLog b/gcc/testsuite/ChangeLog index b411b9dbc108f12bd1931f57d3f4c1f315161ca0..a865ed054597c12de76a953fcf751209c1e4b84c 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * gcc.target/arm/fpscr.c: New file. + 2017-06-22 Martin Liska <mliska@suse.cz> Backport from mainline diff --git a/gcc/testsuite/gcc.target/arm/fpscr.c b/gcc/testsuite/gcc.target/arm/fpscr.c new file mode 100644 index 0000000000000000000000000000000000000000..7b4d71d72d8964f6da0d0604bf59aeb4a895df43 --- /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" } } */ [-- Attachment #3: fix_fpscr_builtin_name_gcc6.patch --] [-- Type: text/x-patch, Size: 2750 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b24b70c7ef819ea3b45b6019b0db4ad37c6dfce8..61578113c5e3dd8cadcaf5e234f0cd5bb7cced38 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * 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. + 2017-06-19 James Greenhalgh <james.greenhalgh@arm.com> Backport from mainline diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 92293c26824881bf5ca5c401fd54c3f2f162552c..809c43e7d8d0a0324afb45455c76bd7611ab3a25 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -1783,10 +1783,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/ChangeLog b/gcc/testsuite/ChangeLog index 620e3c616de4ed0d8c3d6de08fa2cc657543da4e..f25016b590e3d96e92b0817d0b2b785a87d6a804 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * gcc.target/arm/fpscr.c: New file. + 2017-06-19 James Greenhalgh <james.greenhalgh@arm.com> Backport from mainline diff --git a/gcc/testsuite/gcc.target/arm/fpscr.c b/gcc/testsuite/gcc.target/arm/fpscr.c new file mode 100644 index 0000000000000000000000000000000000000000..7b4d71d72d8964f6da0d0604bf59aeb4a895df43 --- /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" } } */ [-- Attachment #4: fix_fpscr_builtin_name_gcc7.patch --] [-- Type: text/x-patch, Size: 2730 bytes --] diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b23ab131099d8120f02c283edea9d6cac3ed957a..83731befa78a9bf8c672ac52c16f8dd9029757b2 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * 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. + 2017-06-20 Andreas Schwab <schwab@suse.de> PR target/80970 diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 8b7eaa8e81c59d0e18e22908d8748f0e01f5a9a2..792b688f66cd666e4fcef568fadc385c5b332be4 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -1893,10 +1893,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/ChangeLog b/gcc/testsuite/ChangeLog index fe738a0960bf4af1db9bf8c20a4acb515251ad26..82f4a382d4b1df30807b10bfbfcdbb322dbd1722 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> + + Backport from mainline + 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> + + * gcc.target/arm/fpscr.c: New file. + 2017-06-19 James Greenhalgh <james.greenhalgh@arm.com> Backport from mainline diff --git a/gcc/testsuite/gcc.target/arm/fpscr.c b/gcc/testsuite/gcc.target/arm/fpscr.c new file mode 100644 index 0000000000000000000000000000000000000000..7b4d71d72d8964f6da0d0604bf59aeb4a895df43 --- /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" } } */ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-06-23 15:49 ` Thomas Preudhomme @ 2017-06-23 15:54 ` Kyrill Tkachov 2017-06-23 19:10 ` Christophe Lyon 1 sibling, 0 replies; 8+ messages in thread From: Kyrill Tkachov @ 2017-06-23 15:54 UTC (permalink / raw) To: Thomas Preudhomme, Prakhar Bahuguna, gcc-patches Cc: Richard.Earnshaw, Ramana.Radhakrishnan Hi Thomas, On 23/06/17 16:48, Thomas Preudhomme wrote: > Hi Kyrill, > > On 10/04/17 15:01, Kyrill Tkachov wrote: >> 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. > > How about a backport to GCC 5, 6 & 7? The patch applied cleanly on each of these versions and the testsuite didn't show any regression for any of the backport when run for Cortex-M7. Yes, thanks. These were always documented "correctly". The patch makes sure the implementation matches that documentation. Kyrill > > Patches attached for reference. > > ChangeLog entries: > > *** gcc/ChangeLog *** > > 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> > > Backport from mainline > 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > * 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/ChangeLog *** > > 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> > > Backport from mainline > 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > gcc/testsuite/ > * gcc.target/arm/fpscr.c: New file. > > > Best regards, > > Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-06-23 15:49 ` Thomas Preudhomme 2017-06-23 15:54 ` Kyrill Tkachov @ 2017-06-23 19:10 ` Christophe Lyon 2017-06-26 11:20 ` Thomas Preudhomme 1 sibling, 1 reply; 8+ messages in thread From: Christophe Lyon @ 2017-06-23 19:10 UTC (permalink / raw) To: Thomas Preudhomme Cc: Kyrill Tkachov, Prakhar Bahuguna, gcc-patches, Richard Earnshaw, Ramana Radhakrishnan Hi Thomas, On 23 June 2017 at 17:48, Thomas Preudhomme <thomas.preudhomme@foss.arm.com> wrote: > Hi Kyrill, > > > On 10/04/17 15:01, Kyrill Tkachov wrote: >> >> 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. > > > How about a backport to GCC 5, 6 & 7? The patch applied cleanly on each of > these versions and the testsuite didn't show any regression for any of the > backport when run for Cortex-M7. > Three's a problem with GCC-5: gcc.target/arm/fpscr.c: unknown effective target keyword `arm_fp_ok' for " dg-require-effective-target 4 arm_fp_ok " Indeed arm_fp_ok effective-target does not exist in the gcc-5 branch. Christophe > Patches attached for reference. > > ChangeLog entries: > > *** gcc/ChangeLog *** > > 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> > > Backport from mainline > 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > * 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/ChangeLog *** > > 2017-06-20 Thomas Preud'homme <thomas.preudhomme@arm.com> > > Backport from mainline > 2017-05-04 Prakhar Bahuguna <prakhar.bahuguna@arm.com> > > gcc/testsuite/ > * gcc.target/arm/fpscr.c: New file. > > > Best regards, > > Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names 2017-06-23 19:10 ` Christophe Lyon @ 2017-06-26 11:20 ` Thomas Preudhomme 0 siblings, 0 replies; 8+ messages in thread From: Thomas Preudhomme @ 2017-06-26 11:20 UTC (permalink / raw) To: Christophe Lyon Cc: Kyrill Tkachov, Prakhar Bahuguna, gcc-patches, Richard Earnshaw, Ramana Radhakrishnan Hi Christophe, On 23/06/17 20:10, Christophe Lyon wrote: > Hi Thomas, > > On 23 June 2017 at 17:48, Thomas Preudhomme > <thomas.preudhomme@foss.arm.com> wrote: >> Hi Kyrill, >> >> >> On 10/04/17 15:01, Kyrill Tkachov wrote: >>> >>> 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. >> >> >> How about a backport to GCC 5, 6 & 7? The patch applied cleanly on each of >> these versions and the testsuite didn't show any regression for any of the >> backport when run for Cortex-M7. >> > > Three's a problem with GCC-5: > gcc.target/arm/fpscr.c: unknown effective target keyword > `arm_fp_ok' for " dg-require-effective-target 4 arm_fp_ok " > > Indeed arm_fp_ok effective-target does not exist in the gcc-5 branch. Oh no. I remember not seeing anything but I can indeed see this with compare_tests from the sum file I save after each testing. Alright, what is done is done, working on a patch now. Best regards, Thomas ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-06-26 11:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-22 10:46 [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names Prakhar Bahuguna 2017-04-10 12:26 ` Prakhar Bahuguna 2017-04-10 14:01 ` Kyrill Tkachov 2017-06-20 13:49 ` [arm-embedded] " Thomas Preudhomme 2017-06-23 15:49 ` Thomas Preudhomme 2017-06-23 15:54 ` Kyrill Tkachov 2017-06-23 19:10 ` Christophe Lyon 2017-06-26 11:20 ` Thomas Preudhomme
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).