x86 AVX512: add support for the _mm512_alignr_epi32 instruction #1265
Conversation
There was a problem hiding this comment.
Dear @NewAgeDaedalus ; thank you for your contribution! It is very welcome. I left some comments, mostly about matching the style of the existing SIMDe code and comparing your implementation to the existing simde_mm256_alignr_epi8 function.
I'll be happy to receive and review additional implementations of the AVX512 alignr functions once this one is merged. Now that you have a sense of our coding style, feel free to bundle related functions into one PR in the future.
And again, thank you!
| * 2020 Evan Nemerson <[email protected]> | ||
| * 2020 Christopher Moore <[email protected]> | ||
| * 2023 Michael R. Crusoe <[email protected]> |
There was a problem hiding this comment.
| * 2020 Evan Nemerson <evan@nemerson.com> | |
| * 2020 Christopher Moore <moore@free.fr> | |
| * 2023 Michael R. Crusoe <crusoe@debian.org> | |
| * 2025 Fabian Penezic <fabian.penezic@fer.hr> |
When making a new file, you should only list yourself; thanks!
| #include "../avx2.h" | ||
| #include "mov.h" | ||
| #include "extract.h" | ||
|
|
||
| #include <stdio.h> |
There was a problem hiding this comment.
| #include "../avx2.h" | |
| #include "mov.h" | |
| #include "extract.h" | |
| #include <stdio.h> | |
| #include "setzero.h" |
| a_p = simde__m512i_to_private(a), | ||
| b_p = simde__m512i_to_private(b), | ||
| r_p; |
There was a problem hiding this comment.
| a_p = simde__m512i_to_private(a), | |
| b_p = simde__m512i_to_private(b), | |
| r_p; | |
| a_ = simde__m512i_to_private(a), | |
| b_ = simde__m512i_to_private(b), | |
| r_; |
To be consistent with the other SIMDe code, we name the private versions of variables with just a _ suffix
|
|
||
| SIMDE_FUNCTION_ATTRIBUTES | ||
| simde__m512i | ||
| simde_mm512_alignr_epi32 (simde__m512i a, simde__m512i b, const int imm8){ |
There was a problem hiding this comment.
| simde_mm512_alignr_epi32 (simde__m512i a, simde__m512i b, const int imm8){ | |
| simde_mm512_alignr_epi32 (simde__m512i a, simde__m512i b, const int imm8) | |
| SIMDE_REQUIRE_CONSTANT_RANGE(count, 0, 255) { |
| a_p = simde__m512i_to_private(a), | ||
| b_p = simde__m512i_to_private(b), | ||
| r_p; | ||
|
|
There was a problem hiding this comment.
| if (HEDLEY_UNLIKELY(count > 31)) | |
| return simde_mm512_setzero_epi32(); | |
| size_t len = sizeof(a_p)/sizeof(a_p.i32[0]); | ||
|
|
||
| for (size_t i = 0; i < (0xF & imm8); i++) { | ||
| r_p.i32[len-i-1] = a_p.i32[(0xF & imm8) - i - 1]; | ||
| } | ||
|
|
||
| for (size_t i = (imm8 & 0xF), j=0; i < len; i++) { | ||
| r_p.i32[len - i - 1] = b_p.i32[len - j - 1]; | ||
| j++; | ||
| } |
There was a problem hiding this comment.
Please review how simde_mm256_alignr_epi8 was implemented. Please try to match the same style and include the SIMDE_VECTORIZE macro.
| #if defined(SIMDE_X86_AVX512F_NATIVE) | ||
| #define simde_mm512_alignr_epi32(a, b, imm8) _mm512_alignr_epi32(a, b, imm8); | ||
| #endif | ||
|
|
There was a problem hiding this comment.
| #if defined(SIMDE_X86_AVX512F_ENABLE_NATIVE_ALIASES) | |
| #undef _mm512_alignr_epi32 | |
| #define _mm512_alignr_epi32(a, b, count) simde_mm512_alignr_epi32((a), (b), (count)) | |
| #endif | |
| simde_assert_m512i_i32(r, ==, test_vec[2].r); | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Thank you for including a test! Did you generate the test value on a AVX512F capable CPU?
Please include the code that you used to generate the test values:
simde/test/x86/avx512/shuffle.c
Lines 493 to 507 in f56ef45
|
Oh, and please add an entry for |
I want to add support for the alignr instructions. Would love feedback if such implementation of these instructions is acceptable and implement the rest of them at a later date.