Tests: extend test-quantize-fns to test nrc=2 (i8mm) kernels

[Rohanjames1997]

The i8mm kernels require nrc == 2 to actually get triggered.
But currently, the CI's test-quantize-fns.cpp tests only the scenario where nrc=1.

[Rohanjames1997]

Hi @ggerganov , could you trigger the CI again?
Thanks!

[Rohanjames1997]

@ggerganov thanks for running the CI!
The failing checks seem unrelated to this PR. Weirdly enough, the "Build on RISCV Linux Machine by Cloud-V" failed at the repo checkout step itself.

Let me know if I can make any more changes

[ggerganov]

I think we need to expand the input data depending on nrc?

[Rohanjames1997]

Sure. Can you tell me an appropriate test_size for nrc=2 ? Would it just be double?

[Rohanjames1997]

I went through the logs of some of the failed tests and it looks like they may be unrelated to the modified CI in this PR. @ggerganov or the team, can you help me with this/ tell me if I'm missing something?

Thank you!

[snadampal]

@Rohanjames1997 , how about you rebase the PR to the main? that will trigger CI and you can check if the current failures have been fixed on the mainline already.

[Rohanjames1997]

Some CI failed again due to the Cloudflare/Github outage yesterday.
Rebased it again 🤞

[snadampal]

the x64_cpu_amx and macOS-latest-swift CI failures don't seem to be related to this PR.
can someone trigger the CI again and merge this PR if the failures are not related? Thank you!

[snadampal]

what's the next step to get this PR merged?

[Rohanjames1997]

Fixed the nrc=2 test to properly prepare independent data for each row slice.

Buffers are now sized with ggml_row_size(), each row is quantized separately from distinct source data, and correct bx/by/bs parameters are passed matching the real mul_mat calling convention.

All 4 elements of the 2x2 output matrix are validated against float references.

[chaxu01]

cc: @taronaeo @max-krasnyansky

[taronaeo]

/bot review

[ggml-gh-bot]

Automated code review

Review of PR #16234 (commit f48b0ba) - extends tests/test-quantize-fns.cpp with an nrc == 2 dot-product test for the ARM i8mm kernels.

Verified correct

• The expected output layout (s[0]=dot(vx0,vy0), s[1]=dot(vx1,vy0), s[bs]=dot(vx0,vy1), s[bs+1]=dot(vx1,vy1)) matches the authoritative consumer in ggml/src/ggml-cpu/ggml-cpu.c (ggml_compute_forward_mul_mat_non_quantized_case, bs = 16, rows stepped by nrows), and matches what the i8mm kernels store (vst1_f32(s, ...) / vst1_f32(s + bs, ...) in ggml/src/ggml-cpu/arch/arm/quants.c for q4_0/q4_1/q8_0/q4_K/q6_K).
• The qfns_cpu->nrows == 2 gate matches exactly the types whose traits set nrows = 2 under __ARM_FEATURE_MATMUL_INT8 (q4_0, q4_1, q8_0, q4_K, q6_K), so non-ARM/non-i8mm builds are untouched.
• Buffer sizing is sound: bx/by derived from ggml_row_size() with pad, rows quantized separately from distinct source data (different offsets and amplitudes), all 4 matrix elements checked against double-precision references. Distinct amplitudes (2.0/1.0/1.5) mean the test would catch transposed-row/lane bugs, which an aggregate check would not.
• std::isfinite guard plus INFINITY initialization correctly catches kernels that write NaN or fail to write.

Blocking (defeats the stated PR goal)

(point 1) The PR is titled "Extend CI for i8mm kernels", but the diff only touches the test file - no workflow is added or changed to build and run test-quantize-fns with i8mm. As is, the new code path almost certainly never executes in CI:

