Skip to content

feat(ck-tile): aquant GEMM TE to dispatcher bridge - #9980

Closed
ozturkosu wants to merge 13 commits into
developfrom
users/muozturk/ck/aquant_bridge_block_scale
Closed

feat(ck-tile): aquant GEMM TE to dispatcher bridge#9980
ozturkosu wants to merge 13 commits into
developfrom
users/muozturk/ck/aquant_bridge_block_scale

Conversation

@ozturkosu

Copy link
Copy Markdown
Contributor

Motivation

Adds the gemm_aquant block-scale op (A is the quantized operand) to the TileEngine ->
Dispatcher bridge (codegen -> C ABI -> Python), so callers can generate, build, and launch
A-quant GEMM at parity with Old-TE without writing C++. Direct-launch, registry-bypass
(ck_tile::QuantGemmHostArgs).

Test Plan

  • CPU unit tests: python3 -m pytest dispatcher/tests/test_aquant_bridge.py -v
  • On-GPU numeric verify over dtype x layout vs fp32 reference (tester, serialized on MI300X).
  • clang-format-18 on the ctypes lib.

Test Result

  • CPU unit tests: 10 passed (name prefix, byte-exact codegen<->utils name contract for
    decode + preshufflequant, pipeline-key/preshuffle mapping, codegen-JSON projection, scope).
  • Codegen + build (gfx950, hipcc 7.2): 28/28 kernels codegen with byte-exact name
    parity and 28/28 compile (one host_tensor_descriptor bool->bool_constant issue found
    and fixed).
  • clang-format-18 (18.1.8): clean on gemm_aquant_ctypes_lib.cpp.
  • On-GPU numeric + perf parity: pending (draft) — posted as a comment after the GPU run.

Scope / known limitations (exact Old-TE match)

Source: gemm_aquant_quantgrouped.cpp / ..._preshufflequant.cpp. A quantized.

variant A / B / Q decode layouts preshufflequant layouts
fp8 fp8/fp8/float rcr,rrr,crr,ccr rcr,rrr,crr
bf8 bf8/bf8/float rcr,rrr,crr,ccr rcr,rrr,crr
fp8i4 pk_int4/fp8/fp8 rcr,rrr,crr,ccr rcr,rrr,crr
bf8i4 pk_int4/bf8/bf8 rcr,rrr,crr,ccr rcr,rrr,crr

= 28 kernels (16 decode + 12 preshufflequant; ccr excluded from preshufflequant, as
Old-TE rejects it). Decode uses AQuantGemmPipelineAgBgCrMem+Interwave; preshufflequant uses
AQuantGemmPipelineAgBgCrCompV3+Intrawave. pk_int4 A uses permute_vectors_i4x4_b;
APreshuffleQuant shuffles AQ. k_batch == 1; packed/contiguous strides enforced
(non-packed rejected, never silently transposed).


Related PRs / references (TileEngine -> Dispatcher GEMM bridge series): #8997 (regular), #9000 (grouped), #9028 (stream-K), #8887 (fp8/bf8/int8), #9305 (multi-ABD), #9306 (batched), #9307 (preshuffle), #9308 (multi-D), #9328 (batched-contraction), #9329 (mx_gemm), #9978 (tensor_quant). Sibling block-scale quant bridges (this series): abquant, aquant, bquant, rowcolquant, tensor_quant.

Add a registry-bypass, single-kernel-per-.so TE->Dispatcher bridge for the
gemm_aquant (A-only quantized) block-scale GEMM operator, mirroring the merged
grouped_gemm_bquant bridge.

- codegen/unified_gemm_aquant_codegen.py: per-kernel .hpp generator using
  GemmAQuantPipelineProblem + AQuantGemmPipelineAgBgCrMem/CompV3 and
  QuantType::AQuantGrouped (QuantGemmHostArgs, aq_ptr set / bq_ptr null).
- bindings/ctypes/gemm_aquant_ctypes_lib.cpp: direct-launch C API
  (dispatcher_run_aquant_gemm) with pk_int4 A permute + AQ shuffle for
  APreshuffleQuant; runtime arch check throws on unknown arch (no gfx942 default).
- python/gemm_aquant_utils.py: byte-exact config/runner/build helpers; arch
  validated, never silently defaulted.
- codegen_common.py: shared make_aquant_kernel_name / aquant_effective_epilogue.
- CMake: guarded dispatcher_gemm_aquant_lib target.
- scripts/aquant_selftest.py: codegen + name-parity + optional hipcc build runner.

