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

[Andre Vieira <Andre dot SimoesDiasVieira at arm dot com>]

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