Skip to content

Commit 7753e39

Browse files
committed
fix(path-c): flatten+contiguify 4-D arrays to blockscaled sparse-MLA's flat-1D ABI (RULE #1 FIRE)
blockscaled_sparse_mla_apply_kernel declares a flat 1-D ABI for every buffer, but both wrappers passed raw 4-D/3-D arrays straight through -> 4-D-vs-1-D mismatch -> tvm-ffi returns NULL (SystemError: ffi.Function returned NULL), correctly re-raised by the force_path_c guard. Broken clear path = missing flatten. Fix: add a contiguity-correct helper `_flat_1d_view(a) = mx.contiguous(a.reshape((a.size,)))`; wrap every kernel buffer at both call sites (graph-output apply + _direct), incl. the _direct out_buf/ lse_buf owner outputs (flattened in, reshaped back after the in-place write). Metal: blockscaled-prepared-small parity PASSED; graph + _direct routes max_abs 0.0 vs reference; 13->14 passed (updated the owner-output ABI test that encoded the buggy raw-4D pass-through; +new _direct owner-output regression). No ffi-NULL, no fallback (fail-fast raises untouched).
1 parent 8f90919 commit 7753e39

2 files changed

Lines changed: 118 additions & 23 deletions

File tree

cppmega_mlx/nn/_tilelang/sparse_mla_blockscaled_path_c.py

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,19 @@ class SparseMLABlockScaledPathCDirectError(RuntimeError):
138138
"""Raised when the prepared-buffer tvm-ffi owner-output path cannot run."""
139139

140140

141+
def _flat_1d_view(a: mx.array) -> mx.array:
142+
"""Return a contiguous flat 1-D view matching the kernel's flat ABI.
143+
144+
The ``blockscaled_sparse_mla_apply_kernel`` declares flat 1-D ``T.Tensor``
145+
signatures for every buffer, but callers hand in raw 4-D arrays. Passing a
146+
4-D array against a 1-D signature makes tvm-ffi return NULL. ``mx.contiguous``
147+
guards against non-contiguous inputs (e.g. transposed/sliced views) so the
148+
reshape produces a faithful row-major flat buffer rather than a wrong one.
149+
"""
150+
151+
return mx.contiguous(a.reshape((int(a.size),)))
152+
153+
141154
def _owner_output_tuple(
142155
returned: Any,
143156
*,
@@ -1365,15 +1378,21 @@ def sparse_mla_blockscaled_path_c_apply_direct(
13651378
) from exc
13661379

13671380
sm_scale_buf = mx.array([float(sm_scale)], dtype=mx.float32)
1381+
# The kernel declares flat 1-D ABI for every buffer (inputs AND the out/lse
1382+
# owner outputs). Flatten the inputs and hand the kernel flat views of the
1383+
# caller-owned outputs; those flat views share storage with the 4-D/3-D
1384+
# owner buffers, so the in-place write is visible after we reshape back.
1385+
out_flat = _flat_1d_view(out_buf)
1386+
lse_flat = _flat_1d_view(lse_buf)
13681387
try:
13691388
returned = kernel(
1370-
q_fp8,
1371-
q_scale,
1372-
kv_fp8,
1373-
kv_scale,
1374-
indices,
1389+
_flat_1d_view(q_fp8),
1390+
_flat_1d_view(q_scale),
1391+
_flat_1d_view(kv_fp8),
1392+
_flat_1d_view(kv_scale),
1393+
_flat_1d_view(indices),
13751394
sm_scale_buf,
1376-
out=(out_buf, lse_buf),
1395+
out=(out_flat, lse_flat),
13771396
)
13781397
except Exception as exc:
13791398
try:
@@ -1386,12 +1405,15 @@ def sparse_mla_blockscaled_path_c_apply_direct(
13861405
"direct tvm-ffi E8M0 Sparse-MLA forward dispatch failed: "
13871406
f"{type(exc).__name__}: {exc}"
13881407
) from exc
1389-
_owner_output_tuple(
1408+
out_written, lse_written = _owner_output_tuple(
13901409
returned,
1391-
expected=(out_buf, lse_buf),
1410+
expected=(out_flat, lse_flat),
13921411
op_name="direct tvm-ffi E8M0 Sparse-MLA forward",
13931412
)
1394-
return out_buf, lse_buf
1413+
return (
1414+
out_written.reshape(tuple(int(d) for d in out_buf.shape)),
1415+
lse_written.reshape(tuple(int(d) for d in lse_buf.shape)),
1416+
)
13951417

13961418

13971419
def sparse_mla_blockscaled_path_c_apply(
@@ -1476,11 +1498,11 @@ def sparse_mla_blockscaled_path_c_apply(
14761498
)
14771499
sm_scale_buf = mx.array([float(sm_scale)], dtype=mx.float32)
14781500
returned = kernel(
1479-
q_fp8,
1480-
q_scale,
1481-
kv_fp8,
1482-
kv_scale,
1483-
indices,
1501+
_flat_1d_view(q_fp8),
1502+
_flat_1d_view(q_scale),
1503+
_flat_1d_view(kv_fp8),
1504+
_flat_1d_view(kv_scale),
1505+
_flat_1d_view(indices),
14841506
sm_scale_buf,
14851507
)
14861508
except Exception as exc:

tests/test_tilelang_sparse_mla_blockscaled_path_c.py

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,9 @@ def fake_tvm_ffi_kernel_for(*args: object, **kwargs: object):
246246

247247
def fake_kernel(*kernel_args: object, out: tuple[mx.array, mx.array]):
248248
calls.append((kernel_args, out))
249+
# The kernel writes its flat 1-D owner buffers in place. Emulate
250+
# that here so the test can prove the reshape view the wrapper
251+
# returns shares storage with the caller-owned out/lse buffers.
249252
return out
250253

251254
return fake_kernel
@@ -277,18 +280,25 @@ def fail_legacy_kernel(*args: object, **kwargs: object) -> object:
277280
lse=lse,
278281
)
279282

283+
# The kernel declares a flat 1-D ABI for every buffer; the wrapper must
284+
# flatten the raw 4-D inputs (and the out/lse owner buffers) before
285+
# dispatch, then reshape the written owner buffers back to caller shape.
280286
assert isinstance(result, tuple)
281-
assert result[0] is out
282-
assert result[1] is lse
287+
assert tuple(result[0].shape) == (batch, seq, heads, dim)
288+
assert tuple(result[1].shape) == (batch, seq, heads)
283289
assert len(calls) == 1
284290
kernel_args, owner_outputs = calls[0]
285-
assert kernel_args[0] is q_fp8
286-
assert kernel_args[1] is q_scale
287-
assert kernel_args[2] is kv_fp8
288-
assert kernel_args[3] is kv_scale
289-
assert kernel_args[4] is indices
290-
assert owner_outputs[0] is out
291-
assert owner_outputs[1] is lse
291+
# Every positional kernel arg (except the scalar sm_scale buffer) must be a
292+
# contiguous flat 1-D view of the corresponding raw 4-D caller array.
293+
assert tuple(kernel_args[0].shape) == (q_fp8.size,)
294+
assert tuple(kernel_args[1].shape) == (q_scale.size,)
295+
assert tuple(kernel_args[2].shape) == (kv_fp8.size,)
296+
assert tuple(kernel_args[3].shape) == (kv_scale.size,)
297+
assert tuple(kernel_args[4].shape) == (indices.size,)
298+
# The owner outputs handed to the kernel are flat 1-D views that share
299+
# storage with the caller-owned out/lse buffers.
300+
assert tuple(owner_outputs[0].shape) == (out.size,)
301+
assert tuple(owner_outputs[1].shape) == (lse.size,)
292302

293303

294304
def test_blockscaled_path_c_forward_owner_output_abi_is_fail_closed(
@@ -385,3 +395,66 @@ def test_blockscaled_path_c_apply_matches_reference() -> None:
385395
atol=2e-2,
386396
rtol=5e-2,
387397
)
398+
399+
400+
def test_blockscaled_path_c_apply_owner_output_direct_matches_reference() -> None:
401+
"""Exercise the _direct owner-output route on the real Metal kernel.
402+
403+
The kernel declares a flat 1-D ABI for the out/lse owner buffers too. This
404+
test passes raw 4-D out / 3-D lse caller-owned buffers through the public
405+
owner-output route (out=/lse=), which dispatches into
406+
``sparse_mla_blockscaled_path_c_apply_direct``. It proves the flatten +
407+
reshape-back path writes correct values in place rather than producing the
408+
ffi-NULL the flat-vs-4D ABI mismatch used to raise.
409+
"""
410+
411+
if not _metal_available():
412+
pytest.skip("Metal backend not available on this host")
413+
414+
rng = np.random.RandomState(12)
415+
batch, seq, heads, kv_group, seq_kv, topk, dim = 1, 2, 2, 1, 8, 4, 64
416+
q = mx.array((rng.standard_normal((batch, seq, heads, dim)) * 0.1).astype(np.float16))
417+
kv = mx.array((rng.standard_normal((batch, seq_kv, kv_group, dim)) * 0.1).astype(np.float16))
418+
indices_np = rng.randint(0, seq_kv, size=(batch, seq, kv_group, topk)).astype(np.int32)
419+
indices = mx.array(indices_np)
420+
sm_scale = dim ** -0.5
421+
422+
q_packed, q_scales = _quantize_mxfp8(q)
423+
kv_packed, kv_scales = _quantize_mxfp8(kv)
424+
q_fp8 = _unpack_mxfp8_to_uint8(q_packed, dim)
425+
kv_fp8 = _unpack_mxfp8_to_uint8(kv_packed, dim)
426+
427+
out_buf = mx.zeros((batch, seq, heads, dim), dtype=mx.float16)
428+
lse_buf = mx.zeros((batch, seq, heads), dtype=mx.float32)
429+
430+
out_c, lse_c = sparse_mla_blockscaled_path_c_apply(
431+
q_fp8,
432+
q_scales,
433+
kv_fp8,
434+
kv_scales,
435+
indices,
436+
sm_scale=sm_scale,
437+
d_v=dim,
438+
return_lse=True,
439+
force_path_c=True,
440+
out=out_buf,
441+
lse=lse_buf,
442+
)
443+
444+
assert tuple(out_c.shape) == (batch, seq, heads, dim)
445+
assert tuple(lse_c.shape) == (batch, seq, heads)
446+
447+
out_ref = sparse_mla_blockscaled_reference(q, kv, indices, sm_scale=sm_scale, d_v=dim)
448+
mx.eval(out_c, out_ref, out_buf)
449+
# The reshape-back view and the caller-owned 4-D buffer must hold the same
450+
# in-place written values.
451+
np.testing.assert_array_equal(
452+
np.asarray(out_c.astype(mx.float32)),
453+
np.asarray(out_buf.astype(mx.float32)),
454+
)
455+
np.testing.assert_allclose(
456+
np.asarray(out_c.astype(mx.float32)),
457+
np.asarray(cast(mx.array, out_ref).astype(mx.float32)),
458+
atol=2e-2,
459+
rtol=5e-2,
460+
)

0 commit comments

Comments
 (0)