Emulate AVX-512-only int64 ops (VPSRAQ/VPMULLQ) in the AVX2 path - #71
Merged
Conversation
A follow-up to the AVX2 SIGILL fixes: a library-wide disassembly scan found that several int64 functions still emitted AVX-512-only instructions in their AVX2 path, which SIGILL on AVX2-only CPUs: - Int64x4.ShiftAllRight -> VPSRAQ (signed 64-bit arithmetic shift) - Int64x4.Mul -> VPMULLQ (64-bit packed multiply) Affected: image RCT (forward/inverse), int64 radix sort, and int64 wavelet 5/3 lifting. hwygen now redirects these to scalar hwy emulation wrappers for the AVX2 target only (matching the existing Max/Min VPMAXSQ/VPMINSQ handling). The AVX-512 target keeps the native instructions since it only runs where they exist; NEON/fallback are unaffected. The emulation wrappers (Mul_AVX2_Int64x4/Uint64x4, ShiftAllRight_AVX2_Int64x4) use the same scalar store/compute/load pattern as the existing Max/Min wrappers. Also fixes hwygen import detection so the hwy import is emitted when a function's only hwy reference is one of these redirected int64 ops (the previous Max/Min redirect only worked when the function used another hwy op). Verified by disassembling a binary linking all contrib packages: no EVEX (AVX-512) instructions remain in any _avx2 function. Added correctness tests for the emulation helpers.
Disassembles a binary linking every hwy package and fails if any go-highway function that isn't AVX-512-gated emits an EVEX-prefixed (0x62) instruction. This makes the SIGILL bug class from #67-#69 and this PR self-detecting instead of discovered reactively per-op. Scoped to go-highway symbols excluding "avx512" names, so it also catches EVEX leaking into generic/fallback helpers an AVX2-only CPU could reach, while excluding our runtime-gated AVX-512 functions and Go's own feature-gated runtime.asyncPreempt. Runs via go test and a dedicated CI step that fails if the scan skips (missing llvm-objdump).
… dirs - Move the cross-compile+disassemble integration test behind the "evexguard" build tag so it runs once in the dedicated CI step (and on-demand via `go test -tags evexguard`) instead of on every `go test ./...`. The fast parser unit test still runs by default. - Treat a non-writable repo root (MkdirTemp failure) as a skip, matching the other prerequisite checks, so a read-only checkout can't spuriously fail the test. - gitignore evexguard-scan-*/ so a leftover temp dir from an interrupted run can't be accidentally committed.
The step previously failed only on "--- SKIP", but a -run that matches nothing (build-tag or test-name drift) prints "no tests to run" and exits 0 -- letting the guard go green while scanning nothing. Gate on a "--- PASS: TestNoEVEXInAVX2ReachableFunctions" line instead, so a skip, a typo, or a tag mismatch all fail the build.
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
1 participant
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.
Summary
Follow-up to #70. A library-wide disassembly scan (a binary linking every
hwy/contrib/*package, disassembled withllvm-objdump) showed that several int64 functions still emitted AVX-512-only instructions in their AVX2 code path, which SIGILL on AVX2-only CPUs (the same EPYC 7763 / GitHub Actions runners from #68/#69):VPSRAQInt64x4.ShiftAllRight(signed arithmetic shift)image.BaseForwardRCT/BaseInverseRCT,sort.BaseRadixPass/BaseRadixPassSigned,wavelet.BaseLiftPredict53/LiftUpdate53/Synthesize53Core/Synthesize53CoreColsVPMULLQInt64x4.Mul(64-bit multiply)image.BaseForwardRCTThese are the same root-cause class as #67/#68 but were outside that PR's scope.
Changes
transformer_ops.go): for the AVX2 target only, redirect 64-bit-intMuland signed-int64 arithmeticShiftAllRighttohwyemulation wrappers — extending the existing block that already does this forMax/Min(VPMAXSQ/VPMINSQ). Unsigned logical shifts (VPSRLQ) and left shifts (VPSLLQ) are AVX2-native and left untouched.emitter.go): emit thehwyimport when a function's onlyhwyreference is one of these redirected int64 ops. Previously theMax/Minredirect only got the import incidentally (when the function also used anotherhwyop);sort's radix pass uses only the shift and exposed the gap.ops_avx2.go):Mul_AVX2_Int64x4,Mul_AVX2_Uint64x4,ShiftAllRight_AVX2_Int64x4, using the same scalar store/compute/load pattern as the existingMax_AVX2_Int64x4/Min_AVX2_Int64x4. Scalar is provably correct by inspection (Go's>>on a signed value is arithmetic;*is the low-64 product) — important since the AVX2 path can't be executed on the arm64 dev host.image/sort/waveletAVX2 files (AVX-512/NEON/fallback unchanged). Addedops_avx2_test.go.Behavior
VPSRAQ/VPMULLQ(they only run where those exist; 23 such instructions remain in_avx512functions).Verification
_avx2function.GOEXPERIMENT=simd).hwyemulation-helper tests compile for amd64 (run on CI).Test plan
GOEXPERIMENT=simd go test ./hwy/exercises the newTestMulAVX2*/TestShiftAllRightAVX2Int64x4