From dafcf10a42ed36bfff8885c7b126043f5ab4784b Mon Sep 17 00:00:00 2001 From: Debasis Mandal Date: Sun, 13 Sep 2026 00:56:46 -0400 Subject: [PATCH] fix(rocm): point the Gemma routines at a capability that exists benchmarks/routines/rocm/support.py mapped gemma_rmsnorm and gemma_fused_add_rmsnorm to the capability op "layernorm", which arch_caps.py no longer declares -- it was removed once it turned out to have no ROCm kernel behind it. capability_available() therefore answered False and both routines filtered to no backends, so #364 shipped 21 of the 23 routines it claimed. Measured on gfx942 at c333f2003: capability_available(d, "layernorm", "hip") -> False capability_available(d, "gemma_rmsnorm", "hip") -> True rocm_supported_backends("gemma_rmsnorm", d) -> [] and through the runner, "[ERROR] No backends to test. Exiting." against a [PERF] row after the fix. Neither change was wrong alone and git merged both cleanly, because they touch different files. What is missing is not a test: test_filter_only_offers_backends_the_cli_accepts already fails on this, for both routines -- verified by restoring the key. It is a *lane*. tests/rocm/test_benchmark_harness.py is skipif(not IS_HIP) and no workflow references it, so the suite that catches this runs only on a maintainer's ROCm box, which is how the removal merged. So the guard goes in tests/rocm/test_arch_caps.py, which arch-caps-conformance.yml runs on every pull request with no GPU. That lane has no torch, and support.py reaches flashinfer, so the registry is parsed with ast rather than imported. The regex scan already in that file cannot cover it either: support.py passes `op` as a variable, so the strings sit in a dict literal and never appear at a call site. Two smaller review items from the same PR: bench_block_sparse_attention.py recorded kind, seq_len, head counts, density and num_blocks per row but not --seed. Recording it alone would have overstated what it pins: --seed reached only _block_mask's local generator, while every q/k/v came from the unseeded global RNG, so --accuracy could not be reproduced from a row. main() now seeds the global RNG too. testlist_rocm.txt's norm section was headed "bf16 and fp16" while all 7 of its commands pass --input_dtype bfloat16. Corrected the heading rather than adding fp16 cases, and recorded why in the file: vec_size is gcd(16/sizeof(T), d) and both are 2-byte, so fp16 would re-run the same path for numerical noise alone. Co-Authored-By: Claude --- .../rocm/bench_block_sparse_attention.py | 8 ++++ benchmarks/rocm/testlist_rocm.txt | 6 ++- benchmarks/routines/rocm/support.py | 4 +- tests/rocm/test_arch_caps.py | 40 +++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/benchmarks/rocm/bench_block_sparse_attention.py b/benchmarks/rocm/bench_block_sparse_attention.py index fd9ecb25833..9ebbc68924d 100644 --- a/benchmarks/rocm/bench_block_sparse_attention.py +++ b/benchmarks/rocm/bench_block_sparse_attention.py @@ -236,6 +236,10 @@ def _sweep(kinds, dry_run_iters: int, repeat_iters: int, nb: int, seed: int) -> "num_kv_heads": num_kv, "density": density, "num_blocks": nb, + # Layout and inputs are both seed-derived, so two + # seeds at one density time differently; without + # this the rows are indistinguishable. + "seed": seed, } try: if kind == "variable": @@ -402,6 +406,10 @@ def main() -> None: "block size would shrink the sparse problem below the dense baseline." ) + # --seed reached only the block mask; q/k/v came from the global RNG, + # so the seed recorded per row did not pin --accuracy's inputs. + torch.manual_seed(args.seed) + for key, value in _provenance().items(): print(f"# {key}: {value}") diff --git a/benchmarks/rocm/testlist_rocm.txt b/benchmarks/rocm/testlist_rocm.txt index 4c3621bb29a..4f4101b6f3e 100644 --- a/benchmarks/rocm/testlist_rocm.txt +++ b/benchmarks/rocm/testlist_rocm.txt @@ -78,12 +78,16 @@ --routine BatchPrefillWithRaggedKVCacheWrapper --backends fa2 auto --batch_size 16 --s_qo 1024 --s_kv 1024 --num_qo_heads 64 --num_kv_heads 8 --head_dim_qk 128 --head_dim_vo 128 --causal --q_dtype bfloat16 --kv_dtype bfloat16 --refcheck --generate_repro_command --case_tag "Llama-3.1-70B" --routine BatchPrefillWithRaggedKVCacheWrapper --backends fa2 auto --batch_size 4 --s_qo 4096 --s_kv 4096 --num_qo_heads 64 --num_kv_heads 8 --head_dim_qk 128 --head_dim_vo 128 --causal --q_dtype bfloat16 --kv_dtype bfloat16 --refcheck --generate_repro_command --case_tag "Llama-3.1-70B" -## Norm -- bf16 and fp16 +## Norm -- bf16 # The backend column reads "cuda": these routines take no backend argument, so # each op runs at its own default, which for norm and rope is native. For the # native-vs-AITER comparison run rocm/bench_norm.py with no flag; --aa there is # native against itself, i.e. the noise floor a ratio has to clear. # +# fp16 is deliberately absent: vec_size is gcd(16/sizeof(T), d) and both +# 2-byte types give the same one, so fp16 would re-run this path for +# numerical noise alone. +# # hidden_size 111 is in the sweep on purpose: the native kernel's vec_size is # gcd(16/sizeof(T), d), so an ill-aligned d goes fully scalar. A powers-of-two # sweep never sees it. diff --git a/benchmarks/routines/rocm/support.py b/benchmarks/routines/rocm/support.py index 4aef6ef5108..1e3d6905125 100644 --- a/benchmarks/routines/rocm/support.py +++ b/benchmarks/routines/rocm/support.py @@ -31,8 +31,8 @@ # norm "rmsnorm": ("rmsnorm", _NATIVE), "fused_add_rmsnorm": ("fused_add_rmsnorm", _NATIVE), - "gemma_rmsnorm": ("layernorm", _NATIVE), - "gemma_fused_add_rmsnorm": ("layernorm", _NATIVE), + "gemma_rmsnorm": ("gemma_rmsnorm", _NATIVE), + "gemma_fused_add_rmsnorm": ("gemma_rmsnorm", _NATIVE), # rope. apply_rope_with_cos_sin_cache is absent: the routine builds # cos_sin_cache in --input_dtype, but the op requires float32 and # --input_dtype offers none, so it fails on CUDA too. diff --git a/tests/rocm/test_arch_caps.py b/tests/rocm/test_arch_caps.py index f0f7a32a752..ba09ebe0a0f 100644 --- a/tests/rocm/test_arch_caps.py +++ b/tests/rocm/test_arch_caps.py @@ -293,6 +293,46 @@ def test_every_op_the_library_asks_for_is_declared(self): f"{sorted(used - declared)}" ) + def test_benchmark_registry_ops_are_declared(self): + """The benchmark registry names capability ops; check the targets exist. + + Parsed rather than imported: this lane has no torch, and support.py + reaches flashinfer. It is also why the regex scan above cannot cover it + -- support.py passes `op` as a variable, so the strings live in a dict + literal and never appear at a call site. + + `layernorm` sat here until arch_caps stopped declaring it; git merged + both changes cleanly and two Gemma routines silently stopped producing + rows. tests/rocm/test_benchmark_harness.py catches it too, but runs on + no CI lane. + """ + import ast + + root = pathlib.Path(__file__).resolve().parents[2] + src = root / "benchmarks" / "routines" / "rocm" / "support.py" + tree = ast.parse(src.read_text()) + referenced = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + if not any( + isinstance(t, ast.Name) and t.id == "_ROCM_ROUTINE_TO_CAP_OP" + for t in node.targets + ): + continue + for value in node.value.values: + op = value.elts[0] + assert isinstance(op, ast.Constant), f"non-literal cap op: {op!r}" + referenced.add(op.value) + + assert referenced, "no capability ops parsed; the registry moved or renamed" + assert "rmsnorm" in referenced, "parse found no known op; the shape changed" + declared = {c.op for c in arch_caps.CAPABILITIES if c.backend == "hip"} + assert referenced <= declared, ( + "benchmark registry names capability ops with no hip row: " + f"{sorted(referenced - declared)}" + ) + def test_known_bad_rows_explain_themselves(self): """A gate with no detail is unactionable for whoever hits it.""" for cap in arch_caps.CAPABILITIES: