Fix AVX2 codegen SIGILL on non-AVX-512 CPUs - #70
Merged
Conversation
The hwygen AVX2 code path emitted instructions that crash with SIGILL on CPUs that support AVX2 but not AVX-512, and broadcast constants that ran before CPU-feature dispatch on CPUs without AVX2 at all. Float64 Log (issues #67, #68): GetExponent generated Int64x4.ShiftAllRight (VPSRAQ) and ConvertExponentToFloat generated Int64x4.ConvertToFloat64 (VCVTQQ2PD), both AVX-512-only. For the AVX2 target only, extract the exponent with an unsigned logical shift (VPSRLQ) and convert int64->float64 via the magic-number trick (VPADDQ + reinterpret + VSUBPD), which is exact over the exponent range. NEON keeps its native signed shift; AVX-512 keeps the native ops since it only runs where they exist. Hoisted constants (issue #69): the sync.Once lazy-init pattern was applied only to AVX-512, so AVX2 broadcast constants still ran at package-init time (before init() CPU detection) and SIGILL'd on non-AVX2 CPUs. Apply lazy init to AVX2 as well, with target-specific helper names since the AVX2 and AVX512 files share a package and build tag. Regenerated the affected *_avx2.gen.go/*_avx512.gen.go files and added a float64 LogTransform regression test (the existing test only covered float32, which never exercised the AVX-512-only instructions).
3 tasks
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
Fixes three SIGILL crashes caused by the hwygen AVX2 code path. All fixes are at the code-generator level so they survive regeneration; the affected
*_avx2.gen.go/*_avx512.gen.gofiles are regenerated in this PR.Logemits AVX-512 instructions.GetExponentgeneratedInt64x4.ShiftAllRight(VPSRAQ) andConvertExponentToFloatgeneratedInt64x4.ConvertToFloat64(VCVTQQ2PD) — both AVX-512-only — soLogTransformon[]float64crashed on AVX2-only CPUs (AMD EPYC 7763, Windows runners). The float32 path was unaffected because it usesVPSRAD/VCVTDQ2PS(AVX). For the AVX2 target only, the exponent is now extracted with an unsigned logical shift (VPSRLQ) and int64→float64 uses the magic-number trick (VPADDQ+ reinterpret +VSUBPD), which is exact over the exponent range. NEON keeps its native signed shift; AVX-512 keeps the native ops since it only runs where they exist.sync.Oncelazy-init pattern was applied only to AVX-512, so AVX2 broadcast constants still ran in package-levelvarinitializers (beforeinit()CPU detection) and SIGILL'd on CPUs without AVX2. Lazy init now also applies to AVX2, with target-specific helper names since the AVX2 and AVX512 files share a package and build tag.Changes
cmd/hwygen/transformer.go,cmd/hwygen/transformer_ops.go: AVX2-only float64 exponent extraction + int64→float64 conversion.cmd/hwygen/emitter.go: extend lazy-init of hoisted constants to AVX2.*_avx2.gen.go/*_avx512.gen.go.hwy/contrib/algo/transform_test.go: addedTestLogTransformFloat64(the existing test only covered float32, which never hit the AVX-512-only instructions).Verification
llvm-objdumpof the regeneratedBaseLogVec_avx2_Float64(and a scan of all_avx2_functions) confirms no AVX-512 (EVEX) instructions remain.[-1023, 1024].GOEXPERIMENT=simd.Note: the AVX2 path could not be executed locally (arm64 host; Rosetta lacks AVX2), so runtime confirmation relies on the disassembly check plus the verified arithmetic — CI on the AVX2-only runner exercises it.
Test plan
GOEXPERIMENT=simd go test ./hwy/contrib/algo/ -run TestLogTransformpasses on amd64Closes #67
Closes #68
Closes #69