* [PATCH][GCC][ARM] Disable neon testing for armv7-m
@ 2015-11-16 10:49 Andre Vieira
2015-11-16 12:07 ` James Greenhalgh
0 siblings, 1 reply; 8+ messages in thread
From: Andre Vieira @ 2015-11-16 10:49 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 748 bytes --]
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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-neon-testing-for-armv7-m.patch --]
[-- Type: text/x-patch; name=0001-Disable-neon-testing-for-armv7-m.patch, Size: 983 bytes --]
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-16 10:49 [PATCH][GCC][ARM] Disable neon testing for armv7-m Andre Vieira
@ 2015-11-16 12:07 ` James Greenhalgh
2015-11-16 13:15 ` Andre Vieira
0 siblings, 1 reply; 8+ messages in thread
From: James Greenhalgh @ 2015-11-16 12:07 UTC (permalink / raw)
To: Andre Vieira; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-16 12:07 ` James Greenhalgh
@ 2015-11-16 13:15 ` Andre Vieira
2015-11-17 10:10 ` James Greenhalgh
0 siblings, 1 reply; 8+ messages in thread
From: Andre Vieira @ 2015-11-16 13:15 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-16 13:15 ` Andre Vieira
@ 2015-11-17 10:10 ` James Greenhalgh
2015-11-18 9:45 ` Andre Vieira
0 siblings, 1 reply; 8+ messages in thread
From: James Greenhalgh @ 2015-11-17 10:10 UTC (permalink / raw)
To: Andre Vieira; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-17 10:10 ` James Greenhalgh
@ 2015-11-18 9:45 ` Andre Vieira
2015-11-20 11:51 ` Kyrill Tkachov
0 siblings, 1 reply; 8+ messages in thread
From: Andre Vieira @ 2015-11-18 9:45 UTC (permalink / raw)
To: James Greenhalgh; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 3714 bytes --]
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?
Cheers,
Andre
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Disable-neon-testing-for-armv7-m.patch --]
[-- Type: text/x-patch; name=0001-Disable-neon-testing-for-armv7-m.patch, Size: 1067 bytes --]
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-18 9:45 ` Andre Vieira
@ 2015-11-20 11:51 ` Kyrill Tkachov
2015-11-20 16:44 ` Andre Vieira
0 siblings, 1 reply; 8+ messages in thread
From: Kyrill Tkachov @ 2015-11-20 11:51 UTC (permalink / raw)
To: Andre Vieira, James Greenhalgh; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-20 11:51 ` Kyrill Tkachov
@ 2015-11-20 16:44 ` Andre Vieira
2015-11-26 10:15 ` Kyrill Tkachov
0 siblings, 1 reply; 8+ messages in thread
From: Andre Vieira @ 2015-11-20 16:44 UTC (permalink / raw)
To: Kyrill Tkachov, James Greenhalgh; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m
2015-11-20 16:44 ` Andre Vieira
@ 2015-11-26 10:15 ` Kyrill Tkachov
0 siblings, 0 replies; 8+ messages in thread
From: Kyrill Tkachov @ 2015-11-26 10:15 UTC (permalink / raw)
To: Andre Vieira, James Greenhalgh; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-26 10:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-16 10:49 [PATCH][GCC][ARM] Disable neon testing for armv7-m Andre Vieira
2015-11-16 12:07 ` James Greenhalgh
2015-11-16 13:15 ` Andre Vieira
2015-11-17 10:10 ` James Greenhalgh
2015-11-18 9:45 ` Andre Vieira
2015-11-20 11:51 ` Kyrill Tkachov
2015-11-20 16:44 ` Andre Vieira
2015-11-26 10:15 ` Kyrill Tkachov
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).