Skip to content

perf(rocke): enable exp2_fast for bf16 head_dim=128 in gfx942 dense prefill - #10832

Closed
AviralGoelAMD wants to merge 3 commits into
developfrom
users/avirgoel/ck/gfx942-dense-bf16-exp2-pr
Closed

AviralGoelAMD wants to merge 3 commits into
developfrom
users/avirgoel/ck/gfx942-dense-bf16-exp2-pr

Conversation

@AviralGoelAMD

@AviralGoelAMD AviralGoelAMD commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

ISSUE ID : AICK-1991

Enable exp2_fast for bf16 head_dim=128 in the dense prefill softmax — the last config still on the slower guarded exp2 (fp16 and bf16-D64 already used it).

The holdout was a stale spill: it was measured on the old pre-lazy-rescale body. The current kernel doesn't spill with exp2_fast (zero scratch on both grids), and the output is bit-identical to plain exp2 (both softmax args are ≤ 0, exactly its precondition) — so it's a free softmax speedup on bf16-D128.

Diff: _use_exp2_fast → True, plus a golden re-bless of the four bf16-D128 IR cases (fp16 and D64 unchanged; gfx950 untouched). No C++ mirror under library/kernels/, so byte-identity doesn't apply.

@AviralGoelAMD
AviralGoelAMD requested review from a team as code owners August 14, 2026 21:44
@github-actions github-actions Bot added project: hip-kernel-provider rocke-lib: attention Touches rocKE library attention/FMHA kernels labels Aug 14, 2026
@therock-pr-bot

therock-pr-bot Bot commented Aug 14, 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

therock-pr-bot Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

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

@AviralGoelAMD AviralGoelAMD changed the title perf(hip-kernel-provider): enable exp2_fast for bf16 head_dim=128 in gfx942 dense prefill perf(rocke): enable exp2_fast for bf16 head_dim=128 in gfx942 dense prefill Aug 14, 2026
@AviralGoelAMD
AviralGoelAMD requested a lite review from Copilot August 14, 2026 22:19

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 updates the gfx942 attention_dense softmax policy to always use the exp2_fast path (removing the prior bf16 + head_dim=128 exception) and refreshes the associated golden IR hash artifacts to match the newly emitted kernels.

Changes:

  • Enable exp2_fast unconditionally in attention_dense softmax policy for gfx942 (including bf16 D128).
  • Update golden IR SHA256/size records for affected bf16 D128 cases under both llvm20 and llvm22.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
dnn-providers/hip-kernel-provider/rocke/library/kernels/gfx942/attention_dense.py Removes the bf16 D128 policy exception by making _use_exp2_fast() always return True.
dnn-providers/hip-kernel-provider/rocke/library/tests/golden/attention_dense_gfx942_ir_sha256.json Updates sizes and SHA256 hashes for the impacted bf16 D128 gfx942 attention_dense golden artifacts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…refill

The bf16-D128 exp2_fast holdout was justified by a pre-lazy-rescale spill over the
waves-per-eu=2 cap. rocprofv3 on the current lazy-rescale kernel shows the opposite:
VGPR drops and scratch stays zero, output numerically identical to plain exp2. Remove
the stale holdout in _use_exp2_fast (return True), update the in-file status/policy
comments that referenced the old exception, and re-bless the four bf16-D128 golden
cases across both llvm flavors; fp16 and D64 are byte-identical.
@AviralGoelAMD
AviralGoelAMD force-pushed the users/avirgoel/ck/gfx942-dense-bf16-exp2-pr branch from 8499f41 to d4de8c1 Compare August 15, 2026 04:04
_use_exp2_fast now returns True for all configs (the bf16 D128 spill
holdout is stale on the lazy-rescale schedule), so the pinned matrix in
test_exp2_fast_gate_matches_the_spill_measured_matrix must expect True
for (128, bf16). Flip the case and refresh the docstring; the parametrized
case fails on the pre-change gate and passes after, guarding the flip.
@therock-pr-bot

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

…no-spill

- black reformats the flipped (128, bf16, True) parametrize case
- _use_exp2_fast docstring now cites the llvm-readelf --notes code-object read
  (.private_segment_fixed_size / .vgpr_count) for the bf16-D128 no-spill claim,
  matching the PR body: the lazy-rescale build reports 0 scratch on both the
  default and persistent grids (VGPR reduced, not increased, vs plain exp2).
correctness one.
"""
return dtype == "fp16" or head_size != 128
return True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason I keep this function to just return true is becasuse we can reuse this later in a new scenario where some datatype would need to be excluded.

@AviralGoelAMD

Copy link
Copy Markdown
Contributor Author

Recreated as #10885 on users/avirgoel/rocke/gfx942-dense-bf16-exp2. This is a rocKE (hip-kernel-provider) PR, not a Composable Kernel PR; the /ck/ branch name was incorrectly triggering CK CI. Identical content; closing this one to avoid duplicate CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants