feat: sm_121a (GB10) build lane + aarch64 c_char portability - #26
Conversation
Two bring-up fixes for NVIDIA GB10 (DGX Spark, aarch64, compute 12.1): - memra-engine build.rs: detect_arch mapped compute_cap 12.1 to the 120a gencode, producing SASS that fails at runtime with CUDA_ERROR_NO_BINARY_FOR_GPU. Add a 121a arch lane (arch=compute_121a,code=sm_121a) and auto-detect it. - Engine arch guard: accept 121a builds on 12.1 devices; stop accepting 120a builds on 12.1 (fail-closed instead of crashing at kernel launch). - cpu_experts.rs / cpu_native_check.rs: error buffers and FFI fn-pointer declarations hardcoded i8 where the ABI is c_char; aarch64 c_char is u8, so the crate did not compile. Use std::ffi::c_char throughout. No behavior change on x86_64 (c_char = i8 there).
There was a problem hiding this comment.
This is an auto review done by revuto.
Verified the two halves of this PR against the tree:
c_char portability — consistent. Base tree confirms MoeTokenFn/RowsFn in cpu_experts.rs were already c_char and only PrefetchFn + the error buffers were i8; the fix brings them in line. Imports are present at head (cpu_experts.rs:9, cpu_native_check.rs:1), every error-buffer consumer uses CStr::from_ptr(error.as_ptr()) (type-correct under both c_char == i8 and == u8), and the only remaining vec![0i8 (lib.rs:4848) is a device int8 activation buffer through htod_i8, not FFI — correctly left alone. No behavior change on x86_64 as claimed.
sm_121a lane — coherent. The 121a string flows unchanged through gencode (arch=compute_121a,code=sm_121a), MEMRA_BUILT_CUDA_ARCH, and the lib.rs guard tuple, so guard-vs-fatbin can't skew. Dropping ("120a", 12, 1) breaks no working configuration (that combo always died at module load with NO_BINARY_FOR_GPU; the new failure is the early rebuild hint). The mmq_fp4.cu stub-on-non-120a path applies to 121a as on B200 — fail-closed, consistent with the PR scope note. memra-probe/CUTLASS exclusions are explicitly deliberate in the PR body; fine.
One consistency finding below (FLAGS.md / install.sh still encode the old 12.1→120a mapping). Everything else looks good.
| let arch = match cap.as_deref() { | ||
| Some("12.0") | Some("12.1") => "120a", | ||
| Some("12.0") => "120a", | ||
| Some("12.1") => "121a", |
There was a problem hiding this comment.
This is an auto review done by revuto.
The new 12.1→121a mapping (and the 121a value accepted at build.rs:224) didn't get propagated to the surfaces that document/select this seam, and they now contradict the code:
docs/FLAGS.md:218(theMEMRA_CUDA_ARCHregistry row) still says "probe the GPU vianvidia-smi compute_cap(12.x→120a, …)" and enumerates the value set as120a/90a/100a/89— no121a. After this PR that row is wrong on both counts: auto-detect maps 12.1→121a, and a GB10 user who follows the doc and pinsMEMRA_CUDA_ARCH=120agets the fail-closed guard at lib.rs:1102 (which no longer accepts("120a", 12, 1)). CLAUDE.md:43-45 requires keepingdocs/FLAGS.mdaligned with the flag surface before integration; this is the exact lane where the doc would mislead.tools/install.sh:39still maps12.0|12.1) ARCH=120a— i.e. it selects the sm_120a prebuilt for a compute-cap-12.1 machine, a combination this PR's guard now deterministically rejects atEngine::new(install claims success, every binary refuses to start, and the "rebuild, MEMRA_CUDA_ARCH auto-detects" hint doesn't apply since there's no sm_121a release artifact). Currently dormant because install.sh:25 refuses aarch64 and no x86_64 cc-12.1 part exists, but it re-creates the exact "wrong arch selected silently" class the PR description set out to kill — dropping the12.1arm (or pointing it at a source build) would close it.
A one-line FLAGS.md row update (+ optionally install.sh) in this PR would keep the registry truthful for the hardware the lane targets.
|
@AnsenIO Hi, thanks for the pr, happy to accept it, please address the review left by revuto. |
|
Closing as stale — no activity since Aug 19 and the review findings (FLAGS.md registry row + install.sh 12.1 arm now contradicting the new guard) were never addressed. The sm_121a lane work itself is solid and appreciated — if you are still on GB10 and want to land this, feel free to reopen (or open a fresh PR) with those two doc/script surfaces aligned and I will review promptly. |
What
Bring-up for NVIDIA GB10 (DGX Spark; aarch64, compute capability 12.1) — two small fixes that take memra from "does not compile / does not run" to "all gates green" on this hardware:
aarch64
c_charportability (cpu_experts.rs,cpu_native_check.rs): error buffers and two FFI fn-pointer declarations (PrefetchFn,RowsFn) hardcodedi8where the ABI type isc_char. On aarch64c_charisu8, somemra-enginedid not compile at all. Switched tostd::ffi::c_charthroughout. No behavior change on x86_64 (c_char == i8there; this is a no-op on the primary lane).sm_121a build lane (
build.rs,lib.rs):detect_arch()mapped compute_cap12.1to the120agencode. The resulting fatbins contain only sm_120a SASS, which is not binary-compatible with sm_121 — every run died at kernel launch withCUDA_ERROR_NO_BINARY_FOR_GPU, and the runtime arch guard explicitly allowed this combination (("120a", 12, 1)), so the failure surfaced late and cryptically.detect_arch:12.1now maps to a new121aarch (arch=compute_121a,code=sm_121a);MEMRA_CUDA_ARCH=121aaccepted as an explicit override.("121a", 12, 1); no longer accepts("120a", 12, 1)— fails closed with the rebuild hint instead of crashing at launch.12.0behavior unchanged.__CUDA_ARCH__ >=style, and themxf4nvf4/kind::f8f6f4inline PTX assembles cleanly undersm_121a(CUDA 13.0.88).Evidence (all on one GB10, CUDA 13.0.88, driver 580+, rustc 1.96)
kernel-check— ALL GREEN (82 cells, 22 skipped) (skips are the twoQwen3.6-35B-A3B-UD-IQ4_XS.gguf-dependent cells; model not on this box):run-genprime gates,Avifenesh/Qwen3.8-27B-NVFP4-MTP-GGUF(Q5K-mtp):run-specK=1..8 self-consistency, same model +frspec-mixed32768trim, real 18-token chat prompt — 8/8 PASS, token-identical to plain decode:First perf touchpoint (same prompt sweep,
MEMRA_PROMPT_DIR, K=4, gen-only tok/s): plain 8.6 → spec 15.0 tok/s (1.74x, 46.5% acceptance). Reference point on the same box: llama.cppllama-benchtg128 = 2.71 tok/s onQwen3.8-27B-Q4_K_M(ngl 99). No tuning claimed — GB10 has no tuned defaults and this PR sets none.Scope / what this PR is not
memra-probe's build.rs still hardcodessm_120a(documented phase-0 spine) — left as-is deliberately; say the word if you want it parameterized too.Assisted-by: Hermione (IABAI agent) for the aarch64/sm_121a bring-up and on-device verification; hardware and gates run by the account owner.