This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 4/4] [ARM] Add attribute/pragma target fpu=
- From: Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- To: Christian Bruel <christian dot bruel at st dot com>
- Cc: Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 13 Nov 2015 11:49:41 +0000
- Subject: Re: [PATCH 4/4] [ARM] Add attribute/pragma target fpu=
- Authentication-results: sourceware.org; auth=none
- References: <55F6D9FF dot 4030600 at st dot com> <55F7F75E dot 4070800 at st dot com> <55FBD3B4 dot 9050709 at arm dot com> <5600096E dot 4030403 at st dot com> <56162EE8 dot 5010209 at arm dot com> <5644A826 dot 9040606 at st dot com>
Hi Christian,
On 12/11/15 14:54, Christian Bruel wrote:
Hi Kyril,
...
The parts in this patch look ok to me.
However, I think we need some more functionality
In aarch64 we support compiling a file with no simd, including arm_neon.h and using arm_neon.h intrinsics
within functions tagged with simd support.
We want to support such functionality on arm i.e. compile a file with -mfpu=vfp and use arm_neon.h intrinsics
in a function tagged with an fpu=neon attribute.
For that we'd need to wrap the intrinsics in arm_neon.h in appropriate pragmas, like in the aarch64 version of arm_neon.h
As discussed, here is arm_neon.h for aarch32/neon with the same programming model than aarch64/simd. As you said lets use one of the fpu=neon attributes even if the file is compiled with -mfpu=vfp.
The drawback for this is that now we unconditionally makes available every neon intrinsics, introducing a small legacy change with regards to error checking (that you didn't have with aarch64). Then it's worth to stress that:
- One cannot check #include "arm_neon.h" to check if the compiler can use neon instruction. Instead use #ifndef __ARM_NEON__. (Found in target-supports.exp)
Checking the macro is the 'canonical' way to check for NEON support,
so I reckon we can live with that.
- Types cannot be checked. For instance:
#include <arm_neon.h>
poly128_t
foo (poly128_t* ptr)
{
return vldrq_p128 (ptr);
}
compiled with -mfpu=neon used to be rejected with
error: unknown type name 'poly128_t' ...
Now the error, as a side effect from the inlining rules between incompatible modes, becomes
error: inlining failed in call to always_inline 'vldrq_p128': target specific option mismatch ...
Well, the previous message is misleading anyway since the user error there is not a type issue
but failure to specify the correct -mfpu option.
I found this more confusing, so I was a little bit reluctant to implement this, but the code is correctly rejected and the message makes sense, after all. Just a different check.
This patch applies on top of the preceding attribute/pragma target fpu= series. Tested with arm-none-eabi configured with default and --with-cpu=cortex-a9 --with-fp --with-float=hard
Do you mean --with-fpu=<something>?
Also fixes a few macro that depends on fpu=, that I forgot to redefine.
Can you please split those changes into a separate patch and ChangeLog and commit the separately?
That part is preapproved.
This patch is ok then with above comment about splitting the arm-c.c changes separately.
Thanks for doing this!
I believe all patches in this series are approved then
so you can go ahead and start committing.
Kyrill
Christian