peter.smith added a comment.
After retesting on a failing example and experimenting a bit, I think that we
should only preserve +crypto with -fno-integrated-as. It would also be good to
mention D66018 <https://reviews.llvm.org/D66018> and maybe add the original
author as a reviewer.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:499
<< "crypto" <<
llvm::ARM::getArchName(llvm::ARM::parseArch(ArchSuffix));
- Features.push_back("-crypto");
+ //-mfpu=crypto-neon-fp-armv8 does allow +sha2 / +aes / +crypto features,
+ // even if compiling for an cpu armv7a, if explicitly defined by the
user,
----------------
Looking into this in more detail I think the comment can be made more specific.
It seems like the main reason to keep +crypto is that when the
non-integrated-assembler is used, then we get the directive:
```
.fpu crypto-neon-fp-armv8
```
In arm-none-eabi-as the assembler will support crypto instructions. Clang will
not as any use of crypto instructions will fail due lack of v8 support.
```
With -fno-integrated-as -mfpu=crypto-neon-fp-armv8 some assemblers such as the
GNU assembler will permit the use of crypto instructions as the fpu will
override the architecture. We keep the crypto feature in this case to preserve
compatibility. In all other cases we remove the crypto feature.
```
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:502
+ // so do not deactivate them.
+ if ((llvm::find(Features, "+fp-armv8") == Features.end()) ||
+ (llvm::find(Features, "+neon") == Features.end()))
----------------
I think we should only do this for -fno-integrated-as as the integrated
assembler will fail if give a crypto instruction even with this change. With
the integrated assembler we get:
```
error: instruction requires: armv8
aese.8 q8, q9
```
================
Comment at: clang/test/Driver/arm-features.c:84
+// Check +crypto does not affect -march=armv7a -mfpu=crypto-neon-fp-armv8, but
it does warn that +crypto has no effect
+// RUN: %clang -target arm-arm-none-eabi -march=armv7a
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck
-check-prefixes=CHECK-WARNONLY,ALL %s
+// RUN: %clang -target arm-arm-none-eabi -march=armv7a+aes
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck
-check-prefixes=CHECK-WARNONLY,ALL,CHECK-HASAES %s
----------------
arm-arm-none-eabi is a vendor specific triple? Better to use arm-none-eabi
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67608/new/
https://reviews.llvm.org/D67608
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
