| Message ID | 564C4894.4030100@arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Andre, On 18/11/15 09:44, Andre Vieira wrote: > On 17/11/15 10:10, James Greenhalgh wrote: >> 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 >> > > This OK? > This is ok, I've committed for you with the slightly tweaked ChangeLog entry: 2015-11-20 Andre Vieira <andre.simoesdiasvieira@arm.com> * lib/target-supports.exp (check_effective_target_arm_neon_ok_nocache): Add check for M profile. as r230653. Thanks, Kyrill > Cheers, > Andre
Hi Kyrill On 20/11/15 11:51, Kyrill Tkachov wrote: > Hi Andre, > > On 18/11/15 09:44, Andre Vieira wrote: >> On 17/11/15 10:10, James Greenhalgh wrote: >>> 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 >>> >> >> This OK? >> > > This is ok, > I've committed for you with the slightly tweaked ChangeLog entry: > 2015-11-20 Andre Vieira <andre.simoesdiasvieira@arm.com> > > * lib/target-supports.exp > (check_effective_target_arm_neon_ok_nocache): Add check > for M profile. > > as r230653. > > Thanks, > Kyrill > > >> Cheers, >> Andre > Thank you. Would there be any objections to backporting this to gcc-5-branch? I checked, it applies cleanly and its a simple enough way of preventing a lot of FAILS for armv7-m. Best Regards, Andre
On 20/11/15 16:44, Andre Vieira wrote: > Hi Kyrill > On 20/11/15 11:51, Kyrill Tkachov wrote: >> Hi Andre, >> >> On 18/11/15 09:44, Andre Vieira wrote: >>> On 17/11/15 10:10, James Greenhalgh wrote: >>>> 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 >>>> >>> >>> This OK? >>> >> >> This is ok, >> I've committed for you with the slightly tweaked ChangeLog entry: >> 2015-11-20 Andre Vieira <andre.simoesdiasvieira@arm.com> >> >> * lib/target-supports.exp >> (check_effective_target_arm_neon_ok_nocache): Add check >> for M profile. >> >> as r230653. >> >> Thanks, >> Kyrill >> >> >>> Cheers, >>> Andre >> > > Thank you. Would there be any objections to backporting this to gcc-5-branch? I checked, it applies cleanly and its a simple enough way of preventing a lot of FAILS for armv7-m. > I agree. I've committed this to the GCC 5 branch for you as r230930. Thanks, Kyrill > Best Regards, > Andre
From cd58546931221197f83a82afa7ac14291d675d48 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 75d506829221e3d02d454631c4bd2acd1a8cedf2..91ff64f3d0d6ede50dcdb30cccf43be90e376a44 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2854,8 +2854,8 @@ 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 - #error Architecture too old for NEON. + #if __ARM_ARCH < 7 || __ARM_ARCH_PROFILE == 'M' + #error Architecture does not support NEON. #endif } "$flags"] } { set et_arm_neon_flags $flags -- 1.9.1