| Message ID | 5649B4A7.6000506@arm.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 16, 2015 at 10:49:11AM +0000, Andre Vieira wrote: > Hi, > > This patch changes the target support mechanism to make it > recognize any ARM 'M' profile as a non-neon supporting target. The > current check only tests for armv6 architectures and earlier, and > does not account for armv7-m. > > This is correct because there is no 'M' profile that supports neon > and the current test is not sufficient to exclude armv7-m. > > Tested by running regressions for this testcase for various ARM targets. > > Is this OK to commit? > > Thanks, > Andre Vieira > > gcc/testsuite/ChangeLog: > 2015-11-06 Andre Vieira <andre.simoesdiasvieira@arm.com> > > * gcc/testsuite/lib/target-supports.exp > (check_effective_target_arm_neon_ok_nocache): Added check > for M profile. > From 2c53bb9ba3236919ecf137a4887abf26d4f7fda2 Mon Sep 17 00:00:00 2001 > From: Andre Simoes Dias Vieira <andsim01@arm.com> > Date: Fri, 13 Nov 2015 11:16:34 +0000 > Subject: [PATCH] Disable neon testing for armv7-m > > --- > gcc/testsuite/lib/target-supports.exp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 75d506829221e3d02d454631c4bd2acd1a8cedf2..8097a4621b088a93d58d09571cf7aa27b8d5fba6 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -2854,7 +2854,7 @@ proc check_effective_target_arm_neon_ok_nocache { } { > int dummy; > /* Avoid the case where a test adds -mfpu=neon, but the toolchain is > configured for -mcpu=arm926ej-s, for example. */ > - #if __ARM_ARCH < 7 > + #if __ARM_ARCH < 7 || __ARM_ARCH_PROFILE == 'M' > #error Architecture too old for NEON. Could you fix this #error message while you're here? Why we can't change this test to look for the __ARM_NEON macro from ACLE: #if __ARM_NEON < 1 #error NEON is not enabled #endif Thanks, James
On 16/11/15 12:07, James Greenhalgh wrote: > On Mon, Nov 16, 2015 at 10:49:11AM +0000, Andre Vieira wrote: >> Hi, >> >> This patch changes the target support mechanism to make it >> recognize any ARM 'M' profile as a non-neon supporting target. The >> current check only tests for armv6 architectures and earlier, and >> does not account for armv7-m. >> >> This is correct because there is no 'M' profile that supports neon >> and the current test is not sufficient to exclude armv7-m. >> >> Tested by running regressions for this testcase for various ARM targets. >> >> Is this OK to commit? >> >> Thanks, >> Andre Vieira >> >> gcc/testsuite/ChangeLog: >> 2015-11-06 Andre Vieira <andre.simoesdiasvieira@arm.com> >> >> * gcc/testsuite/lib/target-supports.exp >> (check_effective_target_arm_neon_ok_nocache): Added check >> for M profile. > >> From 2c53bb9ba3236919ecf137a4887abf26d4f7fda2 Mon Sep 17 00:00:00 2001 >> From: Andre Simoes Dias Vieira <andsim01@arm.com> >> Date: Fri, 13 Nov 2015 11:16:34 +0000 >> Subject: [PATCH] Disable neon testing for armv7-m >> >> --- >> gcc/testsuite/lib/target-supports.exp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp >> index 75d506829221e3d02d454631c4bd2acd1a8cedf2..8097a4621b088a93d58d09571cf7aa27b8d5fba6 100644 >> --- a/gcc/testsuite/lib/target-supports.exp >> +++ b/gcc/testsuite/lib/target-supports.exp >> @@ -2854,7 +2854,7 @@ proc check_effective_target_arm_neon_ok_nocache { } { >> int dummy; >> /* Avoid the case where a test adds -mfpu=neon, but the toolchain is >> configured for -mcpu=arm926ej-s, for example. */ >> - #if __ARM_ARCH < 7 >> + #if __ARM_ARCH < 7 || __ARM_ARCH_PROFILE == 'M' >> #error Architecture too old for NEON. > > Could you fix this #error message while you're here? > > Why we can't change this test to look for the __ARM_NEON macro from ACLE: > > #if __ARM_NEON < 1 > #error NEON is not enabled > #endif > > Thanks, > James > There is a check for this already: 'check_effective_target_arm_neon'. I think the idea behind arm_neon_ok is to check whether the hardware would support neon, whereas arm_neon is to check whether neon was enabled, i.e. -mfpu=neon was used or a mcpu was passed that has neon enabled by default. The comments for 'check_effective_target_arm_neon_ok_nocache' highlight this, though maybe the comments for check_effective_target_arm_neon could be better. # Return 1 if this is an ARM target supporting -mfpu=neon # -mfloat-abi=softfp or equivalent options. Some multilibs may be # incompatible with these options. Also set et_arm_neon_flags to the # best options to add. proc check_effective_target_arm_neon_ok_nocache ... /* Avoid the case where a test adds -mfpu=neon, but the toolchain is configured for -mcpu=arm926ej-s, for example. */ ... and # Return 1 if this is a ARM target with NEON enabled. proc check_effective_target_arm_neon ... Cheers, Andre
On Mon, Nov 16, 2015 at 01:15:32PM +0000, Andre Vieira wrote: > On 16/11/15 12:07, James Greenhalgh wrote: > >On Mon, Nov 16, 2015 at 10:49:11AM +0000, Andre Vieira wrote: > >>Hi, > >> > >> This patch changes the target support mechanism to make it > >>recognize any ARM 'M' profile as a non-neon supporting target. The > >>current check only tests for armv6 architectures and earlier, and > >>does not account for armv7-m. > >> > >> This is correct because there is no 'M' profile that supports neon > >>and the current test is not sufficient to exclude armv7-m. > >> > >> Tested by running regressions for this testcase for various ARM targets. > >> > >> Is this OK to commit? > >> > >> Thanks, > >> Andre Vieira > >> > >>gcc/testsuite/ChangeLog: > >>2015-11-06 Andre Vieira <andre.simoesdiasvieira@arm.com> > >> > >> * gcc/testsuite/lib/target-supports.exp > >> (check_effective_target_arm_neon_ok_nocache): Added check > >> for M profile. > > > >> From 2c53bb9ba3236919ecf137a4887abf26d4f7fda2 Mon Sep 17 00:00:00 2001 > >>From: Andre Simoes Dias Vieira <andsim01@arm.com> > >>Date: Fri, 13 Nov 2015 11:16:34 +0000 > >>Subject: [PATCH] Disable neon testing for armv7-m > >> > >>--- > >> gcc/testsuite/lib/target-supports.exp | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > >>index 75d506829221e3d02d454631c4bd2acd1a8cedf2..8097a4621b088a93d58d09571cf7aa27b8d5fba6 100644 > >>--- a/gcc/testsuite/lib/target-supports.exp > >>+++ b/gcc/testsuite/lib/target-supports.exp > >>@@ -2854,7 +2854,7 @@ proc check_effective_target_arm_neon_ok_nocache { } { > >> int dummy; > >> /* Avoid the case where a test adds -mfpu=neon, but the toolchain is > >> configured for -mcpu=arm926ej-s, for example. */ > >>- #if __ARM_ARCH < 7 > >>+ #if __ARM_ARCH < 7 || __ARM_ARCH_PROFILE == 'M' > >> #error Architecture too old for NEON. > > > >Could you fix this #error message while you're here? > > > >Why we can't change this test to look for the __ARM_NEON macro from ACLE: > > > >#if __ARM_NEON < 1 > > #error NEON is not enabled > >#endif > > > >Thanks, > >James > > > > There is a check for this already: > 'check_effective_target_arm_neon'. I think the idea behind > arm_neon_ok is to check whether the hardware would support neon, > whereas arm_neon is to check whether neon was enabled, i.e. > -mfpu=neon was used or a mcpu was passed that has neon enabled by > default. > > The comments for 'check_effective_target_arm_neon_ok_nocache' > highlight this, though maybe the comments for > check_effective_target_arm_neon could be better. > > # Return 1 if this is an ARM target supporting -mfpu=neon > # -mfloat-abi=softfp or equivalent options. Some multilibs may be > # incompatible with these options. Also set et_arm_neon_flags to the > # best options to add. > > proc check_effective_target_arm_neon_ok_nocache > ... > /* Avoid the case where a test adds -mfpu=neon, but the toolchain is > configured for -mcpu=arm926ej-s, for example. */ > ... > > > and > > # Return 1 if this is a ARM target with NEON enabled. > > proc check_effective_target_arm_neon OK, got it - sorry for my mistake, I had the two procs confused. I'd still like to see the error message fixed "Architecture too old for NEON." is not an accurate description of the problem. Thanks, James
From 2c53bb9ba3236919ecf137a4887abf26d4f7fda2 Mon Sep 17 00:00:00 2001 From: Andre Simoes Dias Vieira <andsim01@arm.com> Date: Fri, 13 Nov 2015 11:16:34 +0000 Subject: [PATCH] Disable neon testing for armv7-m --- gcc/testsuite/lib/target-supports.exp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 75d506829221e3d02d454631c4bd2acd1a8cedf2..8097a4621b088a93d58d09571cf7aa27b8d5fba6 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2854,7 +2854,7 @@ proc check_effective_target_arm_neon_ok_nocache { } { int dummy; /* Avoid the case where a test adds -mfpu=neon, but the toolchain is configured for -mcpu=arm926ej-s, for example. */ - #if __ARM_ARCH < 7 + #if __ARM_ARCH < 7 || __ARM_ARCH_PROFILE == 'M' #error Architecture too old for NEON. #endif } "$flags"] } { -- 1.9.1
Hi, This patch changes the target support mechanism to make it recognize any ARM 'M' profile as a non-neon supporting target. The current check only tests for armv6 architectures and earlier, and does not account for armv7-m. This is correct because there is no 'M' profile that supports neon and the current test is not sufficient to exclude armv7-m. Tested by running regressions for this testcase for various ARM targets. Is this OK to commit? Thanks, Andre Vieira gcc/testsuite/ChangeLog: 2015-11-06 Andre Vieira <andre.simoesdiasvieira@arm.com> * gcc/testsuite/lib/target-supports.exp (check_effective_target_arm_neon_ok_nocache): Added check for M profile.