perf(sse41): add SSE4.1 fallback tier for olmoe and qwen36 int8 GEMV - #1239
Conversation
Verified bit-for-bit identical to scalar reference over 10k random int8 pairs. Uses c/sse41_kernels.h for shared primitives (COLIBRI_FMA macro, 128-bit loadu/storeu/min/max/prefetch). The FMA macro is the #1 source of correctness risk on Sandy Bridge (no hardware FMA) - double-rounding artifact in 1-2 ULP. Reference implementation for porting the same pattern to c/kimi_k3.c, c/inkling.c, c/colibri.c, c/deepseek_v4.c via the same C struct.
…_kernels.h matmul_q_gs (gsgemv.h) and matmul_q (qgemv.h) were inline in qwen36.c, each with its own scalar/AVX2 branches. Extract both verbatim into their own headers and add a new SSE4.1 branch to each, routed through the sse41_kernels.h primitives introduced alongside olmoe's dot_i8_16 in the previous commit -- qwen36 becomes a third consumer of that header rather than a third copy of the intrinsics. Unlike dot_i8_16's pure integer arithmetic, these are float GEMVs: their SSE4.1 reduction tree is not bit-identical to the scalar/AVX2 tiers (float addition is not associative), so the new SSE4.1 branches are the one deliberate exception to this engine's byte-identical requirement. New tests test_gsgemv_sse41/test_qgemv_sse41 check them by tolerance (diff <= 1e-5 + 1e-4*|ref|) instead of memcmp; the plain test_gsgemv/test_qgemv stay memcmp-exact against the unchanged scalar/AVX2 tiers. The SSE4.1 test reference needed its own tail-drop branch to match gsgemv.h's <8-element drop -- otherwise that edge case is structurally unwinnable. Also add test_olmoe_dot_i8_16(_sse41): dot_i8_16 (previous commit) is pure integer arithmetic, so unlike the above it's checked memcmp-exact against a scalar reference over 10k random int8 pairs plus saturation-edge cases. This was previously asserted only in a commit message with no automated test in the repo to back it.
|
CI ran for the first time on this branch (there were no runs at all before — the workflow had never been queued, so I updated the branch onto current dev to trigger it). 21 pass, 2 fail, and both failures are in this PR's own test scaffolding rather than in the kernels. I reproduced and isolated both locally, so here is the diagnosis rather than just the log. 1. Sanitizers:
|
…s to x86_64 Two CI failures flagged in review on JustVugg#1239, both in test scaffolding, not the kernels. - test_gsgemv's AVX2 reference (reference_gs_gemv) computed the per-group scale step as `acc += x * sc[gi]`, an implicit multiply-add the compiler only fuses into the same hardware FMA gsgemv.h's real kernel uses explicitly (fmaf) when contraction is on -- true at -O2/-O3 with the default -ffp-contract=fast, false at -O1 (where ASan/UBSan CI builds) or with contraction off even at -O3. The memcmp-exact comparison held by accident, not by construction. Switched the reference to the same explicit fmaf so it matches regardless of optimisation flags. Verified: ALL PASS under ASan/UBSan at -O1 and at -O3 -ffp-contract=off, both of which previously failed. - tests/test_gsgemv_sse41 and tests/test_qgemv_sse41 pass -msse4.1/-mno-avx2/-mno-fma unconditionally, flags gcc/clang only recognise on x86 -- unconditionally in TEST_BINS they took down `make check` entirely on arm64 (e.g. macOS: "unsupported option '-msse4.1' for target arm64-apple-darwin"). Excluded by default and appended back only under X86_64, mirroring the existing COLI_V4_SUPPORTED pattern in the same file. Being an allow-list rather than an arm64-specific deny-list, this also correctly excludes PPC64 and any future target without needing a target-specific branch. Windows (x86_64, UCRT64) is unaffected -- confirmed via the same TARGET_CPU detection used elsewhere in this Makefile, which reads the compiler's -dumpmachine triple rather than uname, so it is correct there too. Verified make check green end to end after rebasing onto the current PR branch state (JustVugg merged dev into it to get CI running).
There was a problem hiding this comment.
Great job @jtinbergen, but please add the new test.o files and test.exes to the main or tests folder .gitignore to avoid something like #874 happening again
|
Thanks for taking a look. Could you point to which specific files would escape the current
Happy to add a rule if there's a concrete file I'm missing — just want to fix the actual gap rather than a rule that duplicates existing coverage. |
Okay... Approved? |
|
Reviewed and tested locally against current
The one thing missing is CI, and it has never run: as a first-time contribution the workflows wait for a maintainer, and the run objects only appear once the branch moves. I tried to create them by pushing a merge of Could you push one commit to |
…equisites test_makefile_deps.py (added on dev) flagged that olmoe$(EXE) and qwen36$(EXE) include headers this PR introduced without listing them as prerequisites -- editing sse41_kernels.h, gsgemv.h or qgemv.h alone would make report success and leave a stale binary.
|
Ten days of drift on a branch I verified myself. On 2026-09-07 I merged dev into this branch, found no conflicts, and confirmed all three tests green including bit-exactness over 10,000 random pairs. Since then Could you forward-merge dev once more? On the open CHANGES_REQUESTED about gitignoring the test binaries: your rebuttal is right and I am clearing it. Generated test binaries belong in |
# Conflicts: # c/Makefile # c/qwen36.c
CI's macOS job failed the same way the original arm64 fix was meant to prevent: this third _sse41 target (forces -msse4.1 -mno-avx2 -mno-fma) was never added to the x86_64-only TEST_BINS guard, only test_gsgemv_sse41 and test_qgemv_sse41 were. Verified no other test rule uses an x86-only -m flag (swept the whole Makefile), and that TARGET_CPU=arm64 now excludes all three _sse41 targets.
…s to x86_64 Two CI failures flagged in review on JustVugg#1239, both in test scaffolding, not the kernels. - test_gsgemv's AVX2 reference (reference_gs_gemv) computed the per-group scale step as `acc += x * sc[gi]`, an implicit multiply-add the compiler only fuses into the same hardware FMA gsgemv.h's real kernel uses explicitly (fmaf) when contraction is on -- true at -O2/-O3 with the default -ffp-contract=fast, false at -O1 (where ASan/UBSan CI builds) or with contraction off even at -O3. The memcmp-exact comparison held by accident, not by construction. Switched the reference to the same explicit fmaf so it matches regardless of optimisation flags. Verified: ALL PASS under ASan/UBSan at -O1 and at -O3 -ffp-contract=off, both of which previously failed. - tests/test_gsgemv_sse41 and tests/test_qgemv_sse41 pass -msse4.1/-mno-avx2/-mno-fma unconditionally, flags gcc/clang only recognise on x86 -- unconditionally in TEST_BINS they took down `make check` entirely on arm64 (e.g. macOS: "unsupported option '-msse4.1' for target arm64-apple-darwin"). Excluded by default and appended back only under X86_64, mirroring the existing COLI_V4_SUPPORTED pattern in the same file. Being an allow-list rather than an arm64-specific deny-list, this also correctly excludes PPC64 and any future target without needing a target-specific branch. Windows (x86_64, UCRT64) is unaffected -- confirmed via the same TARGET_CPU detection used elsewhere in this Makefile, which reads the compiler's -dumpmachine triple rather than uname, so it is correct there too. Verified make check green end to end after rebasing onto the current PR branch state (JustVugg merged dev into it to get CI running).
# Conflicts: # c/Makefile
3 participants
Summary
c/sse41_kernels.h, a shared 128-bit SIMD primitives header (COLIBRI_FMAemulation, loadu/storeu, min/max, prefetch) for hosts with SSE4.1 but no AVX2
(Sandy Bridge and similar)
c/olmoe.c(dot_i8_16), verified bit-for-bitidentical to the scalar reference over 10k random int8 pairs
matmul_q_gsintoc/gsgemv.handmatmul_qintoc/qgemv.h(previously inline in
qwen36.c), and add a new SSE4.1 tier to each, routedthrough the shared header rather than a third copy of the intrinsics
sse41_kernels.h, alongside olmoe (this PR);the header's own docstring also names
deepseek_v4.c,colibri.c,kimi_k3.c,and
inkling.cas candidates, none of which are touched hereScope
This adds one fallback tier below AVX2/FMA, above scalar, for the three
kernels touched here: olmoe's dot product and qwen36's two GEMVs.
The SSE4.1 branches in
gsgemv.h/qgemv.hare new code, not a port of existingoutput: their reduction tree differs from the scalar/AVX2 tiers (float addition
is not associative), so they're the one deliberate exception to this engine's
byte-identical requirement -- checked by tolerance in the new tests, not memcmp.
Scalar and AVX2 tiers are unchanged and remain memcmp-exact.
Measured
Build (both from commit 1f9f2d8):
Run harness, identical form for every measured run:
256= expert cache slots/layer (256/256 = full per-layer residency -- the code'sown bare default is 16, which starves this model's 256-expert layers and produces
non-representative numbers; worth stating explicitly since this PR's own text
previously omitted it).
4= quant bits, matching the snapshot. NoOMP_NUM_THREADSoverride (default 22 threads).Storage: the snapshot sits on NVMe -> LUKS (dm-crypt) -> LVM -> ext4.
Warm-up policy: expert-weight reads are deliberately evicted from the OS page
cache after each use (
pread+posix_fadvise(DONTNEED), by design, to boundRSS), so standard page-cache warm-up does not apply to them -- a rerun is not
faster. The 200/500/800-token windowed-delta protocol (see below) is itself the
within-run warm-up control: the expert cache fills as generation progresses, and
the 500->800 "steady" window isolates throughput after that fill completes. Runs
across the two builds were interleaved in strict alternating order (scalar,
SSE4.1, scalar, SSE4.1, ...) to cancel any residual directional bias.
Methodology: generate 200, then 500, then 800 tokens from the same prompt (each a
fresh process, not a continuation) -- the first 200 tokens of the 800-token run do
identical work to the standalone 200-token run, so the wall-clock delta between
windows isolates tail-window throughput. The 500->800 delta is reported as
"steady." 5 repetitions per build.
Intel Core Ultra 7 155H (22 threads), qwen36-i4-gs64 snapshot:
This machine had sustained background load throughout measurement (other
processes competing for CPU; 1-minute load average ranged 6.2-13.2 across the
run) -- not an idle box. The numbers above are honest for that condition, not a
best-case result.
Confirmed via objdump on the compiled binaries (not inferred from build flags)
that the SSE4.1 build's
matmul_q/matmul_q_gsOpenMP-outlined bodies contain 0ymm/vfmaddinstructions and do containcvtepi8/pmovsxconversions, and thescalar build's equivalent bodies contain neither -- the SSE4.1 branch is what
actually executed, on the actual binary.
Verification
make check: pass, 0 new warnings (4 pre-existing -Wformat-truncationwarnings in unrelated code, confirmed identical before/after)
test_gsgemv/test_qgemv(AVX2/scalar, memcmp-exact) andtest_gsgemv_sse41/test_qgemv_sse41(forced-msse4.1 -mno-avx2 -mno-fma,tolerance
diff <= 1e-5 + 1e-4*|ref|): all pass, including the SSE4.1 tier'sown <8-element tail-drop case (P5), which required the test reference to
carry a matching SSE4.1 branch -- otherwise that case is structurally
unwinnable
test_olmoe_dot_i8_16/test_olmoe_dot_i8_16_sse41:dot_i8_16ispure integer arithmetic (no rounding), so checked memcmp-exact -- not
tolerance -- against a scalar reference over 10k random int8 pairs plus
saturation-edge cases (-128/127), both natively and forced onto the SSE4.1
path; this is the automated backing for the "bit-for-bit identical" claim
above, which previously rested only on the original commit message
SNAP=./glm_tiny TF=1 ./colibri 64 16 16): 32/32teacher-forcing + 20/20 greedy