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.

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

feat: Add _mm_aesdec_si128 #559

Merged
merged 1 commit into from Dec 24, 2022
Merged

feat: Add _mm_aesdec_si128 #559

merged 1 commit into from Dec 24, 2022

Conversation

@howjmay howjmay force-pushed the aesdec branch 2 times, most recently from 20a059d to 6aca545 Compare December 22, 2022 12:23
@@ -9533,6 +9533,17 @@ FORCE_INLINE __m128i _mm_aesenc_si128(__m128i EncBlock, __m128i RoundKey)
#endif
}

FORCE_INLINE __m128i _mm_aesdec_si128(__m128i a, __m128i RoundKey)
{
#if defined(__aarch64__)
Copy link
Member

Choose a reason for hiding this comment

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

You shall take the following into consideration:

  1. Armv8-A without Cryptographic extension
  2. Armv8-A and A32 with Cryptographic extension
  3. Armv7-A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems we don't have tests for function guarded by "__ARM_FEATURE_CRYPTO" macro. I think maybe I should add another makefile target to run test with +crypto?

Copy link
Member

Choose a reason for hiding this comment

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

Exactly, we shall have conditional +crypto in feasible permutations.

@howjmay howjmay force-pushed the aesdec branch 6 times, most recently from 5237fda to f8c1fde Compare December 23, 2022 16:39
// add round key
return vreinterpretq_m128i_u8(w) ^ RoundKey;
#else /* ARMv7-A NEON implementation */
/* FIXME: optimized for NEON */
Copy link
Contributor Author

@howjmay howjmay Dec 23, 2022

Choose a reason for hiding this comment

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

@jserv the tables for table-based AES algorithm are way too many and huge. Therefore, I am using plain pure C implementation here instead of the table-based AES which is used in _mm_aesenc_si128.
I am trying to optimize it.
A simple table-based AES needs tables like this https://github.com/iVishalr/AES-Encryption/blob/master/lookup.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The implementation of both helper_aesenc and helper_aesdec are table-based AES. Here are its tables https://github.com/qemu/qemu/blob/266469947161aa10b1d36843580d369d5aa38589/crypto/aes.c

Copy link
Member

@jserv jserv Dec 24, 2022

Choose a reason for hiding this comment

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

Agree. Let's stick to shorter implementation since Armv7 path exits for compatibility purpose.

tests/impl.cpp Outdated Show resolved Hide resolved
tests/impl.cpp Show resolved Hide resolved
sse2neon.h Show resolved Hide resolved
sse2neon.h Outdated Show resolved Hide resolved
@howjmay howjmay force-pushed the aesdec branch 4 times, most recently from eeb2ce0 to a34db2c Compare December 24, 2022 04:13
Copy link
Member

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Resolve the conflicts.

mr-c added a commit to mr-c/simde that referenced this pull request Oct 16, 2023
mr-c added a commit to mr-c/simde that referenced this pull request Oct 16, 2023
mr-c added a commit to simd-everywhere/simde that referenced this pull request Oct 16, 2023
nemequ pushed a commit to simd-everywhere/simde-no-tests that referenced this pull request Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants