Skip to content

Fix Metal fence ordering and sealed collective portability - #41

Merged
davidtai merged 16 commits into
mainfrom
fix/cluster-metal-fence-wait
Oct 28, 2026
Merged

davidtai merged 16 commits into
mainfrom
fix/cluster-metal-fence-wait

Conversation

@jonathan308

@jonathan308 jonathan308 commented Oct 23, 2026 •

Copy link
Copy Markdown

An ordinary fast Metal fence wait now follows earlier GPU kernels in the same concurrent pass. Its existing waits for previous command passes remain. This closes a missing dependency before the spinning wait without changing arithmetic, collective operations, or command-buffer limits.

This draft builds on core #35, branch darkbloom/sealed-all-sum, head 360d11b90dcf26fe1b0c9be1789d5cb9d2c49f80. Consumers are coordinated separately through mlx-swift #61, mlx-swift-lm #352, and d-inference #1577. No C API or shader changes are required.

Before

flowchart TD
  A["Earlier GPU producer in the current pass"] --> B["CPU stream work"]
  B --> C["GPU spinning wait"]
  D["Previous-pass dependencies only"] --> C
  C -. "No explicit current-pass barrier" .-> A
  classDef default fill:#e8f0fe,stroke:#174ea6,color:#102a43,stroke-width:2px;
  classDef warning fill:#fff4ce,stroke:#8a6100,color:#332500,stroke-width:2px;
  class C,D warning;
Loading

After

flowchart TD
  A["Earlier GPU producer"] --> B["Current-pass buffer barrier"]
  D["Previous-pass dependencies"] --> C["GPU spinning wait"]
  B --> C
  A --> E["CPU stream work completes"]
  E --> C
  C --> F["GPU consumer may proceed"]
  classDef default fill:#e8f0fe,stroke:#174ea6,color:#102a43,stroke-width:2px;
  classDef guard fill:#fff4ce,stroke:#8a6100,color:#332500,stroke-width:2px;
  classDef success fill:#d7f0df,stroke:#176b39,color:#103b23,stroke-width:2px;
  class B,D guard;
  class F success;
Loading

The standalone metal_fence_chain test builds 96 GPU→CPU→GPU layers with shared side roots, widths 1/2/3/4/8 and three repetitions. It checks every result and zero remaining request-array bytes after drain. CTest registers three separate processes with command-buffer limits 8/50/1024 and a 60-second timeout. The Metal Fork Tests job runs these registrations explicitly and rejects missing tests.

Validation and limits

  • Old and candidate CPU executions each passed 15 cases and 9,123,840 exact comparisons.
  • On M5 Max, all six old/candidate Metal processes passed: 15 cases per process at each of the three buffer limits. The old binary also passes; this synthetic test did not reproduce the installed Qwen27 stall.
  • Both versions used the same 193 copied c11 Cmlx objects, identical regression object and matching metallib; only identically compiled fence.o differed.
  • Five actual-child runner tests pass, including timeout, SIGHUP and forced termination with definitive reaping. Independent source/refactor review found no outstanding issue.
  • Shared-code merge gate: mlx/backend/metal/fence.cpp also serves GLM. The GLM owner must verify its paired execution before merge; this draft does not establish GLM parity.
  • The matching c12 signed app is installed on both Macs. Its normal Darkbloom setup automatically selects TP and completes singleton requests, but two automatic batching attempts hit LOAD memory admission refusals before wide execution. The prior native width3 stall has therefore not been retested. No causal fix for that stall or full-model/performance pass is established here.

Reproduction from a configured Metal build:

cmake --build build --target metal_fence_chain
ctest --test-dir build -R '^metal_fence_chain_' --output-on-failure
./build/tests/metal_fence_chain --cpu
uvx --from clang-format==21.1.8 clang-format --dry-run --Werror \
  mlx/backend/metal/fence.cpp tests/metal_fence_chain.cpp
uvx --from cmakelang==0.6.13 cmake-format --check tests/CMakeLists.txt
python3 scripts/check_forkdiff.py

Evidence: reviewed implementation patch e1c98427cedc685fd6d5b445623e9f4fe48a167aef497ecb10579c508770a7bf; compiled probe inventory 76b3c03f2a2773187a388871bb0a018711263910dc0a06e29eada89a22cc5705; M5 six-process summary ca7f846f14454c7948aa74c233e0dca79dc9b4a2ffc59e5b3cda24c187785747. Publication adds fork metadata, a narrow CTest CI invocation and formatter-only changes. Follow-up66edc771 formats six inherited paths; source bodies are unchanged, with one existing system-include reorder required by clang-format. Lint and fork coverage now pass. Four Linux jobs exposed an inherited unconditional Apple dispatch/QoS dependency at compile time. Commit9ec023f9 guards that platform code and adds a synchronous portable path; hosted Linux and Metal checks for this head remain pending.

Sealed collective portability

Apple dispatch/QoS/Blocks statements retain their existing order and behavior. Other platforms seal the same authenticated pieces synchronously before exchange, then open only complete received frames and apply the same rank-ordered addition. This introduces no worker pool, plaintext transport or arithmetic change.

flowchart LR
  subgraph Before
    L1["Linux sealed collective build"] --> L2["Unconditional Apple headers"] --> L3["Compilation fails"]
  end
  subgraph After
    N1["Platform selection"] --> A1["Apple: existing parallel callbacks"]
    N1 --> P1["Other platforms: synchronous callbacks"]
    A1 --> F1["Same authenticated frames and reduction"]
    P1 --> F1
  end
  classDef input fill:#dbeafe,stroke:#2563eb,color:#172554
  classDef fail fill:#fee2e2,stroke:#dc2626,color:#450a0a
  classDef fixed fill:#dcfce7,stroke:#16a34a,color:#052e16
  class L1,N1 input
  class L2,L3 fail
  class A1,P1,F1 fixed
Loading

Three CPU cases cover callback identity, complete-piece boundaries, exact output, failed-seal zeroing, rejected output and callback drainage after exchange failure. The deterministic fixture codec is not a cryptography test. On macOS, C++20/Werror execution passes 5,251 Apple-branch and 5,263 portable-selected assertions; the latter compiles without Blocks and has no Apple dispatch/QoS/Blocks symbols. Actual Linux headers/ABI and sanitizer execution remain hosted-CI gates. Fork coverage236/236, pinned formatting and independent source/refactor review pass. Patch SHA440b275ec0ea12903da1ca665b4b10135b0777e7e68518f07f659b83739e1e01. Downstream candidate c13 still uses the earlier reviewed4e2455b0 core; portability/default pin updates are separate from its Qwen test.

AI disclosure: implementation assistance, focused validation and independent agent review used OpenAI Codex. This is a maintainer-review draft prepared at the user's explicit request.

🤖 Generated with Claude Code

Current integrated hardware gate — 2026-10-23

Normal signed c16 TP2 completed through the installed Darkbloom command/services/API on M3 Ultra256GB + M5 Max128GB (recursive source SHA d6f6adcedca1aeacfc5265ec427e3ef913633f5b77629b8c61cbdaf4ba278000). Stable endpoint37PASS/0FAIL/1intentional skip, context6/6 through32K, media/structured-output/mixed30/30 and cancellation/survivor/recovery3/3 pass. All61 matching native scopes retire with zero in-use KV;13 reach actual width2 on both ranks. Initial before/after-plan exact outputs remain29PASS/8FAIL/1SKIP. Aggregate throughput at offered concurrency1/2/4/8 is13.26/13.56/13.52/13.43tokens/s at native capacity2. Cleanup and restored idle services pass, but final workers exit1 after input closes; graceful shutdown remains a separate open gate. No full model or beta release is qualified.

Provider follow-up #1577 at signed/Verified b845e27771cb7c355ce1baa35731f343a16fec2c retains typed eager-fence diagnostic forwarding and completed-decode width counters. c17 is installed on both Macs. Its bounded FAST1+EAGER1 diagnostic completed FAIL:11PASS/7FAIL/6intentional skips. Both settings are verified in both fresh native pairs; a real width-two scope then stalls with an M3 JACCL timeout at60001ms and M5 lost input. Original service-job bytes and healthy idle services are restored on both hosts. Eager commits alone do not fix the fast-path stall. The same c17 build is now running short default-policy1/2/4 checks to measure actual batch utilization. Defaults remain the c16 event-synchronization policy for batched Qwen27.

Installed TP MTP remains disabled. The prepared foundation compiles and passes selected CPU checks, but only speculates singleton; true multirow target verification, whole-cohort memory accounting and full-artifact acceptance remain unfinished. The normal c14 fast-path stall and c15 exact-output mismatch remain preserved. No tolerance, memory guard or capability gate is relaxed.

Core#41 at9ec passes7 hosted checks; inherited recoverable UBSAN diagnostics remain recorded. Installed/downstream core still uses4e's Apple fence change;9ec portability changes are not propagated. Swift#61 has3 passing checks. SDK#352 at4f still fails:3613tests/622suites/847issue records plus one separate unit issue, including842 NemotronSuper assertions and two Prism error-contract regressions; four GLM issue records match foundation limitations. Serial CI uses event synchronization with matching reported runner image/OS/toolchain. A common cause with the cluster fast-path stall is not established. Final reviews, exact pins, SDK isolation and full-model checks remain gates.

Progress 2026-10-26: main merged, stack re-pinned, validated on two M5 Ultra 256 GB

