Skip to content

gpu-kernels: the _sb tiles are off by default, so the clang-24 spill no longer costs 3.3x of Q4_K prefill (closes #782) - #1248

Merged
bong-water-water-bong merged 1 commit into
mainfrom
1bit/bump-gpu-kernels-48
Nov 6, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
1bit/bump-gpu-kernels-48

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Bumps the private HIP-kernel add-on pin (config/gpu-kernels.config) to the merge of 1bit-MONSTER/gpu-kernels#48 (262c2f9aced4), for engine#782: the m128_w8_f2_sb / _sb tile family is now off by default, so the clang-24 spill that cost 3.3× of Q4_K prefill is no longer the selected path.

The attribution this bump closes

engine#782 (same llama.cpp pin, same dispatch): rc-2026-10-31c lib = clang-23 code objects (49× clang 23), tip = clang 24 (52×). Clang-24 _sb tiles spill 320-576 B/lane; the default GGML_HRX_HIP_DENSE_PREFILL_M128_SB=1 selected them. Q4_K M=16384 K=2560 N=9728, same hip.dense_prefill.mm in every arm:

arm ms TFLOP/s
rc lib (clang-23 objects) 20.33 / 20.48 40.1
tip, old default (_sb) 70.81 / 70.92 11.5 (3.47x)
tip + this default flip (mosaic_bench, rebuilt libggml-hrx with the rebased addon) 21.11 / 20.42 38.7 / 40.0
tip, M128_SB=1 (old behaviour, still available) 68.61 / 69.22 11.9

Clock-matched at ~1.94-1.97 GHz; the residual ~5 % vs the rc binary is clang-24 vs clang-23 codegen on the scratch-free tile (ROCm/llvm side, not a fork change).

The fork change

gpu-kernels #48 rebased onto current main (94bc42e) after #59/#1064 rewrote the same m128_sb/m128_fits region — both changes kept (swiglu_tile follows m128_sb(), and the default flip lives inside sb_enabled()). Verified on strixhalo: tools/check.sh gfx1151 "headers match the manifest (409 kernels)", 45 add-on objects, 93 artifacts (only the 10 pre-existing hyperconn/lightning fingerprint failures, engine#52); tools/check-probes.sh PASS with the plain table when unset and the old _sb table at =1; the bench above.

Merge gate

The engine's plain build does not compile the add-on; the change is host-side dispatch code whose targeted evidence is the add-on's check/probes plus the bench on the real tensors, recorded above (the lane also offered hw-gate, but the box's gpu-hold FIFO makes a gate cycle a >1 h wait for a non-device change; that policy question is parked in #1077). Merging on green build + this evidence, as the previous gpu-kernels bump (#1246) was.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1064 - Partially compliant

Compliant requirements:

  • The swiglu_tile() function now consults m128_sb() to honor the GGML_HRX_HIP_DENSE_PREFILL_M128_SB knob
  • The PR updates the gpu-kernels pin to include the necessary changes for this behavior

Non-compliant requirements:

  • Verification that the plain tile (m128_w8_f1) exists for every format the fused matcher admits is not explicitly shown in the diff
  • The m64_w8_f1 branch behavior is not explicitly addressed in the diff

Requires further human verification:

  • Confirmation that the plain tile m128_w8_f1 exists for all relevant formats
  • Measurement of the m64_w8_f1 branch behavior to ensure it remains unchanged

1077 - Partially compliant

Compliant requirements:

  • The PR updates the gpu-kernels pin, which is a change to the HRX backend

Non-compliant requirements:

  • The PR does not explicitly address the CI job configuration or labeling requirements

Requires further human verification:

  • The CI job configuration needs to be verified to ensure the HRX backend is compiled
  • The labeling of PRs touching third_party/llama.cpp or ggml/src/ggml-hrx needs to be confirmed

782 - Partially compliant

Compliant requirements:

  • The PR updates the gpu-kernels pin to include the change that disables _sb tiles by default
  • The PR description confirms that the clang-24 spill no longer costs 3.3x of Q4_K prefill

Non-compliant requirements:

  • The PR does not include performance benchmarks to confirm the fix

Requires further human verification:

  • Performance benchmarks should be run to confirm the fix for the Q4_K prefill regression
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Pin Update

The PR updates the gpu-kernels pin to commit 262c2f9aced4f18d0f78c301c9f71030d4d44059. This change is critical for addressing performance issues related to the _sb tiles and ensuring that the HRX backend is properly compiled and tested.

sha=262c2f9aced4f18d0f78c301c9f71030d4d44059

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.

1 participant