Skip to content

Metal: in-place ops on the owning backend, ROPE_APPLY kernel, group norm batch fix, no FP8/INT8 mul_mat abort - #30

Merged
danielhanchen merged 5 commits into
masterfrom
mac-metal-fixes
Oct 12, 2026
Merged

danielhanchen merged 5 commits into
masterfrom
mac-metal-fixes

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Problem

Four problems in the Metal path of the bundled ggml, found while checking why the macOS prebuilt cannot run on Metal on GitHub's macOS runners and what it costs on Apple Silicon.

  • In-place ops can run on a backend that does not own their memory. When an op has no Metal kernel, the scheduler runs it on the CPU and its output lands in a CPU buffer. An in-place op that follows it (ggml_mul_inplace, ggml_add_inplace, ggml_silu_inplace, as in every group-norm affine + SiLU of the UNet and VAE) is a view of that output, but it is still assigned to Metal: GGMLRunner::pin_multi_device_nodes pins every node Metal supports, and the scheduler's GPU expansion does the same without a pin. Metal then gets a pointer into a CPU buffer. On the GitHub macOS runner (a VM whose GPU reports no simdgroup support, so GROUP_NORM falls back to the CPU) SD-Turbo on Metal logs ggml_metal_buffer_get_id: error: tensor ' (view)' buffer is nil and segfaults, on this master and on upstream leejet master alike. A diagnostic build shows the culprit: tensor ' (view)' op=MUL ... view_src='node_169' ... buf_name=CPU. CUDA hides the same assignment on hosts where the GPU can read pageable memory.
  • ROPE_APPLY has no Metal kernel. The precomputed RoPE op (CPU, CUDA and Vulkan only) is what Flux, Z-Image, Qwen-Image, Wan, LTXV and the LLM text encoders use through Rope::apply_rope. On Metal ggml_backend_supports_op fails and every q and k of every layer takes the decomposed path (permute, cont, reshape, mul, sub, add, concat).
  • GROUP_NORM runs one simdgroup per group. The kernel can reduce across simdgroups, but the launch is fixed at 32 threads, so a VAE or UNet group norm (32 groups of up to millions of elements) occupies 32 simdgroups of the whole GPU. The cross-simdgroup reduction also reuses its threadgroup buffer without a barrier, which is why the wider launch was left commented out.
  • FP8 and INT8 weights abort on Metal. ggml_metal_supports_mul_mat_op reports support for any weight type when the input is not F16, but there are no FP8 or INT8 mul_mat kernels, so the pipeline lookup fails at encode time. sd.cpp only casts FP8 weights to BF16 when the backend says no.

Change

Four ggml patches under scripts/unsloth/ggml-patches/, numbered 0010 to 0013 so they sort after 0007 (#27) and 0008/0009 (#28). They apply in order on 0001 to 0006 alone and on 0001 to 0009:

  • 0013-ggml-sched-inplace-op-owner-backend.patch: a pass between the scheduler's pass 4 and pass 5. A non-view op whose result is a view of another tensor is moved to the backend that owns that tensor's memory when its assigned backend cannot use that buffer type and the owner supports the op. Pass 5 then copies its other inputs as usual. Nodes whose backend can already use the memory are untouched, so a graph that runs entirely on one backend schedules exactly as before.
  • 0010-ggml-metal-rope-apply.patch: kernel_rope_apply_f32 (one threadgroup per token, head and batch row, threads over the rotation pairs, arbitrary source strides, interleaved and split layouts), its pipeline, encoder and support check.
  • 0012-ggml-metal-group-norm-threads.patch: one threadgroup per (group, batch) so batches after the first are normalized, the threadgroup grows with the group size up to 1024 threads, and a barrier is added before the second reduction reuses the threadgroup buffer.
  • 0011-ggml-metal-no-fp8-int8-mul-mat.patch: Metal reports FP8 (E4M3, E5M2) and INT8 weights as unsupported for MUL_MAT and MUL_MAT_ID, so sd.cpp casts FP8 to BF16 and the scheduler places INT8 matmuls elsewhere instead of aborting.

Results

All Metal numbers are from GitHub macos-15 runners (VirtualMac2,1, Apple M1 virtual GPU, 3 vCPU, 7 GB), staging runs on a fork. base = master afac7a7c, head = this branch, both built exactly like the macOS arm64 release leg (static, -DSD_METAL=ON -DGGML_METAL_EMBED_LIBRARY=ON, deployment target 14.0). Arms are interleaved (order reversed every repeat), one fresh sd-cli process per measurement, seed 42.

The runner GPU reports MTLGPUFamilyApple5 with no simdgroup reduction, simdgroup matrix or bfloat, so ggml-metal sends most ops to the CPU there. A CI-only diagnostic build (not part of this PR) turns those features on with an env var; with it test-backend-ops -b MTL0 passes ADD, MUL, GET_ROWS, CPY, SOFT_MAX, NORM, RMS_NORM, MUL_MAT (1267 cases), FLASH_ATTN_EXT (5004 cases), IM2COL, CONV_2D, UPSCALE, CONCAT, GLU and ROPE with 0 failures, so that mode behaves like a physical M1. Rows marked "forced" use it.

In-place op fix (0013), the runner's own GPU (not forced):

SD-Turbo f16, 512, 2 steps runs ok image s (median) PSNR vs CPU
base, Metal 0/3 (segfault, 6 buffer is nil each) - -
head, Metal 3/3 89.4 50.1 dB
CPU backend (--backend cpu) 3/3 61.8 ref

The same base crash happens with upstream leejet master on that runner. On CUDA (B200, sd-cli built with -DSD_CUDA=ON), base and head produce pixel-identical SD-Turbo images (512, 2 steps, also with --clip-on-cpu --vae-on-cpu), 2 runs each. Z-Image-Turbo on CUDA is not run-to-run deterministic here (base vs base 23.6 dB, head vs head 23.2 dB), and base vs head (30.7 dB) is inside that.

A host-side repro of the assignment, without Metal: a two-backend graph (CUDA + CPU) where y = custom(x) can only run on the CPU and z = ggml_mul_inplace(y, w) has w on the GPU. base puts z on CUDA0 while y is on the CPU (auto and pinned like sd.cpp does), head puts z on the CPU. The numeric result is correct either way on this host only because its GPU can read pageable memory.

ROPE_APPLY kernel (0010), Z-Image-Turbo Q4_K_M, forced:

cell arm sample s (median, spread) image s vs
256 px, 2 steps, 3 pairs base 82.5 (30.7) 101.8 PSNR vs CPU 35.4 dB
head 60.6 (4.8) 75.1 PSNR vs CPU 35.3 dB
512 px, 2 steps, 2 pairs base 95.7 (10.5) 127.9 ref
head 79.7 (1.4) 116.7 PSNR vs base 59.7 dB

head sampled faster than base in all 5 interleaved pairs, and kernel_rope_apply_f32 is compiled only in the head runs. The 7 GB runner cannot keep this checkpoint resident (auto-fit streams the DiT weights), so absolute times are not what a 16 GB Mac sees; Qwen-Image-2.1-Turbo Q2_K at 256 px (2 steps, 2 pairs) moved 21.5 s to 20.1 s, inside its spread.

Group norm (0012), forced:

  • Correctness: GROUP_NORM(ne=[3,5,64,2]) (batch 2) fails on base kernels, ERR 0.97 (only batch 0 was normalized), and passes on head.
  • Kernel time, test-backend-ops perf -o GROUP_NORM, 3 interleaved pairs: [64,64,512] 2.06 to 2.19 ms on base vs 0.65 to 0.76 ms on head; [256,256,256] 17.4 to 17.8 ms vs 6.6 to 6.7 ms.
  • End to end, SD VAE decode does not move (SD-Turbo 512: 13.1 s base, 13.0 s head): decode time is dominated by the convolutions.

FP8 / INT8 (0011): base Metal reports MUL_MAT(type_a=f8_e4m3, type_b=f32) as supported, although no FP8 or INT8 kernel_mul_mv_* / kernel_mul_mm_* exists, so the encoder would hit ggml_metal_encoder_set_pipeline: nil Metal pipeline. head reports it unsupported.

Accuracy (LPIPS AlexNet, one prompt and seed per cell, same-weights reference, not a bf16 checkpoint): SD-Turbo Metal vs CPU 0.0010 base / 0.0010 head; Z-Image 256 0.0065 / 0.0066; Qwen-Image-2.1 256 0.0057 / 0.0056; SD1.5 cfg 7 0.0301 / 0.0299; Z-Image 512 head vs base Metal 0.0001. None of the four patches is a lossy lever: head sits at base's distance from the CPU reference in every cell.

Metal vs CPU on the same runner, head, forced:

model CPU s/image Metal s/image sampling CPU / Metal PSNR Metal vs CPU
SD-Turbo f16, 512, 2 steps 61.8 20.6 22.6 / 4.2 s 47.5 dB
SD1.5 Q8_0, 512, 4 steps, cfg 7 140.6 64.2 104.6 / 52.0 s 31.5 dB
Z-Image-Turbo Q4_K_M, 256, 2 steps 230.8 75.1 206.8 / 60.6 s 35.3 dB
Qwen-Image-2.1-Turbo Q2_K, 256, 2 steps 102.7 45.2 68.8 / 20.1 s 39.3 dB

The CPU has only 3 vCPU here, so the ratios overstate what an 8 to 12 core Mac sees. With --diffusion-fa, SD1.5 sampling on Metal drops from 53.4 s to 16.9 s; --diffusion-conv-direct makes the same UNet trip the GPU watchdog (kIOGPUCommandBufferCallbackErrorTimeout, 3/3) and --vae-conv-direct makes the decode 5.5x slower, so callers should not pass direct conv on Metal.

Tests

No test code is part of this PR; the checks below were run locally with test-backend-ops cases kept out of the patches.

  • Metal (test-backend-ops -b MTL0, on the runner, forced): ROPE_APPLY cases (interleaved and split layouts, contiguous and permuted sources) 8/8 pass on head and are not supported on base; a batch-2 GROUP_NORM case fails on base and passes on head.
  • Scheduler (CUDA + CPU, ggml built with 0001 to 0006 and 0010 to 0012 as base, plus 0013 as head, B200): the standalone graph described above. base puts the in-place z on CUDA0 while its memory y is in a CPU buffer, in both auto and pinned mode; head runs z on the CPU in both modes. The numeric result is identical on both arms, since this GPU can read pageable memory.
  • CUDA regression (same two builds, test-backend-ops test -b CUDA0 vs CPU): GROUP_NORM, ROPE, ROPE_APPLY, MUL and ADD pass with no failures on head and base. The Metal kernels cannot run on this host.
  • All four patches pass git apply --check in order on 0001 to 0006 (this master) and on 0001 to 0009 (with the patches from ggml-cuda: generic DiT and VAE fusions for every family, plus a rope PDL race fix #27 and CPU bundles: runtime-selected ggml CPU variants and repacked quantized weights (1.3x to 3x faster on CPU) #28).

Not covered

  • No physical Apple Silicon. Every Metal number is from GitHub's M1 virtual machines; the "forced" rows enable features the VM GPU executes correctly but does not advertise. A real M1 Max or M4 may weigh ROPE_APPLY and group norm differently.
  • The kernel-time gain of the wider group norm does not show up end to end on SD VAEs here; it is shipped for the batch fix and the race fix.
  • FP8 and INT8 checkpoints were not run end to end on Metal; the FP8 claim rests on the support check and the missing kernels.
  • Vulkan, ROCm and Windows were not run. 0010 to 0012 only touch ggml-metal; 0013 touches the shared scheduler and was checked on CUDA and CPU only (pixel-identical SD-Turbo, the standalone graph, test-backend-ops).
  • macos-15-intel was not run (that build has Metal off).

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T04:50:50.757684Z 55575cf Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Oct 12, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

1 similar comment
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 55575cfeda

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen
danielhanchen merged commit 2a4ff8f into master Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant