[PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Prakhar Bahuguna]

[-- 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

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Prakhar Bahuguna]

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

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Kyrill Tkachov]

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

[arm-embedded] [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Thomas Preudhomme]

[-- 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

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Thomas Preudhomme]

[-- 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" } } */

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Kyrill Tkachov]

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

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Christophe Lyon]

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

Re: [PATCH, GCC/ARM, Stage 1] Rename FPSCR builtins to correct names

[Thomas Preudhomme]

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