Re: [PATCH][GCC][ARM] Disable neon testing for armv7-m

[James Greenhalgh <james dot greenhalgh at arm dot com>]

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