Skip to content

chore(build): reconcile auto-detected CUDA arch with the shipped shape - #1946

Merged
inureyes merged 7 commits into
mainfrom
update/issue-1943-auto-detect-arch-shape
Sep 21, 2026
Merged

inureyes merged 7 commits into
mainfrom
update/issue-1943-auto-detect-arch-shape

Conversation

@inureyes

@inureyes inureyes commented Sep 21, 2026

Copy link
Copy Markdown
Member

Summary

With MLX_CUDA_ARCHITECTURES unset, sm_arch_with_suffix appended CUDA's architecture-specific a suffix to every SM from 90 up, so a default cargo build --features cuda on this project's GB10 resolved the whole MLX target to 121a. That is the shape PR #1938 decided against, and it is self-defeating: the per-source injection in src/lib/mlx-cpp/CMakeLists.txt matches plain entries only and skips an a-carrying one as already architecture-specific, so on an auto-detected build it never fired at all. The defect is visible in this checkout's own target/, where two debug build directories from 2026-09-20 configured 121a and their libmlx.a carries every image architecture-specific, 98 of 98. A fixed auto-detect release build carries 1 of 99.

Auto-detection now stops suffixing at a shared FIRST_PLAIN_SM boundary, which is what makes the shipped mechanism work: Hopper keeps 90a, Blackwell is plain. Nothing here rests on throughput; that A/B was run and refuted, sitting inside the measurement method's own floor (docs/benchmark_results/data/blackwell-arch-specific-gb10-2026-09-21/).

The Hopper decision

Hopper keeps 90a because release.yml ships 90a and the job of auto-detection is to agree with it. Dropping it would create on Hopper exactly the local-versus-shipped divergence this closes on Blackwell.

The stated rationale for it is false at the current pin, so it is replaced rather than repeated. MLX_CUDA_SM90A_ENABLED has no definition in the pinned tree (81ba1c6a): upstream 44540d12, "[CUDA] JIT-compile qmm_sm80/qmm_sm90/gather_gemm", deleted the line defining it and moved those kernels to runtime NVRTC compilation, and jit_module.cpp derives the NVRTC --gpu-architecture from the running device, appending a itself from compute capability 9 up. Cross-compiled the pinned tree at 90 and at 90a here, with the real build's flags from its own compile_commands.json, and compared with cuobjdump. No Hopper device needed.

translation unit kernels emitted SASS lines changed of which .headerflags PTX changed
qmm_sm90.cu 0 0 0 0
qmm.cu 0 0 0 0
qmm_sm80.cu 0 0 0 0
qmv.cu 2898 5796 5796 0
fp_qmv.cu 54 108 108 0

So the suffix changes no instruction at this pin: the three JIT-compiled units emit no device function either way, and the two that emit code differ only in EF_CUDA_ACCELERATORS on the .headerflags line, the ELF flag marking a cubin architecture-specific. That is a reason to stop claiming 90a gates a kernel, not a reason to drop it: CUTLASS_ARCH_MMA_SM90A_ENABLED still keys on __CUDA_ARCH_FEAT_SM90_ALL, the way CUDA_ARCH_CONDITIONAL(1210) gates the Blackwell equivalent, so a later pin can make it matter. The no-detection fallback literal stays 90a on the same shipped-list argument: a fallback spelled 90 would hand a host without nvidia-smi a shape no published archive uses.

cuda_arch.rs is a deliberate extension beyond the issue's scope list: suggested_architecture applies the same rule, and the startup mismatch message is the only place the project hands an operator an MLX_CUDA_ARCHITECTURES value to paste. On a Blackwell card it named 121a, the one spelling this repository rejects everywhere else. The guard reads both functions, since rule 3 cannot see a list no workflow writes down. Two doc comments in that file were showing the shape the guard now rejects: compiled_cuda_architectures illustrated the string with "80;86;89;90a;100a-real;100;120a-real;120", replaced by the real x86_64 release list, and the test module's release-list header still justified Hopper's 90a by the deleted gate.

A sweep for other prose carrying the same dead rationale found eight live sites in total, all corrected here and none of them behavior: both release.yml jobs cited the deleted macro, three CUDA jobs in ci.yml justified their pin by auto-detection "yielding 121a" (it now yields 121, so each keeps the reason that survives rather than losing its rationale), and scripts/bench_block_width.sh carried the one claim that inverts rather than merely going stale, telling readers a row from an auto-detected build is not comparable with records pinned at plain 121 when after this change it is. No architecture list is touched. The draft-block-width-gb10-2026-09-20/harness/ directory counts as live rather than archival, and its four files are corrected too: the post-#1939 sweep and the #1943 A/B both re-used that harness, and draft-block-width-post-1939-gb10-2026-09-21/ ships no harness/ of its own because of it. The record prose under docs/benchmark_results/ and the TECHNICAL_REPORTS entry are left as written, since they document what was true when they were measured. The surface kept being wider than assumed at every pass, which is the pattern worth noting more than any single site.

Test plan

  • check_cuda_arch_lists.py passes here and fails on unmodified main, three findings per site naming this issue; check_cuda_arch_lists_test.sh 13 cases pass.
  • cargo fmt --check and cargo clippy -p mlxcel --lib --tests -- -D warnings at default features both exit 0. The warnings in that log are C++ build-script output from files this branch does not touch.
  • cargo test -p mlxcel-core --release --features cuda --lib -- --test-threads=1 cuda_arch in the auto-detect arm at this head: 27 passed.
  • Two arms, each in its own CARGO_TARGET_DIR because build.rs declares cargo:rerun-if-env-changed=MLX_CUDA_ARCHITECTURES and reuses one OUT_DIR. Both caches read 121; qmv.cu.o and fp_qmv.cu.o list sm_121 only; their --dump-sass hashes are equal across arms (84f0e788…, cc2c4d46…); each archive holds 1 architecture-specific image of 99, the auto arm's being fp_quantize.cu.o with 216 F2FP.SATFINITE.E2M1 instructions, so the injection fired.
  • Raw .o bytes do differ between arms, which the change does not explain: compiling one unmodified unit twice with identical flags in one directory also yields two different objects with the same SASS hash. nvcc is not byte-reproducible here, which is why the criterion is stated on SASS.
  • cargo check --features cuda,xla-iree --all-targets left to CI: IREE_DIST is unset on this host.

Closes #1943

With MLX_CUDA_ARCHITECTURES unset, `sm_arch_with_suffix` appended CUDA's architecture-specific `a` suffix to every SM from 90 up, so a default `cargo build --features cuda` on a GB10 resolved the whole MLX target to `121a`. That is the whole-target form PR #1938 decided against: every translation unit compiles under `__CUDA_ARCH_SPECIFIC__`, CUTLASS keys `CUTLASS_ARCH_MMA_SM121A_ENABLED` on the same macro, and the per-source injection in `src/lib/mlx-cpp/CMakeLists.txt` never fires at all, because an entry already carrying `a` is recorded as architecture-specific and skipped rather than given a duplicate `--generate-code`. A local build and a shipped build then differ in the machine code their decode kernels run, not merely in architecture coverage.

Both functions that spell a list from a detected capability now stop suffixing at a shared `FIRST_PLAIN_SM` boundary: `sm_arch_with_suffix` in the build script, and `CudaArchMismatch::suggested_architecture`, whose startup message was telling an operator on a Blackwell card to rebuild with `MLX_CUDA_ARCHITECTURES=121a`, the one value this repository rejects everywhere else.

Hopper keeps `90a` because `release.yml` ships `90a`. The stale rationale for it is replaced rather than repeated: upstream MLX commit `44540d12` moved `qmm_sm80`, `qmm_sm90` and `gather_gemm` to runtime NVRTC compilation and deleted the `MLX_CUDA_SM90A_ENABLED` definition the old comments cited, and `jit_module.cpp` now derives the NVRTC `--gpu-architecture` from the running device, appending `a` itself from compute capability 9 up.

`check_cuda_arch_lists.py` grows rule 6 for the auto-detect path, which rule 3 cannot see because no workflow writes that list down, and `check_cuda_arch_lists_test.sh` grows three cases for it.

Refs #1943, #1934, PR #1938.
The section stated the auto-detect rule as suffixing every SM from 90 up, citing a Hopper quantized-kernel gate, and then told operators not to set `a` on Blackwell and that building `121a` is a step backwards. Both passages described the same code path in opposite terms, and `check_docs` in the guard script only verifies that the release lists appear verbatim, so nothing caught it.

It now states the rule once, the way the build script and the release workflow both spell it, and explains what `121a` costs and why the per-source injection makes it unnecessary. The Hopper paragraph records what is actually true at the current MLX pin: upstream `44540d12` moved the Hopper quantized kernel to runtime NVRTC compilation and removed the `MLX_CUDA_SM90A_ENABLED` definition this section used to cite, `90a` is kept because both release lists ship it, and `CUTLASS_ARCH_MMA_SM90A_ENABLED` still keys on `__CUDA_ARCH_FEAT_SM90_ALL` so a later pin can make it matter again.

Refs #1943.
The first pass said the quantized translation units produce identical SASS at `90` and `90a`. Measured across all five, that was too strong in one direction and too weak in another. `qmm_sm90.cu`, `qmm.cu` and `qmm_sm80.cu` emit no device function at either spelling, because upstream `44540d12` moved those kernels to runtime NVRTC compilation; their objects carry 21 lines of fatbin header and zero kernel symbols. `qmv.cu` (2898 kernels) and `fp_qmv.cu` (54) do emit device code, and it is instruction-identical, but their cubins are not byte-identical: the `90a` build adds `EF_CUDA_ACCELERATORS` to the `.headerflags` line, which is the ELF flag marking a cubin architecture-specific. So the suffix changes no instruction at this pin, which is what the decision rests on, and the text now says that rather than implying the objects match byte for byte.

Also drops the two remaining statements of the pre-#1943 rule that the new guard does not read: the inline comment in `detect_cuda_arch`, and the `90a is load-bearing` rationale on the x86_64 release job, which cited the same deleted macro. The architecture lists themselves are untouched.

Refs #1943.
@inureyes inureyes added status:review Under review type:chore Maintenance tasks (build, CI, etc.) priority:medium Medium priority area:core mlxcel-core: MLX FFI, primitives, KV cache, layers platform:linux Linux (CUDA / packaging) specific labels Sep 21, 2026
The issue tied the `90a` fallback literal to the Hopper decision, and the cross-compile shows the suffix buys no instruction at this pin, so leaving the literal in place needed a reason rather than silence. The reason is the same one that keeps Hopper suffixed at all: `release.yml` ships `90a`, and a fallback spelled `90` would hand a host without `nvidia-smi` a build shape no published archive uses, which is the divergence this issue closed on Blackwell. The doc comment on `resolve_cuda_architectures` said only that `90a` is the last resort.

Refs #1943.
The doc comment on `compiled_cuda_architectures` illustrated the string with `"80;86;89;90a;100a-real;100;120a-real;120"`, which names Blackwell architecture-specific twice. A doc comment on a parser is where a reader copies a list from, and after this branch the project's own guard rejects that shape, so the file's example and the file's rule disagreed. It now shows the real x86_64 release list, which exercises the same parser features that mattered, pre-Blackwell plain entries and a Hopper `a` entry, and cannot drift into recommending something the build refuses. The `-real` and `-virtual` forms are still described on `parse_cuda_arch_entry` and round-tripped in `entry_display_round_trips_through_the_parser`, which is where an illustration of the grammar belongs.

The same pass corrects the header on the release-list constants in the test module, which still justified Hopper's `90a` by the quantized-kernel gate that upstream deleted.

Refs #1943.
…hape

Four comments justified themselves by what auto-detection used to produce. Three CUDA jobs in `ci.yml` said their pin exists because auto-detection "yields `121a` here"; after this branch it yields `121`. Each now states the reason that survives, that a gate should name the list it compiles rather than infer it from whichever GPU the runner has, so the pin keeps a rationale instead of becoming a bare value someone deletes as redundant.

`scripts/bench_block_width.sh` is the one that inverted rather than going stale. It told a reader that a row measured on an auto-detected build is not comparable with records pinned at plain `121`, which after this branch is backwards, since an auto-detected GB10 build now resolves to exactly `121`. A wrong comparability rule in a benchmark harness produces discarded runs or published numbers someone trusts, so it now says which rows are actually suspect: those recorded between #1934 and #1943 on an unpinned build.

The aarch64 release job carried the same deleted-macro rationale the x86_64 one did, missed in the earlier pass because only one of the two was grepped.

Refs #1943.
The `draft-block-width-gb10-2026-09-20/harness/` directory is a live tool despite its dated path: the post-#1939 width sweep and the #1943 architecture A/B both re-used it, and `draft-block-width-post-1939-gb10-2026-09-21/` ships no `harness/` of its own for exactly that reason. Its four files told a reader that `build.rs` auto-detects `121a` here, so a row from an unpinned build is not comparable with the pinned records. After this branch that is backwards: auto-detection resolves to plain `121`, the same list those records pin.

Each now keeps its pin and its reason, states that the pin no longer corrects a shape but still names what was compiled, and identifies the rows that are genuinely suspect, those recorded between #1934 and #1943 on an unpinned build, rather than implying every auto-detected build is.

The record prose under `docs/benchmark_results/` and the `TECHNICAL_REPORTS` entry are left as written; they document what was true when they were measured.

Refs #1943.
@inureyes
inureyes merged commit 6f257ab into main Sep 21, 2026
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core mlxcel-core: MLX FFI, primitives, KV cache, layers platform:linux Linux (CUDA / packaging) specific priority:medium Medium priority status:review Under review type:chore Maintenance tasks (build, CI, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: Reconcile auto-detect CUDA arch with the shipped build shape

1 participant