x86/gfni: fix AltiVec paths on big-endian POWER#1418
Draft
fo40225 wants to merge 2 commits into
Draft
Conversation
The POWER AltiVec implementations of simde_x_mm_gf2p8matrix_multiply_epi64_epi8 (the helper behind the gf2p8affine family) produce wrong results on big-endian targets: * P8: byte_interleave / byte_deinterleave are vec_perm byte-index tables. The compiler applies a little-endian adjustment to vec_perm, so the same index vector selects different bytes on big-endian; provide the reversed tables for BE. bit_select feeds vec_bperm, whose bit numbering is big-endian on both targets, so it remains shared. * P8: vec_bperm leaves its gather in bits 48:63, which is unsigned-short element 4 in little-endian element numbering but element 3 in big-endian; splat the correct element per endianness. * P6: vec_sum2s leaves the 8-bit gather in the low byte of words 1 and 3 -- byte offset 4/12 on little-endian but 7/15 on big-endian; use the endianness-correct byte_select table.
fo40225
marked this pull request as draft
July 19, 2026 02:47
The AltiVec paths of the GFNI matrix-multiply helper were fixed for big-endian POWER in the previous commit (ef81068), but CI has no big-endian POWER coverage: the only POWER target in the gcc-qemu matrix is little-endian (powerpc64le/ppc64el), and the only big-endian target is s390x, which takes the ZVECTOR paths and never exercises AltiVec. Add a power9be entry to the gcc-qemu matrix, cross-compiling with the Ubuntu 24.04 gcc-14 powerpc64-linux-gnu toolchain and running the tests under qemu-ppc64-static -cpu power9. The new cross file mirrors power9-gcc-14-ccache.cross with the big-endian triple, qemu binary, sysroot and endian=big; -mcpu=power9 is kept, so the P8 AltiVec paths are compiled and now run in their big-endian variants.
Collaborator
|
Good catch, congratulations for finding and correcting the problem. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The POWER AltiVec (P6 and P8) implementations of
simde_x_mm_gf2p8matrix_multiply_epi64_epi8the helper used by thegf2p8affine/gf2p8affineinvfamily return wrong results on big-endian POWER. The code was correct only for little-endian.Root cause
Three endianness assumptions baked into vector constants and element indices:
P8,
vec_permindex tables (byte_interleave,byte_deinterleave):vec_permbyte indices are subject to the compiler's little-endian adjustment, so the same table selects different bytes on big-endian.The BE variants are the reversed index vectors.
bit_selectis not affected: it feedsvec_bperm, whose bit numbering is big-endian on both targets, so it stays shared.P8,
vec_bpermresult position: the gathered bits land in bits 48:63 of the vector unsigned-short element 4 in little-endian element numbering, but element 3 in big-endian.The
vec_splatindex must follow.P6,
byte_selecttable:vec_sum2sleaves the 8-bit gather in the low byte of words 1 and 3, i.e. byte offset 4/12 on little-endian but 7/15 on big-endian.Changes
simde/x86/gfni.honly: the three constants/indices above become endianness-specific viaSIMDE_ENDIAN_ORDER.No change on little-endian; no functional change to any other backend.
Testing
Existing gfni munit tests exercise these paths; verified on ppc64 (big-endian) and ppc64le.