Skip to content

ggml-metal: fix copy bounds and F32 precision, optimize short-K matmul - #609

Open
davidxifeng wants to merge 3 commits into
0xShug0:mainfrom
davidxifeng:codex/metal-f32-copy-small-k
Open

davidxifeng wants to merge 3 commits into
0xShug0:mainfrom
davidxifeng:codex/metal-f32-copy-small-k

Conversation

@davidxifeng

@davidxifeng davidxifeng commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

This follows up on the ggml changes removed from #599, now that ZipVoice has merged. It separates two correctness fixes from the short-K performance optimization into three commits, with no model-specific changes.

  • Strided copy bounds: skip padded rows in kernel_cpy_t_t so short, permuted rows cannot overwrite subsequent channels or memory past the logical output tensor. A regression checks exact output and tail sentinels across 24 shapes, five repetitions each.
  • Explicit F32 precision: use a float-operand tiled kernel for F32 × F32 operations requesting GGML_PREC_F32, with matching shared-memory sizing and offsets. The existing half staging loses small differences and can turn finite inputs around 70000 into nonfinite outputs. Default precision and other input types retain their existing kernels.
  • Short-K dispatch: allow the F32 tiled path for 32 <= K < 64 when M >= 64 and N >= 32, retaining the existing device/layout checks. Tests cover dispatch thresholds, partial tiles, cancellation, large values, and batch broadcasting.

Validation on Apple M3 against upstream a7b58a6:

  • Standalone ggml build succeeds; both new CTest regressions pass.
  • Both tests fail on the unmodified upstream baseline, detecting copy corruption and explicit-F32 precision loss respectively.
  • In the preceding isolated ZipVoice audit, F32 decoder CPU/Metal relative RMSE at 128 frames improved from 4.90e-4 to 6.98e-7. A 2048-frame decoder evaluation improved from 763.58 ms with correctness fixes only to 426.48 ms with the short-K optimization (median of three runs, each averaging five warm evaluations). These are prior model-level measurements, not a new end-to-end RTF benchmark on this rebased branch.

The technical report is included below.

Technical report and reproduction instructions

Metal copy bounds, explicit F32 precision, and short-K matmul

Scope

These changes follow up on the ggml changes removed from ZipVoice PR #599.
They contain two correctness fixes and one performance optimization, in separate
commits. No model implementation, package specification, CUDA kernel, or default
precision policy is changed.

The branch is based on upstream a7b58a6 and preserves the subsequently merged
Metal BF16 rounding work. Validation was performed on an Apple M3 on 2026-09-19.

Strided copy bounds

kernel_cpy_t_t batches short rows into a threadgroup. The last group can include
rows with i01 >= ne01. Without a guard, those threads calculate source addresses
using another channel's strides and write past the logical destination tensor.
The added early return excludes these padded rows.

The regression uses a real permutation from [C,K,R,B] to [K,R,C,B], followed
by ggml_cpy. Both backing allocations contain explicit extra space, and the
output tail is initialized with sentinels. Thus the test detects logical
out-of-bounds writes without relying on an allocation fault. It checks 24 shapes,
five repetitions each, including singleton rows and widths around the batching
threshold. Any incorrect output or changed sentinel fails the executable.

In the original local audit, K=32, R=1, C=5, B=2 produced up to 272 incorrect output
values and overwrote 224 float sentinels. All patched cases were correct.

Explicit F32 matrix multiplication

The existing F32-by-F32 tiled kernel stages operands as half. Float accumulation
cannot recover information lost during that conversion, even when the operation
requests GGML_PREC_F32. The new specialization stages both operands as float;
pipeline selection, shared-memory allocation, and the B-buffer offset are updated
together. Default precision and other input types retain their existing kernels.

The regression compares GPU results with double accumulation over the actual
input floats. It covers ordinary values, values around 70000 (outside the finite
FP16 range), cancellation of small differences, partial tiles, and batch
broadcasting. Explicit F32 cases must have finite outputs and maximum error
normalized by 1 + sum(abs(products)) below 2e-6. Default-precision and F16
cases are also executed as controls. Missing Metal hardware returns CTest skip
code 77 rather than reporting a pass.

Original audit, K=128, M=67, N=35, batch=2:

