Title: kernel/x86_64: make AVX-512 asum/sum kernels independent of buffer alignment - #5972
Open
davidheff wants to merge 1 commit into
Open
Title: kernel/x86_64: make AVX-512 asum/sum kernels independent of buffer alignment#5972davidheff wants to merge 1 commit into
davidheff wants to merge 1 commit into
Conversation
…ignment The skylakex/cooperlake d/s/c/z asum and c/z sum microkernels peel leading elements until the input pointer reaches a 64-byte boundary (a scalar loop in dasum/sasum, a masked header load in the complex variants) before entering an aligned-load accumulator loop. The peel count depends on the buffer address mod 64, so the grouping of the sum into accumulators - and therefore the rounding of the result - depends on where the caller's buffer happens to sit in memory. The same data at a different address can give a bitwise-different sum. That address dependence surfaced as non-reproducibility in OrcaFlex: LAPACK's dstein scales each inverse-iteration eigenvector by 1/dasum(...) over a heap array whose alignment varies with allocation history, so eigenvectors from identical inputs differed run to run in the last bits, which zero-tolerance regression comparison flags. Fix by dropping the alignment peel and using unaligned loads throughout, so the summation order is a function of the length alone. On AVX-512 hardware unaligned load instructions on addresses that happen to be aligned cost the same as aligned loads; only genuinely split cache lines pay a small penalty, negligible for these level-1 reductions.
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 skylakex/cooperlake/sapphirerapids
?asumand?summicrokernels(
dasum_microk_skylakex-2.cand siblings) peel leading elements until theinput pointer reaches a 64-byte boundary — a scalar loop in
dasum/sasum,a masked header load in the complex variants — before entering an
aligned-load (
_mm512_load_p?) accumulator loop. The peel count depends onthe buffer address mod 64, so the grouping of the sum into the four
accumulators, and therefore the rounding of the result, depends on where the
caller's buffer happens to sit in memory: the same data at a different
address can return a bitwise-different sum.
We hit this downstream in OrcaFlex (Orcina's offshore engineering package):
LAPACK's
dsteinscales each inverse-iteration eigenvector by1/dasum(...)over a workspace array whose alignment varies with the host application's
heap history, so eigenvectors computed from identical inputs differed in the
last bits from run to run, which our zero-tolerance regression comparison
flags. Reproduced directly against the DLL:
dasumon an identical430-element vector returns bitwise-different results at different 64-byte
misalignments, and likewise
sasumanddzasumat various sizes ≥ 256(single-threaded build, so this is purely the alignment peel, not threading).
Fix
Drop the alignment peel and use unaligned loads (
_mm512_loadu_p?,_mm512_maskz_loadu_p?) throughout, so the summation order is a function ofthe vector length alone. Touched kernels:
dasum_microk_skylakex-2.c,sasum_microk_skylakex-2.c(scalar peel)casum_microk_skylakex-2.c,zasum_microk_skylakex-2.c,csum_microk_skylakex-2.c,zsum_microk_skylakex-2.c(masked align header)On AVX-512-capable hardware, unaligned load instructions on addresses that
happen to be aligned cost the same as aligned loads; only genuinely split
cache lines pay a small penalty, which is negligible for these level-1
reductions. (These kernels only require alignment because
_mm512_load_p?faults on unaligned addresses — the peel was correctness scaffolding for the
aligned loads, not an optimisation in itself.)
Testing
dasum/sasum/dzasumnow return bitwise-identical results across buffer offsets 0..56 (mod 64)
for sizes 61..4001, where the unpatched build differed;
ddot,dnrm2and
dgemvwere alignment-invariant before and after.DYNAMIC_ARCH=1 USE_THREAD=0 NOFORTRAN=1build (gcc 16.1,MSYS2/MinGW64), dispatching Cooperlake kernels on Zen 5; downstream
application regression suite that previously showed run-to-run eigenvector
differences is now clean.