Head caa9ed53: fast-forward of d11238f1 plus #35/main merges, the head-dim-256 NAX attention JIT fix (ce21a37d) and MLX_QMV_BATCH_INVARIANT (caa9ed53).

Nothing merged or released; the owners sign and release.

Update 2026-10-26 17:15 PDT: main/master 0.10.3 merged through the stack

Head ea318ee0: the refreshed #35 merged; this branch fixed the same JIT bug as main (ce21a37d), and get_steel_attention_nax_kernel now reads exactly as main's. Every PR in the cluster stack (mlx #33 → #35 → #41, mlx-c #15, mlx-swift ml-explore#54 → ml-explore#56 → ml-explore#61, SDK ml-explore#308 → ml-explore#352, d-inference ml-explore#1407 → ml-explore#1426 / ml-explore#1577, platform ml-explore#54) is again mergeable in that order, as drafts.

Review fixes and CI follow-up (2026-10-27)

Head e637499f: review fix b2d92e5b (eval_impl updates the fast fence before the event fence, MLX_EAGER_FENCE_COMMIT commits after both; metal_fence_chain covers two GPU streams that feed the CPU stream) and #35's fixes merged in.

CI follow-up: build-cpp builds without MLX_METAL_JIT, so kernels_jit_compiled() is false and quantized_tile_tests compares the 32 x 32 kernel with itself. The 64-row, 64-column and BK=64 tiles are not exercised in CI. Covering them needs a JIT C++ lane.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 and others added 2 commits October 23, 2026 12:06
Apply the existing clang-format and cmake-format policies to six inherited
paths. Preserve all test coverage and source bodies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Guard Apple dispatch, QoS and Blocks code. Preserve the Apple path and use synchronous authenticated frame callbacks elsewhere. Add CPU coverage for framing, failures and callback ownership.

Validated both selected branches on macOS with C++20 and warnings as errors; actual hosted Linux execution remains pending.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jonathan308 jonathan308 changed the title Order fast Metal fence waits after current-pass producers Fix Metal fence ordering and sealed collective portability Oct 23, 2026
With fast synchronization (MLX_METAL_FAST_SYNCH=1) a GPU fence wait is a
kernel that spins until its producer's timestamp arrives. That is safe for a
CPU producer, but Metal does not promise that a second command queue runs
while a pass of the first one spins. A widened Qwen tensor-parallel decode on
mlx-swift put one graph on two GPU streams: quantized_matmul cast the batched
projections' bfloat16 scales and biases on the thread's default stream rather
than the caller's, while mlx-swift runs everything else on its own
process-wide stream. The two ranks then stopped meeting in the next
collective (the cluster's c14, c17 and c39 stalls).

- Helper ops keep the caller's stream: quantized_matmul's casts, gather and
  scatter index broadcasts, the gather matmuls' index casts and
  affine_quantize's scale select (ops.cpp).
- A GPU stream never spins on another GPU stream: evaluation waits for it on
  a shared event, and so does a GPU stream waiting for CPU work that can
  itself wait for another GPU stream; an array read both by the CPU and by
  another GPU stream updates both fences. Other CPU <-> GPU waits keep the
  fast spin (transforms.cpp, Fence(Stream, fast) in fence.h, and the Metal,
  CUDA and no-GPU fences).
- metal_fence_chain adds an exact two-GPU-stream chain at widths 1, 2 and 4;
  ops_tests checks that quantized_matmul's casts run on the caller's stream.
  using_streams.rst and fork.yaml describe both rules.

Synchronization only: results do not change. On one Mac (two ranks over a
local ring) the stall reproduced without the fix and not with either part
(seeds 7, 11 and 13, two and four lanes, a 600-step soak), with tokens
byte-identical to the event path. On the two-Mac pair (M3 Ultra + M5 Max,
Thunderbolt RDMA) every widened Qwen tensor-parallel owner runs on fast
fences since the provider build that pins this core: the fast-fence
diagnostic qwen27-c40-fast1-02 (two lanes) passed, and the installed
acceptance qwen27-c44-auto-mtp-01 widened to four lanes with rows retiring
mid-cohort, every concurrent row byte-equal to its solo run, and no stall.

Tests: the new C++ cases (metal_fence_chain's two-GPU chain, ops_tests'
caller-stream check) were not built or run here; the fork's Fork Tests
workflow builds and runs them. The Swift cluster runtime suites that run the
fixed paths passed on the M5 Max (GPU batch 14, GPU MTP 30).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 25, 2026
Source/Cmlx/mlx moves to a5d6547b (Layr-Labs/mlx#41): helper ops such as
quantized_matmul's casts keep the caller's stream, and a GPU stream waits for
another GPU stream on a shared event instead of a spinning fast fence. A
widened Qwen tensor-parallel decode had put one graph on two GPU streams and
stalled.

- QuantizedLinear.independentRowsForward runs affine float32 rows through
  MLX's single-row (M=1) kernel, each as its own call would run it, with the
  FP32 widening of the BF16 scales and biases that callAsFunction caches (one
  copy for both, made on the caller's stream) instead of a fresh widening
  inside every batched quantized matmul.
- fork.yaml describes both.

Tests: the cluster runtime suites that run independentRowsForward passed on
the M5 Max with this pin (QwenTensorParallel CPU suites, GPU batch 14, GPU
MTP 30, in the provider's c44 candidate chain); on the two-Mac pair the
installed c44 acceptance kept every concurrent row byte-equal to solo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a GPU without neural accelerators the steel `affine_qmm_t` kernel computed
every product in 32 x 32 output tiles. A product of at least 256 rows with a
64-aligned output width now takes a 64-row x 32-column tile for 16-bit inputs
and a 32 x 64 tile for FP32 inputs by default, JIT-instantiated from the same
`qmm_t_impl`. Every output element still accumulates the same K slabs in the
same order, so a tile decides which threadgroup computes an element, never its
bits. NAX-eligible products and decode-sized products (fewer than 256 rows) are
unchanged.

- `MLX_QMM_STEEL_TILE` (32, 64, 6432, 3264, 6464) and
  `MLX_QMM_STEEL_TILE_MIN_GROUPS` choose or restore the tile;
  `MLX_QMM_STEEL_TILE=32` is the previous kernel path exactly.
- `mlx_metal_set_thread_qmm_steel_tile` overrides the tile for the products the
  calling thread encodes (tests and benches).
- environment_variables.rst and fork.yaml describe both.

Measured on the M3 Ultra (the TP rank without NAX): a 27B 4K prompt 12.37 ->
12.06 s, a 9B 4K prompt 3.55 -> 3.46 s, same tokens. On the two-Mac pair (Qwen
3.8 27B, tensor parallel 1:1, with prompt frames submitted every four layers)
time to first token at 1K/4K/16K/32K fell 1.4/1.6/3.1/2.2%, tokens identical.

Tests: the cluster runtime's steel-tile tests (bytes for every tile and the
default, BF16 and FP32) and its two-rank prompt-submission GPU test passed on
the M5 Max, and the provider's c45 candidate chain passed. The core's own C++
tests were not built or run here; Fork Tests runs them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 26, 2026
Source/Cmlx/mlx moves to 2e16455f (Layr-Labs/mlx#41): on a GPU without neural
accelerators, prompt-sized affine quantized matmuls take 64 x 32 (16-bit) or
32 x 64 (FP32) output tiles instead of 32 x 32, with every element's bits
unchanged.

- `GPU.withQuantizedMatmulSteelTile(_:_:)` runs a body with the steel tile the
  calling thread encodes (the C entry `mlx_metal_set_thread_qmm_steel_tile` is
  declared in the vendored `command_buffer_limits.h`), for tests and benches.
- fork.yaml describes both.

Tests: the cluster runtime's steel-tile byte tests and two-rank prompt-
submission GPU test (M5 Max), and the provider's c45 candidate chain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prompt-sized steel tiles (2e16455) exist only as run-time instantiations.
CMake's default build (MLX_METAL_JIT=OFF: the Python wheels and Fork Tests)
loads kernels from the precompiled library, which holds `affine_qmm_t` at
32 x 32 only, so a prompt-sized product asked it for a kernel it does not
have: "Unable to load kernel
affine_qmm_t_float16_t_gs_64_b_4_alN_true_batch_0_bm64_bk32_bn32" in
test_quantized.test_qmm_large_dims, and the FP32 32 x 64 kernel in
test_export_import.

- `kernels_jit_compiled()` (kernels.h; true in jit_kernels.cpp, false in
  nojit_kernels.cpp) tells the two builds apart, and `qmm` takes a tile other
  than 32 x 32 only when it is true. Precompiled builds run exactly the
  previous kernels; JIT builds (mlx-swift) are unchanged.
- tests/quantized_tile_tests.cpp (Metal only): 512 x 1024 products (one qmm,
  no split-K) at K = 96, which no neural-accelerator kernel takes, and at
  K = 192 in groups of 64, for float16, bfloat16 and float32. The 32 x 32
  result matches the dequantized product, and every tile (the default, 64,
  6432, 3264, 6464) gives its bytes.
- environment_variables.rst, the header comments and fork.yaml say where the
  tiles apply.

Tests on the M3 Ultra (no neural accelerators, like the GitHub runner), the
core's C++ tests built with CMake: the new case fails on 2e16455 with
precompiled kernels (30 of 36 checks throw "Unable to load kernel
affine_qmm_t_..."), and passes 36/36 here with precompiled kernels and with
the JIT, where every wide tile, the 64-deep one included, gives the 32 x 32
bytes. The whole doctest suite passes in both builds (294 cases, 4,093
assertions). mlx's pre-commit hooks (clang-format 21.1.8, cmake-format 0.6.13,
check-yaml) and the fork.yaml check (237/237) pass. The Python suite was not
run here; Fork Tests runs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 26, 2026
Source/Cmlx/mlx moves to d11238f1 (Layr-Labs/mlx#41): MLX builds that load
kernels only from the precompiled library keep 32 x 32 steel tiles for affine
quantized matmuls rather than asking for wider tiles the library never
instantiated. mlx-swift compiles the core's kernels at run time
(jit_kernels.cpp), so its kernels and results are unchanged.

Tests: the core's C++ tests on the M3 Ultra, precompiled and JIT builds (see
the core commit), and mlx-swift's generated-Metal check with the new core
nested. mlx-swift itself was not rebuilt here; build-and-test runs it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift-lm that referenced this pull request Oct 26, 2026
Package.swift pins mlx-swift 3832c0de, whose core (Layr-Labs/mlx#41) keeps
32 x 32 steel tiles in MLX builds that load kernels only from the precompiled
library. The SDK reaches the core through mlx-swift, which compiles kernels at
run time, so no kernel or result changes here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 and others added 3 commits October 26, 2026 00:52
…e fence-ordering branch

Brings in merge/35 at 2b5253a: main f78f0fa (the upstream ml-explore/mlx
sync to afc03c5 and the fork fixes #40, #43-#46) through the JACCL and
cluster-collectives branches, with their conflict resolutions. This
branch's tip is d11238f (32 x 32 steel tiles where kernels are
precompiled).

Conflicts and how they were resolved:
- mlx/transforms.cpp: this branch's event fences (a GPU stream never spins
  on another GPU stream, nor on CPU work that can wait for one) with main's
  value-based waits (upstream ml-explore#4552). FenceNeed records the value of each
  of its two updates (fast_value, event_value); every wait uses the value of
  the update it waits for, fast and event fences alike, and a fenced input
  gets the fast value. Waits look the fence up with at(), as main does.
- mlx/backend/{metal,cuda,no_gpu}/fence.cpp: Fence(Stream, bool fast) next to
  the value-based wait. The Metal fence-wait barrier was already in the
  merged collectives branch (the same line), so it merged cleanly.
- mlx/backend/metal/quantized.cpp (qmm): this branch's affine steel tiles
  (MLX_QMM_STEEL_TILE, 64 x 32 / 32 x 64 for prompts) beside main's FP4
  half-loader 64 x 64 tile (upstream ml-explore#4709). The modes do not overlap:
  wide affine tiles keep the "_bm<m>_bk<k>_bn<n>" name, the FP4 tile keeps
  main's precompiled "_bm_64_bn_64" name. The wide-tile special case is gone:
  main's affine_qmm_t has has_global_scale before BM, BK, BN, so every
  transposed qmm now passes (global scale, bm, bk, bn) in main's order. The
  old call would have instantiated affine_qmm_t<..., bm, bk, bn> into the
  wrong template parameters. d11238f's kernels_jit_compiled() gate is kept.
- mlx/ops.cpp: main's 1-bit affine_quantize branch, with this branch's
  caller stream on the scales select.
- mlx/distributed/sealed.cpp: this branch's __APPLE__ guard around the
  dispatch include.
- tests/CMakeLists.txt: metal_fence_chain beside main's GPU_TEST_SOURCES
  (renamed from METAL_TEST_SOURCES upstream), which now also lists
  d11238f's quantized_tile_tests.cpp.
- .github/workflows/fork-tests.yml: main's restructured workflow; the
  bounded Metal fence chain step now runs in the build-cpp job after the
  standalone allocation cap tests (main has no single build-and-test job
  any more).

fork.yaml merged without conflicts (this branch's sections inside the
merged file). The steel-tile section now also lists kernels.h,
jit_kernels.cpp and nojit_kernels.cpp: main dropped the "Upstream v0.32.2
sync" section that described them on this branch, and d11238f's
kernels_jit_compiled() lives there.

Tests (Ultra A, M5 Ultra; Debug build with Metal, 8 jobs):
- tests/tests on the GPU: 315 of 321 cases passed. The 6 failures (linalg
  QR, inverse, Cholesky, pseudo-inverse, eigh, LU) fail the same way on
  main f78f0fa on this machine: TF32 is on by default on a NAX GPU. With
  MLX_ENABLE_TF32=0: 321 of 321 (with quantized_tile_tests).
- tests/tests with DEVICE=cpu: 321 of 321 passed (with the sealed stream
  tests).
- ctest process_allocation_cap: 15 of 15; metal_fence_chain (fast synch,
  8/50/1024 ops a buffer): 3 of 3.
- check_forkdiff.py --upstream-ref upstream/main: 144 changed files, all
  described. clang-format 21: no changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's upstream sync brought the head-dim split NAX attention kernel
(attention_nax_dsplit, used for head dims 256 and 512 on NAX GPUs). Its
template has no BV parameter (the value width is BD) and its precompiled
instances leave BV out, but get_steel_attention_nax_kernel in
jit_kernels.cpp built the run-time source with attention_nax's arguments,
BV included. Every argument after BD then moved one parameter to the left
(WM got BV, WN got WM, MaskType got WN), and the source did not compile:

  steel_attention_nax.h:950:122: error: reference to overloaded function
  could not be resolved; did you mean to call it?
  ... attention_nax_dsplit<bfloat16_t, 64, 32, 256, 256, 4, 2, bfloat16_t>

mlx-swift builds jit_kernels.cpp, so with the merged core every head-dim
256 or 512 prefill that takes the NAX full-attention kernel on an M5
threw "Unable to build metal library from source". CMake and wheel builds
use the precompiled library and were not affected, which is why Fork
Tests did not see it. Upstream ml-explore/mlx main still has the bug
(checked at 7ca4003).

The split kernel's template now leaves BV out; attention_nax keeps its
arguments. Kernel names do not change. fork.yaml describes the fix.

Tests (Ultra A, M5 Ultra, Release build with MLX_METAL_JIT=ON): a probe of
SDPA prefill, causal, no mask and bool mask, for head dims 64, 128, 256 and
512, bfloat16, float16 and float32, 1024 and 2048 rows: before, all 12
head-dim 256 cases with a causal or bool mask threw the error above (the
unmasked and head-dim 512 cases did not take the split kernel); after, all
run, and the causal head-dim 256 outputs are byte-equal to the original
fix/cluster-metal-fence-wait tip 2e16455. The bool-mask head-dim 256
outputs differ from 2e16455 because upstream ml-explore#4416 now runs them in the
fused NAX kernel instead of the unfused fallback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main's upstream sync added two routes that take some row counts of a
quantized x @ w.T below the vector limit (get_qmv_batch_limit) away from
the vector kernels:

- qmv_mma / qmv_nax (ml-explore#4712): 6 to 16 rows of the
  floating-point modes (NVFP4, MXFP4, MXFP8) when K % 128 == 0 and N and K
  are large (N, K >= 2048 on GPUs with neural accelerators);
- the staged NAX split-K kernel (qmm_splitk_nax, ml-explore#4682): affine products
  from 8 rows to the limit with M * N >= 16384, on GPUs with neural
  accelerators.

A row then has other bits at those counts than at 2 to 5 rows. Requests
that share one launch (the speculative verify windows of several requests,
GLM's row groups) then do not get the bits that each request gets alone.

MLX_QMV_BATCH_INVARIANT=1 (env::qmv_batch_invariant, read once in each
process) skips both routes for rows below the vector limit, so they run
dispatch_qmv as before the sync: qmv for one row, qmv_wide from two rows
(qmv where qmv_wide does not serve the mode). Each row then has the same
bits at each count from 2 to the limit - 1. From the limit on, and without
the switch, the routes do not change. A row alone (qmv) can have other
bits than the same row among two or more rows (qmv_wide), as before.

- tests/qmv_batch_invariant_tests.cpp (Metal): GLM/Qwen TP shard shapes
  (N x K = 4096 x 4096, 6144 x 4096, 4096 x 6144, 8704 x 5120,
  5120 x 8704, 3072 x 5120, 2048 x 4096, 1024 x 4096), affine 4-bit g64
  and NVFP4, bfloat16 and float32 inputs. Each window of 2 to `count`
  rows, at the start and at the end, has the bytes of the same rows in the
  `count`-row product, and each row alone has the bytes of a batch of
  single rows (x [R, 1, K] with broadcast weights: the decode-row path).
  `count` is the vector limit - 1 with the switch and 5 without it.
- tests/CMakeLists.txt: the qmv_batch_invariant ctest entry runs the case
  in a new process with the switch on, and fails if no case has the name.
  Fork Tests runs that entry.
- environment_variables.rst and fork.yaml describe the switch.

Tests (Ultra A, M5 Ultra). Debug build with precompiled kernels, as the CI
build-cpp job: the doctest suite with the switch off and on (322 cases;
10,608 and 11,400 assertions) passes except the 6 linear-algebra cases
that already fail with TF32 on neural accelerators; it passes 322/322 with
MLX_ENABLE_TF32=0 (switch off and on) and on the CPU. The
qmv_batch_invariant entry (1,209 assertions), the allocation-cap entries
(15) and the fence-chain entries (3) pass. Release builds with
MLX_METAL_JIT=ON (mlx-swift's mode), the 32 shape, mode and input-type
cases above at 1 to 31 rows: with the switch, every product below the
vector limit is byte-equal to the pre-sync #41 head d11238f, and verify
windows of 2 to 6 rows that share a product below the limit are
byte-equal to each window alone; without the switch, every product is
byte-equal to ce21a37. clang-format 21, cmake-format 0.6.13 and the
fork.yaml check (145/145) pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 26, 2026
The PR branch moved on GitHub after this refresh started: 3832c0d pins
Source/Cmlx/mlx to d11238f1 (Layr-Labs/mlx#41), the core that keeps
precompiled MLX builds on 32 x 32 steel tiles. Merging it makes this branch
a fast-forward of the published head. The merge is clean, and the gitlink
moves 2e16455f -> d11238f1. The forward merge of #56 and the restitch to the
merged #41 core (which contains d11238f1) follow.

Tests: run on the restitch commit that follows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 26, 2026
Restitches the nested pins after the merge of main 6959486:
- Source/Cmlx/mlx: d11238f1 -> ce21a37d. That is the merge of the refreshed
  mlx#35 into Layr-Labs/mlx#41, a fast-forward of the published d11238f1,
  plus the fix of the head-dim split NAX attention's run-time template.
  Without that fix, the JIT build that mlx-swift uses throws "Unable to
  build metal library from source" for head-dim 256 and 512 causal or masked
  prefill on NAX GPUs.
- Source/Cmlx/mlx-c: 73720efb (taken from #56), the merge of mlx-c main into
  mlx-c#15.

Regenerated as tools/update-mlx.sh does from those pins:
- The mirrored Source/Cmlx/include headers, the embedded JIT and CPU
  sources and the Xcode project match the regeneration.
- The include-framework copies of the core's metal/device.h and utils.h gain
  #41's steel-tile declarations: thread_qmm_steel_tile, qmm_steel_tile and
  its minimum-groups setting.
- Metal.hpp differs only in its "Autogenerated" line and is kept as it is.

fork.yaml's pin paragraph names the merged core and C.

Tests (Ultra A, Xcode 27.1, at these pins):
- tools/update-mlx.sh regeneration from the two pins matches the tree
  (Metal.hpp aside, as above).
- check-generated-metal-sources.py: pass.
- swift build --build-tests: complete.
- Fork Tests, all 3 shards of the CI plan, with an AOT mlx.metallib from
  this core:
  - XCTest: 853 listed, 849 passed (including HadamardLayerTests and
    Float16ConstantCastTests), and the 2 permitted skips.
  - The fresh-process allocation-cap test and both multi-process ring
    suites passed.
  - Swift Testing: 749 of 749 passed.
  - test-shards.py verify: every test ran exactly once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 26, 2026
Source/Cmlx/mlx: ce21a37d -> caa9ed53 (Layr-Labs/mlx#41 merged with main + the opt-in switch that keeps quantized products of 2 to the vector limit - 1 rows on qmv_wide, so a request's rows get the same bits whatever shares their launch; off by default, main's routes unchanged). tools/update-mlx.sh regenerated from caa9ed53 + mlx-c 73720efb changes only include-framework/mlx-utils.h (the new env::qmv_batch_invariant(); Metal.hpp aside, as before: the regeneration used the vendored metal-cpp). Source/Cmlx/mlx-c unchanged (73720efb). No kernel source changed: the mlx-generated sources are identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jonathan308 and others added 2 commits October 26, 2026 04:21
#35 now carries 9ec023f (cherry-picked as a135f49) so its Linux jobs build; #41 already had it, so its tree does not change (fork.yaml: #41's).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ce-ordering branch

Main's head-dim split NAX attention JIT fix (#48) and this branch's ce21a37 fix the same bug; get_steel_attention_nax_kernel now reads exactly as main's (main's comment and argument list), so the branch no longer differs from main there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jonathan308
jonathan308 marked this pull request as ready for review October 27, 2026 18:49
@jonathan308
jonathan308 requested a review from davidtai October 27, 2026 18:50
jonathan308 and others added 2 commits October 27, 2026 13:24
This branch already describes the fix as "Run-time template of the head-dim split NAX attention"; that section is kept and the parent's duplicate dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…at feed the CPU

- transforms.cpp: an array read by the CPU and by another GPU stream updates
  the fast fence first and the event fence second. The fast update with
  cross_device can move the array (CUDA queues a copy to unified memory and
  swaps the pointer), so an event recorded before it let the other GPU
  stream read the new buffer before the copy finished. MLX_EAGER_FENCE_COMMIT
  now commits after both updates.
- metal_fence_chain: a case where both GPU streams feed the CPU stream, so
  the producer waits for the CPU result on an event of the CPU stream and the
  CPU stream waits for both GPU streams. Before, only GPU-to-GPU event waits
  ran.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@davidtai

Copy link
Copy Markdown

Review fixes in b2d92e5:

  • eval_impl updates the fast fence before the event fence. On CUDA the cross-device fast update moves the array to unified memory (an async copy and a pointer swap). With the event recorded first, another GPU stream could read the new buffer before the copy finished. MLX_EAGER_FENCE_COMMIT now commits after both updates.
  • metal_fence_chain: new case where both GPU streams feed the CPU stream. This covers the event wait on CPU work and the event fence on a CPU stream.

Not changed (CI design question): build-cpp builds without MLX_METAL_JIT, so kernels_jit_compiled() is false and quantized_tile_tests compares the 32 x 32 kernel with itself. The 64-row, 64-column and BK=64 tiles are not exercised in CI. Covering them needs a JIT C++ lane.

… branch

Brings #35's fenced-input, fenced-wait and INT_MAX sealed-frame fixes and
#33's JACCL fixes under this branch's fence-order fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davidtai and others added 2 commits October 27, 2026 18:01
…ering branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g branch

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davidtai added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 27, 2026
Source/Cmlx/mlx moves from ea318ee0 to e637499f, the head of
Layr-Labs/mlx#41: the fast fence is updated before the event fence,
fenced inputs stay in device memory and take their waits first, a
sealed frame over INT_MAX is refused, and the JACCL mesh shares its
failure flag across wires and does all_gather offsets in 64 bits.
The range changes .cpp files that the package already compiles and
headers that it does not mirror (distributed/sealed.h, sealed_stream.h,
jaccl/lib). No Metal kernel, JIT source, CPU preamble input or CUDA
device file changes, so mlx-generated does not change. The Metal.hpp
commit stamp is written only by a cmake run and is left as it is.

Source/Cmlx/mlx-c moves from 73720efb to 2ac897e3, the head of
Layr-Labs/mlx-c#15. Only comments in mlx/c/distributed_group.h change;
both mirrored copies are refreshed as tools/update-mlx.sh and
tools/update-mlx-xcodeproj.sh copy them. The other 28 headers are
unchanged byte for byte.

fork.yaml: the Submodule pins prose names the new pins and fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davidtai added a commit to Layr-Labs/mlx-swift that referenced this pull request Oct 27, 2026
Source/Cmlx/mlx moves from e637499f to 72dffa2d. The range changes only
tests/jaccl/progress_guard_tests.cpp and
tests/jaccl/startup_wait_budget_group_tests.cpp, which Package.swift
excludes, so no mirrored header or generated source changes. mlx-c
stays at 2ac897e3.

fork.yaml: the Submodule pins prose names the new core pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davidtai added a commit to Layr-Labs/d-inference that referenced this pull request Oct 27, 2026
The closure carries the runtime fixes of the MLX review: the JACCL mesh failure flag
and 64-bit all_gather offsets (Layr-Labs/mlx#33), fenced inputs kept on device and sealed
frames over INT_MAX refused (Layr-Labs/mlx#35), the fast fence before the event fence
(Layr-Labs/mlx#41), and GLM5NextClusterImagePlan.inspectPNG offsets taken from the Data's
startIndex (Layr-Labs/mlx-swift-lm#308). Pins: libs/mlx-swift-lm d5bd32b8, libs/mlx-swift c633d221 (the revision mlx-swift-lm's Package.swift pins), libs/mlx 72dffa2d (mlx-swift's Source/Cmlx/mlx). mlx-c in that mlx-swift is 2ac897e3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davidtai added a commit to Layr-Labs/d-inference that referenced this pull request Oct 28, 2026
Brings the cluster beta (PR 1407, PR 1426, PR 1577) and master 52f79ef to
dev, with the independent review's fixes: the EngineV2LogprobsChannel cap
tests are restored (T1), a member that refuses inference releases its service
reservation (B1), and a stop while the pair is forming retires the slot
before it returns (C1). The MLX closure is pinned to the reviewed heads:
libs/mlx 72dffa2d (Layr-Labs/mlx#41), libs/mlx-swift c633d221
(Layr-Labs/mlx-swift#61; mlx-c 2ac897e3), libs/mlx-swift-lm d5bd32b8
(Layr-Labs/mlx-swift-lm#352). PR CI 40602298933 green except the two Vercel
projects whose root console-ui/ master deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@davidtai
davidtai changed the base branch from darkbloom/sealed-all-sum to main October 28, 2026 01:07
@davidtai
davidtai merged commit f24d7e6 into main Oct 28, 2026
22 checks passed
@davidtai
davidtai deleted the fix/cluster-metal-fence-wait branch October 28, 2026 01:36
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.

2 participants