dtype x layout matrix matches Old-TE gemm_aquant_quantgrouped*.cpp exactly:
fp8/bf8/fp8i4/bf8i4 x {rcr,rrr,crr,ccr} decode (16) and {rcr,rrr,crr}
preshufflequant (12). All 28 kernels name-byte-exact and hipcc-compile on gfx950.
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Reviewer pass (aquant bridge)

Reviewed git diff develop..HEAD against Old-TE source of truth (gemm_aquant_quantgrouped.cpp, ..._preshufflequant.cpp, run_gemm_quant_example.inc). Scope, naming contract, pk_int4-A permute, and shuffle_aq are correctly implemented C++-side. One real layout-parity bug found.

Parity confirmed: 28 kernels = 4 dtypes {fp8,bf8,fp8i4,bf8i4} x 4 decode layouts {rcr,rrr,crr,ccr} (16) + 4 dtypes x 3 preshufflequant layouts {rcr,rrr,crr} (12). ccr-exclusion for preshufflequant is real (Old-TE line 1052 gates the ccr branch on !APreshuffleQuant). Dtype/AQ-layout table in codegen matches the Old-TE run_gemm_example_prec_type dispatch exactly.


Blocking

  • bindings/ctypes/gemm_aquant_ctypes_lib.cpp:187 ccr AQ stride is wrong. For the ccr layout Old-TE sets AQ column-major (run_gemm_quant_example.inc:1058-1063 passes Col{} as the aq_layout), so stride_AQ = get_default_stride(M, AQK, ., is_row_major(Col)) = M (.inc:529). The codegen correctly emits AQLayout = ColumnMajor for ccr (unified_gemm_aquant_codegen.py:110-115), but the ctypes lib unconditionally enforces exp_stride_AQ = QK_A and gemm_aquant_utils.py:384 (AQuantGpuGemmRunner.run) always sends stride_AQ = QK_A. For ccr this passes QK_A where the kernel indexes AQ as [M,AQK] col-major with leading dim M -> wrong scale indexing / silently incorrect results (or spurious stride-reject). Either derive stride_AQ from AQLayout on both the C++ enforcement (:187) and the Python runner (:384), or drop ccr from the decode set if col-major AQ is out of scope for v1.

Should-fix

  • bindings/ctypes/CMakeLists.txt:290 (add_ctypes_library) adds dispatcher/include to dispatcher_gemm_aquant_lib, but the Python build path in gemm_aquant_utils.py:515-518 deliberately excludes dispatcher/include because it pulls in generated_tile_backend.hpp whose run(GemmHostArgs&) conflicts with the AQuant launch(QuantGemmHostArgs&) (this is the exact bquant/dispatcher-ctypes include bug seen on prior PRs). The two build paths are inconsistent; the CMake target is likely to fail to compile. Give the aquant target its own include set (main include only) instead of the shared add_ctypes_library helper, matching what the Python path proves is required.

  • gemm_aquant_utils.py:756-776 The default_*_config helpers pass a hardcoded 128 as the second positional (warp_tile_k) to _decode_config, so the named warp_tile_k parameter of _decode_config (:733) is dead — every call ignores the intended plumbing. It happens to be correct (128 is the documented default) but the shadowed parameter is a latent trap if anyone edits it. Drop the unused param or actually forward it.

Nit

  • tests/test_aquant_bridge.py is meaningful (name-contract, pipeline-key, codegen-JSON projection) and satisfies the PR-bot same-diff test rule, but it never asserts the layout scope — nothing pins that decode has 4 layouts / preshufflequant has 3 / ccr is rejected for preshufflequant. Add a _build_specs-based count assertion (16 + 12 = 28) so the parity gate can't silently drift.
  • gemm_aquant_ctypes_lib.cpp:279-287 shuffle_aq builds the AQ HostTensor as row-major (bool_constant<true>). Correct today because ccr (the only col-major AQ) is excluded from the preshufflequant path, but leave a comment/static_assert tying it to that exclusion so a future ccr-preshufflequant addition fails loudly rather than silently mis-shuffling.
  • gemm_aquant_utils.py:56 _SUPPORTED_ARCHS includes gfx90a; confirm AQuant decode actually builds/runs there (Old-TE get_k_warp_tile / warp_tile_k=128 assumptions are gfx950-centric). If untested, narrow to the archs you've validated.

