Open oviano opened 1 year ago
randombit
commented
1 year ago This is only seen on the Apple version of clang. I'm not sure what the problem is; possibly it's simply because for Apple hardware, all aarch64 hardware in existence supports the crypto extension, so explicitly marking support for it is "pointless". But of course doing so is required for standard Clang, or with GCC. The warnings are afaik harmless.
oviano
commented
1 year ago I've had a bit of a closer look at this.
It seems it might relate to a difference between the way function attribute(target) is treated on gcc vs clang:
https://gcc.gnu.org/onlinedocs/gcc-7.5.0/gcc/AArch64-Function-Attributes.html https://clang.llvm.org/docs/AttributeReference.html#target
I have found that:
#if defined(BOTAN_SIMD_USE_SSE2)
#define BOTAN_SIMD_ISA "sse2"
#define BOTAN_VPERM_ISA "ssse3"
#define BOTAN_CLMUL_ISA "pclmul"
#elif defined(BOTAN_SIMD_USE_NEON)
#if defined(BOTAN_TARGET_ARCH_IS_ARM64)
#define BOTAN_SIMD_ISA "+simd"
#define BOTAN_CLMUL_ISA "+crypto"
#else
#define BOTAN_SIMD_ISA "fpu=neon"
#endif
#define BOTAN_VPERM_ISA BOTAN_SIMD_ISA
#elif defined(BOTAN_SIMD_USE_ALTIVEC)
#define BOTAN_SIMD_ISA "altivec"
#define BOTAN_VPERM_ISA "altivec"
#define BOTAN_CLMUL_ISA "crypto"
#endifRemoving the + preceding "simd" still doesn't get it recognised with my compiler.
However, according to this page:
https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html
Specifically this excerpt:
‘+simd’
The Advanced SIMD (Neon) v1 and the VFPv3 floating-point instructions. The extensions ‘+neon’ and ‘+neon-vfpv3’ can be used as aliases for this extension.It turns out that if I replace the "+simd" with "neon" it is recognised and removes the warning.
Finally, the line #define BOTAN_SIMD_ISA "fpu=neon" which presumably derives from the command line option -mfpu=neon is not mentioned by either gcc or clang as an option that can be specified via a function attribute so perhaps that should just be "+simd" (gcc) or "neon" (clang) too?
Essentially if I change all "+crypo" to "crypto", "+simd" to "neon" and "fpu=neon" to "neon" then it builds without warnings on all platforms (macOS, iOS, Android) with my clang.
So maybe it just needs tweaking slightly for clang vs gcc?
Of course, maybe this is all irrelevant if the code generated is the same, but I'm a pedant and wanted to understand where it was coming from.
randombit
commented
1 year ago Thanks for looking into this. I was hoping we could just use crypto everywhere but GCC won't have it
error: arch extension 'crypto' should be prefixed by '+'
oviano
commented
1 year ago Does the "fpu=neon" cause any warnings on gcc?
Also, I don't know if it's only Apple's clang that doesn't like the preceding "+", or whether it's clang on all/other platforms, and whether it varies according to the version.
randombit
commented
1 year ago Does the "fpu=neon" cause any warnings on gcc?
No. I just checked and GCC 12 targeting arm32 will accept fpu=neon, or +simd (I suspect that is new behavior), but not neon or simd.
randombit
commented
1 year ago Looks like broadly LLVM is interested in being compatible with GCC, but isn't yet https://github.com/llvm/llvm-project/issues/56480
oviano
commented
1 year ago Ok. I'm trying to find a definite source that describes how function target attributes work.
This seems like the one for gcc
https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
Haven't found the equivalent for clang yet.
EDIT: probably this is the one, but it's fairly vague.
oviano
commented
1 year ago Ok, I ran some tests on Godbolt and it seems that from clang 16 onwards) there will be greater compatibility at least for armv8-a. It seems that the compatibility fixes haven't made it to armv7-a trunk.
https://godbolt.org/z/56bE9o7dh
armv7-a clang (trunk) targeting armv7-a
+crypto FAIL (expected)
+simd FAIL
+neon FAIL
crypto OK (unexpected?)
simd FAIL
neon OK
fpu=neon FAIL
arm gcc 12.2 targeting armv7-a
+crypto FAIL (expected)
+simd OK
+neon OK
crypto FAIL (expected)
simd FAIL
neon FAIL
fpu=neon OK
armv8-a clang (trunk) targeting armv8-a
+crypto OK
+simd OK
+neon OK
crypto OK
simd OK
neon OK
fpu=neon FAIL (expected)
ARM64 gcc trunk targeting amv8-a
+crypto OK
+simd OK
+neon FAIL
crypto FAIL
simd FAIL
neon FAIL
fpu=neon FAIL (expected)There is still not going to be a one-size-fits-all solution though.
I suggest something like this:
#if defined(BOTAN_SIMD_USE_SSE2)
#define BOTAN_SIMD_ISA "sse2"
#define BOTAN_VPERM_ISA "ssse3"
#define BOTAN_CLMUL_ISA "pclmul"
#elif defined(BOTAN_SIMD_USE_NEON)
#if defined(BOTAN_TARGET_ARCH_IS_ARM64)
#if defined(BOTAN_BUILD_COMPILER_IS_CLANG)
#define BOTAN_SIMD_ISA "neon"
#define BOTAN_CLMUL_ISA "crypto"
#else
#define BOTAN_SIMD_ISA "+simd"
#define BOTAN_CLMUL_ISA "+crypto"
#endif
#else
#if defined(BOTAN_BUILD_COMPILER_IS_CLANG)
#define BOTAN_SIMD_ISA "neon"
#else
#define BOTAN_SIMD_ISA "fpu=neon"
#endif
#endif
#define BOTAN_VPERM_ISA BOTAN_SIMD_ISAand elsewhere
if defined(BOTAN_BUILD_COMPILER_IS_CLANG)
BOTAN_FUNC_ISA("crypto")
#else
BOTAN_FUNC_ISA("+crypto")
#endif
I get this warning together with a similar one about "++simd" on my macOS arm64 host when building native macOS, arm64 iOS or arm64 Android.
I also get a similar warning about when building Android armv7 about "+fpu=neon".
Here's an example with iOS arn64:
Then I get output like this in places:
If I change the "+crypto" to "+nocrypto" in the above, then I get a bunch of compile errors, which makes sense:
So my question really is do these warnings matter?