• The ubuntu-24.04-arm job in .github/workflows/build-cpu.yml builds with GGML_NATIVE=ON and runs ctest -L main on GitHub's arm64 runners, which are Ampere Altra (ARMv8.0-A). Altra does not implement i8mm, so __ARM_FEATURE_MATMUL_INT8 is not defined, nrows stays 1, and the new branch is dead code there.
• The only workflow that builds with i8mm flags (build-android.yml, -DGGML_CPU_ARM_ARCH=armv8.5-a+fp16+i8mm) only packages artifacts and runs no tests.
• The Snapdragon workflow builds with i8mm and runs on real i8mm-capable hardware (SM8750/SM8850/QCS9075M), but its test step runs model-level tests only (no ctest), and its paths filter would not even trigger on a tests/**-only change.

Note that simply forcing -DGGML_CPU_ARM_ARCH=...+i8mm on the Altra runner is not a fix: the vmmlaq instructions would SIGILL on non-i8mm hardware, so coverage requires matching hardware. Concretely, I'd suggest verifying the claim first (run the arm64 job and check whether the new dot product error (nrc=2) output ever appears - it only prints under -v), and then either adding a test-quantize-fns run to the Snapdragon QDC job (plus adding tests/** / ggml/src/ggml-cpu/** to its path filter), or another job on i8mm-capable hardware. Without that, this is a good test improvement but not a CI extension, and the PR should be retitled/rescoped accordingly.

Will slow the review

(point 2) dot_product_error now has a mode-switch signature: 7 params where test_data3/test_data4 are nullptr in one mode and the nrc flag picks between two disjoint code paths. A separate dot_product_error_nrc2() helper would remove the nullable params and the mode flag at no cost in reuse (the shared part is 4 lines). Minor, but it keeps each function single-purpose.

Nits

(point 3) In the err lambda, std::isfinite(e) ? e : INFINITY is redundant: the caller's !(err < max_allowed_error) already treats NaN as failure, and fabsf(NaN - ref)/test_size is NaN. A plain fabsf(val - ref)/test_size would do. Optional.

(point 4) generate_data's new amplitude default argument is fine, but the 2.0f case is now spelled at two call sites implicitly and two explicitly - fine either way, just noting the default arg is C++-only, which is OK for this .cpp file.

Summary: the test logic itself is correct and well designed (it mirrors the real mul_mat calling convention including bs=16 and per-row strides). The one real issue is (point 1): as submitted, nothing in CI will actually exercise the new path, which contradicts the PR's purpose. Please add the corresponding workflow change or confirm which runner provides i8mm coverage.

This review was generated automatically by pi coding agent using zai-org/GLM-5.3. It may contain mistakes. Maintainers make the final call.

[Rohanjames1997]

Addressing the "Blocking" comment:

The ubuntu-24.04-arm job runs on GitHub's arm64 runners, which are Ampere Altra (ARMv8.0-A). Altra does not implement i8mm.. and the new branch is dead code there.

True. This PR future-proofs the CI.
FWIW, I've changed the PR title that the bot had issue with...

[Rohanjames1997]

@taronaeo let me know your thoughts.
CC: @chaxu01

[taronaeo]

Since we are moving to HuggingFace runners, I was wondering if HuggingFace Jobs has any ARM CPU that supports nrc = 2 so that we can actually run this? I don't seem to see it in this list:

$ hf jobs hardware

Hint: A new version of huggingface_hub (1.30.0) is available! You are using version 1.23.0.
To update, run: hf update
NAME            PRETTY NAME            CPU      RAM     STORAGE  ACCELERATOR              COST/MIN COST/HOUR
--------------- ---------------------- -------- ------- -------- ------------------------ -------- ---------
cpu-basic       CPU Basic              2 vCPU   16 GB   50 GB                             $0.0002  $0.01    
cpu-upgrade     CPU Upgrade            8 vCPU   32 GB   50 GB                             $0.0005  $0.03    
cpu-performance CPU Performance        32 vCPU  256 GB  1024 GB                           $0.0317  $1.90    
cpu-xl          CPU XL                 16 vCPU  124 GB  1000 GB                           $0.0167  $1.00    
t4-small        Nvidia T4 - small      4 vCPU   15 GB   50 GB    1x T4 (16 GB)            $0.0067  $0.40    
t4-medium       Nvidia T4 - medium     8 vCPU   30 GB   100 GB   1x T4 (16 GB)            $0.0100  $0.60    
a10g-small      Nvidia A10G - small    4 vCPU   15 GB   110 GB   1x A10G (24 GB)          $0.0167  $1.00    
a10g-large      Nvidia A10G - large    12 vCPU  46 GB   200 GB   1x A10G (24 GB)          $0.0250  $1.50    
a10g-largex2    2x Nvidia A10G - large 24 vCPU  92 GB   1000 GB  2x A10G (48 GB)          $0.0500  $3.00    
a10g-largex4    4x Nvidia A10G - large 48 vCPU  184 GB  2000 GB  4x A10G (96 GB)          $0.0833  $5.00    
a100-large      Nvidia A100 - large    12 vCPU  142 GB  1000 GB  1x A100 (80 GB)          $0.0417  $2.50    
a100x4          4x Nvidia A100         48 vCPU  568 GB  4000 GB  4x A100 (320 GB)         $0.1667  $10.00   
a100x8          8x Nvidia A100         96 vCPU  1136 GB 8000 GB  8x A100 (640 GB)         $0.3333  $20.00   
h200            Nvidia H200            23 vCPU  256 GB  3000 GB  1x H200 (141 GB)         $0.0833  $5.00    
h200x2          Nvidia H200            46 vCPU  512 GB  6000 GB  2x H200 (282 GB)         $0.1667  $10.00   
h200x4          Nvidia H200            92 vCPU  1024 GB 12000 GB 4x H200 (564 GB)         $0.3333  $20.00   
h200x8          Nvidia H200            184 vCPU 2048 GB 24000 GB 8x H200 (1128 GB)        $0.6667  $40.00   
rtx-pro-6000    Nvidia RTX PRO 6000    23 vCPU  256 GB  475 GB   1x RTX PRO 6000 (96 GB)  $0.0458  $2.75    
rtx-pro-6000x2  Nvidia RTX PRO 6000    46 vCPU  512 GB  950 GB   2x RTX PRO 6000 (192 GB) $0.0917  $5.50    
rtx-pro-6000x4  Nvidia RTX PRO 6000    92 vCPU  1024 GB 1900 GB  4x RTX PRO 6000 (384 GB) $0.1833  $11.00   
rtx-pro-6000x8  Nvidia RTX PRO 6000    184 vCPU 2048 GB 3800 GB  8x RTX PRO 6000 (768 GB) $0.3667  $22.00   
l4x1            1x Nvidia L4           8 vCPU   30 GB   400 GB   1x L4 (24 GB)            $0.0133  $0.80    
l4x4            4x Nvidia L4           48 vCPU  186 GB  3200 GB  4x L4 (96 GB)            $0.0633  $3.80    
l40sx1          1x Nvidia L40S         8 vCPU   62 GB   380 GB   1x L40S (48 GB)          $0.0300  $1.80    
l40sx4          4x Nvidia L40S         48 vCPU  382 GB  3200 GB  4x L40S (192 GB)         $0.1383  $8.30    
l40sx8          8x Nvidia L40S         192 vCPU 1534 GB 6500 GB  8x L40S (384 GB)         $0.3917  $23.50   
Hint: Use `hf jobs run --flavor <name> ...` to request a specific hardware flavor.

[CISC]

Since we are moving to HuggingFace runners, I was wondering if HuggingFace Jobs has any ARM CPU that supports nrc = 2 so that we can actually run this?

No ARM runners, not sure if/when there will be any.

[taronaeo]

I think we are good to merge? We will still need to find an ARM CI runner that has support for nrc = 2 though, otherwise this is just an unused test.

[CISC]

I think we are good to merge? We will still need to find an ARM CI runner that has support for nrc = 2 though, otherwise this is just an unused test.

Any idea on what does?

[taronaeo]

Did a quick google search, looks like SMMLA instruction support is what we need to look out for.

For AWS, it seems like Graviton 3 through 5 supports this instruction.

If I recall correctly, we have an existing KleidiAI self-hosted CI runner that uses AWS right? Let me check if it supports the instruction.

[chaxu01]

Yes, we do have Graviton 4 available as an Arm-hosted runner for ggml-org/llama.cpp.

[taronaeo]

Yes, we do have Graviton 4 available as an Arm-hosted runner for ggml-org/llama.cpp.

Yay OK let's merge this and work making a CI run nrc = 2 in another PR.

[taronaeo]

Merging in a few hours if no further comments. CI failures appear to not be related.

[max-krasnyansky]

@zhiyuan8 would be good to enable these in the Snapdragon CI