Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add missing aes and sha2 ISA target for armv8 #3780

Closed
wants to merge 1 commit into from
Closed

Add missing aes and sha2 ISA target for armv8 #3780

wants to merge 1 commit into from

Conversation

lieser
Copy link
Collaborator

@lieser lieser commented Oct 27, 2023 •
edited
Loading

This fixes the armv8 amalgamation build using the clang compiler from the Android NDK r26.

Fixes #3774.

@lieser lieser added this to the Botan 3.3.0 milestone Oct 27, 2023
@lieser lieser requested a review from randombit October 27, 2023 13:55
@lieser lieser self-assigned this Oct 27, 2023
lieser
lieser commented Oct 27, 2023
View reviewed changes
src/lib/block/aes/aes_armv8/aes_armv8.cpp
@@ -57,7 +57,7 @@ BOTAN_FUNC_ISA_INLINE("+crypto") void dec_last(uint8x16_t& B, uint8x16_t K, uint
B = veorq_u8(vaesdq_u8(B, K), K2);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear to me why aes ISA is not needed for this function.

@lieser
Copy link
Collaborator Author

lieser commented Oct 27, 2023

The change causes problem for GCC (./configure.py --compiler-cache=ccache --build-targets=static --without-documentation --extra-cxxflags=-fPIC --cc=gcc --cpu=armv8 --amalgamation --cc-bin=aarch64-linux-gnu-g++):

  • error: pragma or attribute ‘target("aes")’ is not valid
  • error: pragma or attribute ‘target("sha2")’ is not valid

Before I do more work on this, is the fix even going in the right direction?

@coveralls
Copy link

Coverage Status

coverage: 91.692% (+0.006%) from 91.686% when pulling 5d00753 on pl/fix-armv8_amalgamation into 60f0343 on master.

@lieser lieser mentioned this pull request Mar 5, 2024
Closed
@reneme
Copy link
Collaborator

reneme commented Mar 5, 2024

It seems that the comma-notation does not work with gcc. Instead, you'd have to specify BOTAN_FUNC_ISA("+crypto+aes"), for instance. Also, on Android, just specifying +aes seems to be fine. But on GCC it needs both to inline successfully. I'll close this, and continue with #3931 which also adds amalgamation builds for arm64.

@reneme reneme closed this Mar 5, 2024
reneme added a commit to Rohde-Schwarz/botan that referenced this pull request Mar 5, 2024
@reneme @lieser
FIX: BOTAN_FUNC_ISA() annotations for arm64-android
b1ec2db 
This is a follow up for randombit#3780 that fixes the ISA issues for both
clang (for Android NDK) and gcc.

Co-Authored-By: Philippe Lieser <philippe.lieser@rohde-schwarz.com>
@reneme reneme mentioned this pull request Mar 5, 2024
Merged
@reneme reneme deleted the pl/fix-armv8_amalgamation branch March 6, 2024 07:20
reneme added a commit to Rohde-Schwarz/botan that referenced this pull request Mar 6, 2024
@reneme @lieser
FIX: BOTAN_FUNC_ISA() annotations for arm64-android
b1d4261 
This is a follow up for randombit#3780 that fixes the ISA issues for both
clang (for Android NDK) and gcc.

Co-Authored-By: Philippe Lieser <philippe.lieser@rohde-schwarz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@randombit randombit Awaiting requested review from randombit

Assignees

@lieser lieser

Labels
build problem
Projects
None yet
Milestone
Botan 3.3.0
Development

Successfully merging this pull request may close these issues.

ARM64-v8a compiler error when amalgamation and armv8crypto is used
3 participants
@lieser @coveralls @reneme