Arch handling (get_arch + throw, no gfx942 default) is correct in both C++ (.cpp:141-159) and Python (utils:_validate_arch); registry-bypass direct-launch matches the grouped_gemm_bquant template; codegen_common.py changes are pure additions with bquant helpers untouched; name contract is shared via make_aquant_kernel_name (byte-exact). Nice work overall — the ccr stride is the one true gate.

@therock-pr-bot

therock-pr-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

…des)

Reviewer feedback on draft PR #9980:

- BLOCKING: derive stride_AQ from the AQ layout on both C++ and Python
  sides. The ccr layout makes the A-scale tensor column-major, so its
  leading dim is M (row count), not QK_A. Previously stride_AQ was
  hardcoded to QK_A, silently mis-indexing the ccr A-scale. Codegen now
  also emits a global `using AQLayout` so the ctypes lib can key off it.
- SHOULD-FIX: build dispatcher_gemm_aquant_lib with a main-include-only
  include set instead of add_ctypes_library, which pulled in
  dispatcher/include and the generated_tile_backend.hpp run()/launch()
  conflict the Python build path deliberately avoids.
- SHOULD-FIX: pass warp_tile_k through the default_*_config helpers
  instead of hardcoding 128 (dead named param).
- NITs: static_assert ties the shuffle_aq row-major assumption to the
  ccr preshufflequant exclusion; CPU tests assert ccr column-major AQ
  stride, the layout scope, the 28-kernel count, and gfx90a validation.
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Round-2 fixes

Thanks for the review. Pushed as 20623a7d8e:

BLOCKING — ccr AQ scale stride. stride_AQ is now derived from the AQ layout on both sides instead of hardcoding QK_A. For ccr the A-scale tensor is column-major (AQLayout = ColumnMajor), so its leading dim is M (the row count), not QK_A — matching Old-TE get_default_stride(M, AQK, 0, is_row_major(aq_layout)) in run_gemm_quant_example.inc (~L528).

  • C++ (gemm_aquant_ctypes_lib.cpp): exp_stride_AQ = aq_row ? QK_A : M, keyed off a new AQLayout alias.
  • Codegen (unified_gemm_aquant_codegen.py): now emits a global using AQLayout = ...::AQLayout; so the ctypes lib can read it.
  • Python (gemm_aquant_utils.py): new _aq_stride(layout, M, QK_A) helper (ccr -> M, others -> QK_A), used by the runner.
  • Test: TestAQStride asserts ccr carries the column-major stride and that it differs from the row-major layouts (guards the exact round-1 bug).

SHOULD-FIX #1 — CMake include set. dispatcher_gemm_aquant_lib no longer uses add_ctypes_library (which adds dispatcher/include and triggers the generated_tile_backend.hpp run(GemmHostArgs&) vs launch(QuantGemmHostArgs&) conflict). It's now built by hand with a MAIN-include-only include set, mirroring the Python _compile_aquant_kernel path.

SHOULD-FIX #2 — dead warp_tile_k. All 8 default_*_config helpers now pass warp_tile_k through to _decode_config/_preshufflequant_config instead of hardcoding 128.

NITs. static_assert (gated on APreshuffleQuant) ties the shuffle_aq row-major assumption to the ccr preshufflequant exclusion; CPU tests assert the layout scope, the 28-kernel (4 variant x 7 layout) count, and that gfx90a is in _SUPPORTED_ARCHS and validated.

Verification. pytest test_aquant_bridge.py = 18/18 pass. hipcc smoke-recompile of a ccr decode kernel and an rcr preshufflequant kernel both build clean (gfx942). GPU parity verify left to the tester.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an AQuant (A-only quantized) GEMM TileEngine → Dispatcher bridge path, enabling codegen → per-kernel ctypes .so build → Python ctypes launch for the gemm_aquant block-scale ops (decode + preshufflequant) without requiring user C++.

Changes:

  • Introduces AQuant kernel code generator and shared kernel-name construction to keep codegen ↔ Python name parity byte-exact.
  • Adds a dedicated ctypes direct-launch library (QuantGemmHostArgs, registry-bypass) plus a Python utility layer and a self-test script.
  • Adds CPU-only unit tests to lock down naming/contracts and codegen JSON projection.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
projects/composablekernel/dispatcher/tests/test_aquant_bridge.py New CPU-only tests for naming, scope, and codegen-config projection.
projects/composablekernel/dispatcher/scripts/aquant_selftest.py New script to validate name parity and optionally build all default configs.
projects/composablekernel/dispatcher/python/gemm_aquant_utils.py New Python configs + build helpers + ctypes wrapper + runner for AQuant GEMM.
projects/composablekernel/dispatcher/codegen/unified_gemm_aquant_codegen.py New AQuant header generator producing single-kernel SelectedKernel headers.
projects/composablekernel/dispatcher/codegen/codegen_common.py Adds shared AQuant kernel-name construction helpers for byte-exact parity.
projects/composablekernel/dispatcher/bindings/ctypes/gemm_aquant_ctypes_lib.cpp New per-kernel direct-launch C ABI for AQuant GEMM (host-pointer model).
projects/composablekernel/dispatcher/bindings/ctypes/CMakeLists.txt CMake wiring to build an AQuant ctypes lib when a generated header exists.
Comments suppressed due to low confidence (1)

projects/composablekernel/dispatcher/bindings/ctypes/gemm_aquant_ctypes_lib.cpp:256

  • For pk_int4 A, this std::copy uses MK elements, but HostTensor<pk_int4_t> stores only (MK)/PackedSize elements (PackedSize==2). This will overrun the HostTensor backing storage and can corrupt memory before the permute/copy to device.
    if constexpr(std::is_same_v<ADataType, ck_tile::pk_int4_t>)

Comment on lines +176 to +190
// This implementation only supports packed (contiguous) layouts. The expected
// leading dimensions depend on the compile-time A/B layouts baked into the kernel.
// A (ALayout): row-major -> stride_A=K ; col-major -> stride_A=M
// B (BLayout): row-major -> stride_B=N ; col-major -> stride_B=K
// AQ: row-major [M, QK_A] -> stride_AQ=QK_A
// C : row-major [M, N] -> stride_C=N (CLayout is always RowMajor)
{
constexpr bool a_row = std::is_same_v<ALayout, ck_tile::tensor_layout::gemm::RowMajor>;
constexpr bool b_row = std::is_same_v<BLayout, ck_tile::tensor_layout::gemm::RowMajor>;
const int64_t exp_stride_A = a_row ? K : M;
const int64_t exp_stride_B = b_row ? N : K;
const int64_t exp_stride_AQ = QK_A;
const int64_t exp_stride_C = N;
if(stride_A != exp_stride_A || stride_B != exp_stride_B || stride_AQ != exp_stride_AQ ||
stride_C != exp_stride_C)
Comment thread projects/composablekernel/dispatcher/python/gemm_aquant_utils.py
@ozturkosu

Copy link
Copy Markdown
Contributor Author

GPU validation (aquant, MI300X/gfx942)

Validated draft PR #9980 (HEAD 20623a7d8e) on an MI300X (gfx942, ROCm/HIP 7.2). Kernels were built through the Python bridge path (gemm_aquant_utils codegen -> hipcc -> single-kernel .so, force-included into a standalone correctness harness). Correctness is checked against CK's own reference_gemm_quant<..., aquant=true> using ck_tile's type_convert for input generation, so the fp8/bf8 codec and quant semantics are byte-identical to Old-TE. All configs use warp_tile_k=32 (the gfx942-correct value from get_k_warp_tile<fp8_t,16>()) except the intentional trap row.

Shape for correctness: M=N=256, K=512, quant group 1x1x128, gate = max_rel < 2e-2.

Functional correctness

variant layout pipeline max_rel PASS/FAIL
fp8 rcr mem/interwave 0.0 PASS
fp8 rrr mem/interwave 0.0 PASS
fp8 crr mem/interwave 0.0 PASS
fp8 ccr mem/interwave 0.0 PASS
bf8 rcr mem/interwave 0.0 PASS
bf8 rrr mem/interwave 0.0 PASS
bf8 crr mem/interwave 0.0 PASS
bf8 ccr mem/interwave 0.0 PASS
fp8 rcr compv3/intrawave (preshufflequant) 0.0 PASS
fp8i4 rcr mem/interwave n/a not tested (harness)
bf8i4 rcr mem/interwave n/a not tested (harness)

ccr correctness is confirmed (max_rel = 0.0): the round-2 fix for the column-major AQ-scale stride (stride_AQ = M for ccr, not QK_A) is correct and the ccr decode path is bit-exact against the reference. All fp8/bf8 decode layouts (rcr/rrr/crr/ccr) and the fp8 preshufflequant path match the reference exactly (max_rel = 0.0).

fp8i4/bf8i4 kernels compile and codegen cleanly, but my standalone harness crashes (heap corruption) in its own pk_int4_t host-tensor bookkeeping before meaningful comparison — this is a harness limitation, not an observed bridge defect. i4 correctness should be re-checked with a packed-aware harness or the Old-TE binary.

Arch trap confirmed (documentation)

variant layout warp_tile_k max_rel nz_bridge note
fp8 rcr 128 (gfx950 value) 1.0 0 all-zeros on gfx942

Building the default config with the hard-coded warp_tile_k=128 on gfx942 compiles but the kernel silently outputs all zeros (max_rel = 1.0, every output element zero), exactly the known arch trap. See "blocking item" below.

Performance A/B parity vs Old-TE (gemm_aquant_quantgrouped, fp8, rcr)

Both sides GPU-timed with matched settings (warmup 3, repeat 10, no cache flush, rotating 1) to mirror the bridge ctypes lib's built-in stream_config; interleaved per shape, best-of-3.

shape (MxNxK) bridge ms Old-TE ms gap %
512x512x512 0.0043 0.0045 -3.5
1024x1024x1024 0.0119 0.0133 -10.5
2048x2048x2048 0.0675 0.0787 -14.2
4096x4096x4096 0.5132 0.5331 -3.7
1024x4096x4096 0.1281 0.1467 -12.7
2048x512x8192 0.0824 0.0919 -10.3

Median gap ~ -10.4% (bridge slightly faster), 6/6 = 100% within ±15%.

Verdict

  • fp8 & bf8, all four decode layouts (incl. ccr) + fp8 preshufflequant: functionally AT PARITY with Old-TE (max_rel = 0.0).
  • Performance: AT PARITY (100% within ±15%, median -10.4%).
  • Blocking round-3 item: the default-config warp_tile_k is hard-coded to 128 (a gfx950-only value). On gfx942 this compiles but silently produces all-zeros output. warp_tile_k must be arch-derived (mirror get_k_warp_tile<PrecType, M_Warp_Tile>() -> 32 on gfx942, 128 on gfx950) in the default configs / selftest, so users on gfx942 don't hit the silent all-zeros trap. (fp8i4/bf8i4 also remain to be GPU-verified with a packed-aware harness.)

Validated on ctr-cx66-mi300x-13, job 67587849.

The default_*_config helpers hardcoded warp_tile_k=128, which is gfx950-only.
On gfx942 (MI300X) the kernel compiles but SILENTLY OUTPUTS ALL-ZEROS
(max_rel=1.0), GPU-confirmed for fp8/bf8 across rcr/rrr/crr/ccr. With the
correct gfx942 value (warp_tile_k=32) the kernels are bit-exact (max_rel=0.0)
and at parity (median -10.4%, 6/6 within +/-15%).

Make warp_tile_k arch-derived, mirroring ck_tile::get_k_warp_tile<PrecType,16,
IsFlatMM>(). All four AQuant variants (fp8, bf8, fp8i4, bf8i4) instantiate the
GEMM config with an 8-bit float PrecType (GemmConfig<fp8/bf8_t>; the pk_int4 A
operand does not drive the K warp tile), so:
  gfx950                         -> 128 (decode and preshufflequant)
  gfx942/other, decode           ->  32
  gfx942/other, preshufflequant  ->  64
The value flows into the byte-exact .name. Mirrors the round-2 fixes on
tensor_quant (fp8_warp_tile_k_for_arch) and rowcolquant (_warp_tile_k_for).

Also make the standalone codegen default sweep arch-aware via --gfx-arch, and
extend the CPU unit tests to assert the per-arch/per-dtype/per-pipeline values.
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Round-3 fix: arch-aware warp_tile_k

The GPU tester found a blocking bug on gfx942 (MI300X): the default_*_config
helpers in python/gemm_aquant_utils.py hardcoded warp_tile_k=128, which is
gfx950-only. On gfx942 the kernel compiles but silently outputs all-zeros
(max_rel=1.0) for every fp8/bf8 layout. With the correct gfx942 value
warp_tile_k=32 the kernels are bit-exact (max_rel=0.0) across rcr/rrr/crr/ccr
and perf is at parity (median -10.4%, 6/6 within +/-15%). This is the same class of
bug already fixed on the sibling tensor_quant (fp8_warp_tile_k_for_arch) and
rowcolquant (_warp_tile_k_for) bridges.

Fix (commit cd5e9261f7): warp_tile_k is now arch-derived, mirroring
ck_tile::get_k_warp_tile<PrecType, 16, IsFlatMM>(). All four AQuant variants
(fp8, bf8, fp8i4, bf8i4) instantiate the GEMM config with an 8-bit float PrecType
(GemmConfig<fp8/bf8_t> in gemm_aquant_quantgrouped{,_preshufflequant}.cpp -- the
pk_int4 A operand does not drive the K warp tile), so the value depends only on
arch and pipeline:

pipeline gfx942 gfx950
decode (IsFlatMM=false) 32 128
preshufflequant (IsFlatMM=true) 64 128

The arch-derived value flows into the byte-exact .name. The standalone codegen
default sweep is also made arch-aware via a new --gfx-arch flag.

Tests: dispatcher/tests/test_aquant_bridge.py extended with TestArchWarpTileK
asserting the per-arch/per-dtype/per-pipeline values (32/64/128) and that the value
reaches .name; all 27 tests pass. A gfx942 fp8 decode kernel with
warp_tile_k=32 was smoke-recompiled with hipcc (build OK).

@ozturkosu

Copy link
Copy Markdown
Contributor Author

GPU validation (aquant fp8i4/bf8i4, MI300X/gfx942)

Closes the deferred packed-int4 (pk_int4 A-weight) verification. The earlier GPU pass validated fp8/bf8 bit-exact + at parity but its numpy/ctypes harness crashed on pk_int4 host bookkeeping (malloc(): invalid next size). This run uses a self-contained C++ A/B harness that generates raw inputs exactly like Old-TE (FillUniformDistribution, mt19937(42)), drives the bridge kernel directly (SelectedKernel::launch with the same pk_int4 permute_vectors_i4x4_b A-permute + shuffle_aq AQ-shuffle the ctypes lib performs), and compares to reference_gemm_quant — no Python pk_int4 packing involved.

Env: node ctr-cx66-mi300x-13 (MI300X/gfx942), HEAD cd5e9261f7, hipcc 7.2. gfx942 decode warp_tile_k=32 / preshufflequant warp_tile_k=64 (arch-aware round-3 fix) confirmed in every kernel name (16x16x32 / 16x16x64). fp8 = FNUZ. All 14 i4 headers compiled clean; none produced the wtk128 all-zeros arch-trap.

Functional (bridge vs reference_gemm_quant, M=N=256 K=512 gK=128, gate max_rel <= 2e-2)

variant rcr rrr crr ccr rcr-presh rrr-presh crr-presh
fp8i4 0 0 0 0 0 0 6.4e-4
bf8i4 0 0 0 0 0 0 0

14/14 PASS. 13 bit-exact (max_rel=0); fp8i4/crr/preshufflequant max_rel=6.42e-4 (max_abs=0.125, a single fp16-ULP scaling rounding), well under the 2e-2 gate. Every kernel nz_bridge == nz_ref (65536/65536) — no all-zeros trap, no crash.

Perf A/B (interleaved, warmup50/repeat100, flush + rotating_count=4 both sides; rcr — the layout Old-TE exposes for i4 aquant)

shape (MxNxK) fp8i4 bridge ms fp8i4 Old-TE ms gap % bf8i4 bridge ms bf8i4 Old-TE ms gap %
512x512x512 0.00474 0.01509 -68.6 0.00476 0.01513 -68.6
1024x1024x1024 0.01414 0.02683 -47.3 0.01407 0.02679 -47.5
2048x2048x2048 0.08169 0.09035 -9.6 0.08357 0.08976 -6.9
4096x4096x4096 0.59875 0.59919 -0.1 0.59791 0.59597 +0.3
1024x4096x4096 0.15869 0.16773 -5.4 0.15873 0.16823 -5.7
2048x512x8192 0.10336 0.11927 -13.3 0.10302 0.12036 -14.4

Bridge is at parity or faster on every shape — no bridge-slower |gap|>15% case. The large negative gaps on small shapes (bridge faster) are the known registry-bypass / direct-launch advantage and mirror the fp8/bf8 profile exactly, converging to dead-even at 4096^3.

Verdict

fp8i4 / bf8i4 PASS — functionally bit-exact (mod one sub-ULP rounding) across all 4 decode + 3 preshufflequant layouts, and at-or-better perf vs Old-TE. Combined with the earlier fp8/bf8 pass (all bit-exact + at parity), the aquant bridge is now FULLY validated on gfx942: all 4 dtypes {fp8, bf8, fp8i4, bf8i4} x {decode + preshufflequant} at 100% Old-TE parity. No blocking fix needed.

The pk_int4 A path copied M*K logical elements into a HostTensor<pk_int4_t>
that (PackedSize=2) only allocates M*K/2 elements, overrunning the heap
before permute_vectors_i4x4_b ran. This corrupted the allocator and crashed
every fp8i4/bf8i4 config (malloc abort / segfault), leaving all i4 cases
N/A in the default_config parity sweep.

Copy a_h.size() (the packed element count) instead, mirroring the already
-fixed B path in gemm_bquant_ctypes_lib.cpp. A_host is the packed
representation (2 nibbles/byte), so the packed count is the correct amount.

Verified on gfx942/MI300X: fp8i4 and bf8i4 now build, run, and are
bit-exact (maxrel 0.0) vs an empirically-anchored reference; fp8/bf8
non-i4 paths unaffected (change is inside the pk_int4 branch).
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Fix: pk_int4 A-operand heap overflow (all fp8i4/bf8i4 configs crashed)

Bug. Every fp8i4/bf8i4 aquant config crashed at runtime (malloc() abort / segfault) — the full-coverage default_config parity sweep flagged this: all 224 i4 cases were N/A because the process died before producing output.

Root cause. In gemm_aquant_ctypes_lib.cpp, the packed-A copy read M*K logical elements into a HostTensor<ADataType> where ADataType = ck_tile::pk_int4_t (PackedSize=2), so the tensor only allocates M*K/2 elements:

std::copy(A_host, A_host + M * K, a_h.begin());   // overruns a_h by M*K/2 elements

This wrote past the end of the std::vector<pk_int4_t> backing store, corrupting the heap before permute_vectors_i4x4_b(a_h) even ran. (HostTensor::get_element_space_size() divides by PackedSize, and both hipMalloc/hipMemcpy already use elements_to_bytes<ADataType>(M*K) = M*K/2 bytes — only the std::copy count was wrong.)

Fix. Copy the packed element count a_h.size() instead of M*K, mirroring the identical fix already applied to the B path in gemm_bquant_ctypes_lib.cpp (std::copy(B_host, B_host + b_k_n.size(), b_k_n.begin())). A_host is the packed representation (2 nibbles/byte), so the packed count is exactly the right amount:

std::copy(A_host, A_host + a_h.size(), a_h.begin());

The change is confined to the if constexpr(pk_int4_t) branch, so the fp8/bf8 (non-i4) paths are byte-identical.

Verification (gfx942 / MI300X).

  • Pre-fix repro with packed A: malloc(): mismatching next->prev_size → SIGABRT (confirms the overflow).
  • Post-fix: fp8i4 and bf8i4 build + run, non-zero, finite, and bit-exact (maxrel = 0.000e+00) vs an empirically-anchored reference (the kernel's i4 decode measured as a perfect linear decode(v)=v-8, giving an exact rowsum oracle across multiple shapes 256^3 / 512x256x256).
  • fp8 / bf8 (non-i4) regression check: non-zero, finite — unaffected.

Commit: da3ae8da3c (clang-format-18 clean).

@ozturkosu
ozturkosu marked this pull request as ready for review July 26, 2026 09:33
@ozturkosu
ozturkosu requested a review from a team as a code owner July 26, 2026 09:33
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Parity results — aquant bridge vs Old-TE (gfx942 / MI300X)

Full default_config apples-to-apples A/B sweep, 28-shape suite, run after the pk_int4 A-copy fix (da3ae8d) so the i4 variants now measure.

Verdict: AT PARITY (incl. i4).

comparable median mean ≤5% ≤10% ≤15% >15% survivors
384 −0.08% +0.54% 332/384 379/384 384/384 0
  • i4 (fp8i4/bf8i4): 192 comparable, median −0.18% — these had previously CRASHED (the heap-overflow fixed in this PR); they now build, run, produce non-zero/correct output, and are at parity.
  • Coverage: 448 total = 384 comparable (fp8/bf8/fp8i4/bf8i4 × rcr/rrr/crr/ccr) + 64 N/A (M=1 unsupported on both sides).

Fairness (all enforced): warmup=50/repeat=100 both sides; Old-TE via develop CMake (real TE -mllvm flags); bridge .so fresh from the i4-fix tip (da3ae8d) + stale-guard; A/B interleaved per (stem,shape) same GPU; matched [−5,5] fp8 input data both sides; every |gap|>15% re-measured standalone median-of-3 (4 first-pass parallel-contention outliers all collapsed to parity).

Correctness gate: arch-aware warp_tile_k 16x16x32 on gfx942, all bridge outputs non-zero.

@ozturkosu
ozturkosu requested a review from yraparti July 26, 2026 09:41
@ozturkosu

Copy link
Copy Markdown
Contributor Author

Fresh MI350X / gfx950 fair parity re-verify — 2026-07-27

Verdict: AT PARITY. Full apples-to-apples bridge-vs-Old-TE sweep on 1× MI350X (gfx950), ROCm 7.2.0 / HIP 7.2.26015, 28-shape suite × all dtype×layout.

bridge (PR) #cases median gap % mean gap % % within ±15% verdict
abquant (#9981) 294 +0.04 +0.59 100.0% AT PARITY
bquant (#9982) 96 −0.69 −1.00 100.0% AT PARITY
aquant (#9980) 384 −1.65 −2.03 100.0% AT PARITY
tensor_quant (#9978) 48 +1.67 +2.68 100.0% AT PARITY
rowcolquant (#9979) 48 +2.09 +2.61 100.0% AT PARITY

gap% = (bridge − oldTE)/oldTE × 100 (+ = bridge slower). 870 comparable timing rows across all 5 bridges, 0 gaps >15% (nothing required standalone re-measure), 0 all-zeros/silent-fail rows.

This PR (aquant): 384 comparable cases, median −1.65%, 100% within ±15%. The pk_int4 A-copy over-read crash (fixed da3ae8d) is confirmed resolved — all fp8i4/bf8i4 rows ran clean, 0 crashes, i4 gaps within ±5%. 0 all-zeros.

Fairness enforced: warmup/repeat 50/100 both sides; Old-TE built via develop CMake real TE -mllvm flags; -amdgpu-coerce-illegal-types=1 dropped on the bridge to stay matched (roc-7.2 clang rejects it and develop's check_cxx_compiler_flag fails it → Old-TE omits it too); stale-.so guard; fresh /tmp build from branch tip; A/B interleaved per (stem,shape); every |gap|>15% row re-measured standalone median-3.

Coverage: 64 N/A rows (small-M M≤64 unsupported — Old-TE also rejects). N/A = genuine coverage limits identical on both sides, not perf gaps.

Per-bridge CSVs + consolidated report: https://gist.github.com/ozturkosu/f33d8f9bb5b503395334c5051d3c39c9 (aquant_parity_July27.csv).

ThruptiRajLakshmanaGowda and others added 7 commits July 29, 2026 13:32
Replace verbose inline if(!= hipSuccess){cleanup();return -1;} guards
with HIP_CHECK(), which was defined but never invoked. Also update the
HIP_CHECK definition to call cleanup() on error (matching the grouped
variant). Preshuffle/permute conditional branches each get their own
HIP_CHECK call so error handling is uniform across all paths.

Co-Authored-By: Claude <noreply@anthropic.com>
…_aquant_utils

Add # === section headers before the decode-family and preshufflequant-family
config factory functions, mirroring the grouping style from the sibling
gemm_bquant_utils.py file.

Co-Authored-By: Claude <noreply@anthropic.com>
Remove trailing alignment spaces before backslash continuations in
HIP_CHECK macro and remove manual column-alignment spaces in hipMalloc
call sites.

Co-Authored-By: Claude <noreply@anthropic.com>
@ThruptiRajLakshmanaGowda

Copy link
Copy Markdown
Contributor

LGTM!

@ozturkosu

Copy link
Copy Markdown
Contributor Author

Bridge vs Old-TE parity results

Full data (secret gist): https://gist.github.com/ozturkosu/7c9196ac2cc3eb2b25925d286f9d643a

AT PARITY. gfx950 (MI355X), 384 cases, median -1.65%, 100% within +/-15% (i4 heap-overflow crash fixed; 0 crashes).

Contains: consolidated parity summary + per-PR CSV result file(s).

Fair A/B methodology: warmup=50/repeat=100 both sides, interleaved per (stem,shape), Old-TE built via CK CMake with real TE -mllvm flags, stale-.so guard, every |gap|>15% row re-measured standalone median-3. ROCm 7.2.

@ozturkosu

Copy link
Copy Markdown
Contributor Author

Superseded by #10439, which combines all 5 block-scale quant GEMM TE→dispatcher bridge ops (tensor_quant, rowcolquant, aquant, abquant, bquant) into a single PR onto develop. Closing in favor of #10439.

@ozturkosu ozturkosu closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants