-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds sha3 to arm compile options which is needed for ios compilation. #1934
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Frederik Wedel-Heinen <[email protected]>
699f192
to
f13e569
Compare
Thanks for the contribution! How would this behave on arm platforms that do not have the sha3 extensions? |
TBH I don't know and I've trouble finding documentation on these flags. Perhabs you have some suggestions on where to look? |
@@ -55,7 +55,7 @@ if(CMAKE_C_COMPILER_ID MATCHES "Clang|GNU") | |||
else() | |||
# Assume sensible default like -march=x86-64, -march=armv8-a, etc. | |||
if(ARCH_ARM64v8) | |||
set(OQS_OPT_FLAG "-march=armv8-a+crypto") | |||
set(OQS_OPT_FLAG "-march=armv8-a+crypto+sha3") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not guarded/if'd by reference to ios ("APPLE"?) if it's indeed a problem only there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it is Apple specific. As I stated in my previous comment I wasn't able to find any documentation on the flags. Do you know where to find the documentation? Maybe this affects other vendors than Apple?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know where to find the documentation?
Nope -- I'm out of my depth with ARM64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take this one back to draft and have a look at your suggestion in the related issue. Maybe I can get it to work with the toolchain file of this project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @fwh-dc , this PR makes compilation on Cortex-A53 (as you can see in CI) failed. It does not have SHA3 extension.
Tagging @hanno-becker to see if he has any insights on ARM compilation flags. |
I would recommend only using the How does one detect/convey more details about the target in CMake? In addition, |
On my Apple M1, __ARM_FEATURE_SHA3 is not defined by default (it will be defined if one compiles with -march=armv8-a+sha3). So I agree with Hanno that currently by default we should compile with -march=armv8-a. A feature detection is recommended to pass +sha3 conditionally. |
The testing macros can be found here: https://developer.arm.com/documentation/101028/0012/5--Feature-test-macros. As hardware varies, you might want to test your target platforms yourself. |
Nice thanks. I'll see when I can find the time to have a look at it. |
Fixes #1933