Skip to content

ggml-cpu: enable Q2_0 VNNI kernel on AVX-VNNI-only CPUs #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gondoi wants to merge 1 commit into
base: prism
Choose a base branch
from
Open

ggml-cpu: enable Q2_0 VNNI kernel on AVX-VNNI-only CPUs #76

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions ggml/src/ggml-cpu/arch/x86/quants.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,17 @@ void ggml_vec_dot_q2_0_q8_0(int n, float * GGML_RESTRICT s, size_t bs, const voi

float sumf = 0.0f;

#if defined(__AVX512VNNI__) && defined(__AVX512VL__)
// AVX-512-VNNI: unpack 2-bit codes c in {0,1,2,3} (value = c-1), then
#if (defined(__AVX512VNNI__) && defined(__AVX512VL__)) || defined(__AVXVNNI__)
// VNNI: unpack 2-bit codes c in {0,1,2,3} (value = c-1), then
// dot((c-1), qy) = dpbusd(c, qy) - dpbusd(1, qy).
// The kernel only uses 256-bit registers, so it runs unchanged on

@bri-prism bri-prism Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not quite right: the block also uses XMM ops (the idx vectors and the movq load below), and VNNI plus 256-bit registers is not a sufficient condition, which is exactly how the gate above ended up too wide. Suggest: "uses only SSE/AVX2 ops (no 512-bit or AVX512-only instructions); requires AVX2". I would also drop the CPU model list, it is already stale (Meteor Lake, Sierra Forest).

// AVX-VNNI-only CPUs (e.g. Intel Alder/Raptor Lake, where AVX512 is
// unavailable); the AVX-VNNI intrinsic differs only in name.
#if defined(__AVX512VNNI__) && defined(__AVX512VL__)

@bri-prism bri-prism Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider dropping the macro and calling mul_sum_us8_pairs_float (defined ~470 lines up in this file) with the whole block gated on __AVX2__ instead. It is bit-exact for this input range: codes are masked to 0..3, so its maddubs fallback cannot saturate, and the sums stay well inside exact-float territory. That removes the fifth copy of this exact dpbusd dispatch in the tree and extends the speedup to every AVX2 CPU without VNNI (Haswell through Rocket Lake, Zen 1 to 3), including the shipped haswell/skylakex variants, which this PR currently leaves on the scalar loop.

#define GGML_Q2_0_DPBUSD(acc, a, b) _mm256_dpbusd_epi32(acc, a, b)
#else

@bri-prism bri-prism Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the macro stays, make this #elif defined(__AVXVNNI__) with a trailing #else + #error, matching the other dpbusd dispatches in quants.c, repack.cpp, and sgemm.cpp. The bare #else silently emits the AVX-VNNI intrinsic for any future config that reaches it without the feature, and __AVX512VNNI__ without __AVX512VL__ is already constructible today with -DGGML_AVX512_VNNI=ON without -DGGML_AVX512=ON.

#define GGML_Q2_0_DPBUSD(acc, a, b) _mm256_dpbusd_avx_epi32(acc, a, b)
#endif
const __m256i ones = _mm256_set1_epi8(1);
const __m128i idxlo = _mm_setr_epi8(0,0,0,0,1,1,1,1,2,2,2,2,3,3,3,3);
const __m128i idxhi = _mm_setr_epi8(4,4,4,4,5,5,5,5,6,6,6,6,7,7,7,7);
Expand All @@ -591,12 +599,13 @@ void ggml_vec_dot_q2_0_q8_0(int n, float * GGML_RESTRICT s, size_t bs, const voi
r0 = _mm256_and_si256(_mm256_srli_epi16(_mm256_mullo_epi16(r0, mul), 6), three);
r1 = _mm256_and_si256(_mm256_srli_epi16(_mm256_mullo_epi16(r1, mul), 6), three);
__m256i codes = _mm256_permute4x64_epi64(_mm256_packus_epi16(r0, r1), 0xD8); // 32 codes in order
const int dp = hsum_i32_8(_mm256_dpbusd_epi32(_mm256_setzero_si256(), codes, qy));
const int sy = hsum_i32_8(_mm256_dpbusd_epi32(_mm256_setzero_si256(), ones, qy));
const int dp = hsum_i32_8(GGML_Q2_0_DPBUSD(_mm256_setzero_si256(), codes, qy));

@bri-prism bri-prism Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two reductions can be one: keep the dpbusd results as vectors and do hsum_i32_8(_mm256_sub_epi32(dpv, syv)). Bit-identical (lanes are bounded well inside int32) and it halves the reduction work, which is latency-serialized through vmovd. The q1_0 sibling below goes further with a float accumulator and a single hsum per call if you want to match that idiom.

const int sy = hsum_i32_8(GGML_Q2_0_DPBUSD(_mm256_setzero_si256(), ones, qy));
sumi += d1 * (float)(dp - sy);
}
sumf += d0 * sumi;
}
#undef GGML_Q2_0_DPBUSD
#else
for (int i = 0; i < nb; i++) {

@bri-prism bri-prism Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing, but this PR rewrites the exact #if/#else around it: this scalar branch is byte-for-byte identical to ggml_vec_dot_q2_0_q8_0_generic, and the ARM version of this function already delegates instead (arch/arm/quants.c). ggml_vec_dot_q2_0_q8_0_generic(n, s, bs, vx, bx, vy, by, nrc); return; deletes the duplicate and leaves one copy to maintain.

const float d0 = GGML_CPU_FP16_TO_FP32(x[i].d);
Expand Down