Input Original F32 specialization
Ordinary values: relative RMSE 2.82134e-5 1.86801e-7
Values around 70000: nonfinite outputs 4690 / 4690 0
Small differences with cancellation: relative RMSE 1 0 in this case

Short-K dispatch

Previously, tiled MM required K >= 64. The added path allows 32 <= K < 64 only
for F32-by-F32 operands with explicit F32 precision, M >= 64 and N >= 32, while
retaining the existing layout and device eligibility checks. The new precision
kernel is a prerequisite: routing these contractions through the old half-staged
kernel would introduce an avoidable precision regression.

Tests include K=31/32/33/48/63/64/65/128 and M/N values immediately below and above
the dispatch thresholds. Timing is printed but is not a pass/fail criterion.
Changing the reduction order means the results are not necessarily bit-identical.

Model-level evidence from the preceding isolated audit

The following measurements predate the rebase onto a7b58a6; they are retained
as model-level evidence, not presented as a new full-model benchmark of this PR.
The same F32 ZipVoice weights and model code were linked against original ggml,
correctness fixes only, and the full patch. CPU served as the reference.

Stage Original Metal relative RMSE Full patch relative RMSE
Text encoder output 3.13564e-4 4.61151e-7
Decoder velocity, 128 frames 4.90226e-4 6.97989e-7
Vocos, 17 mel frames 1.27479e-2 2.09830e-5

For a 2048-frame decoder velocity evaluation, each process warmed up once and
averaged five evaluations; the table gives the median of three process means:

Variant Time
Original 764.17 ms
Correctness fixes only 763.58 ms
Full patch 426.48 ms

The short-K change reduced this workload's time by about 44% (1.79x speedup).
Full patch versus correctness-only output relative RMSE was 8.85730e-7. These are
single decoder evaluations, not end-to-end audio RTF measurements.

Q8 CPU/Metal differences remain: the F32 specialization does not unify quantized
backend arithmetic. For example, Q8 decoder relative RMSE at 128 frames changed
only from 0.00991154 to 0.00989252. No listening-quality claim is made.

Reproduction on this branch

The top-level audio.cpp build disables upstream ggml tests, so configure ggml
standalone:

cmake -S external/ggml -B build/ggml-metal-review \
  -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF \
  -DGGML_METAL=ON -DGGML_BUILD_TESTS=ON -DGGML_BUILD_EXAMPLES=OFF
cmake --build build/ggml-metal-review \
  --target test-metal-copy-bounds test-metal-f32-matmul -j 8
ctest --test-dir build/ggml-metal-review \
  -R 'test-metal-(copy-bounds|f32-matmul)' --output-on-failure

Both tests pass on M3 with the rebased changes. Both also fail on the isolated
unmodified a7b58a6 baseline: copy output/sentinel corruption and excessive
explicit-F32 matmul error, respectively. To repeat this negative control, copy the two test sources and their CMake registrations into an isolated
checkout of a7b58a6, build with the same options, and run the same command.

Limits and review considerations

Only Apple M3 was exercised. Other Apple GPUs, especially the M5 tensor API path,
require additional validation. This is not a claim of regression coverage for all
models or backends. Explicit F32 can cost more than half-staged computation on
some shapes; the short-K speedup should not be generalized to every matrix size.
The copy fix affects other users of the generic strided-copy kernel, which is why
it includes direct output and sentinel regression checks.

@davidxifeng
davidxifeng force-pushed the codex/metal-f32-copy-small-k branch from a17917b to c3ad8d1 Compare September 19, 2026 09:34
@0xShug0 0xShug0 added the ggml label Sep 20, 2026
@0xShug0

0xShug0 commented Sep 20, 2026

Copy link
Copy Markdown
Owner

@davidxifeng Run the test external/ggml/tests/test-metal-f32-matmul.cpp

Shape Base PR Delta
K=48 M=67 N=35 bc=0 0.1703 ms 0.2498 ms +46.7% slower
K=63 M=67 N=35 bc=0 0.1570 ms 0.1910 ms +21.7% slower
K=32 M=67 N=35 bc=1 0.1762 ms 0.2102 ms +19.3% slower
K=48 M=67 N=35 bc=1 0.2783 ms 0.1539 ms -44.7% faster

It's unclear what other models could hit the slow region and regress, and how many models would benefit from the correctness fixes. Could you provide more information? I need to understand the trade-offs before deciding on the next steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants