Skip to content

refactor(rocke/attention): parity 2D lane defaults to production spec (ceiling behind --variant hlpv) - #9466

Closed
AviralGoelAMD wants to merge 1 commit into
users/avirgoel/ck/d256-gfx950-optfrom
users/avirgoel/ck/d256-parity-prod-default
Closed

AviralGoelAMD wants to merge 1 commit into
users/avirgoel/ck/d256-gfx950-optfrom
users/avirgoel/ck/d256-parity-prod-default

Conversation

@AviralGoelAMD

@AviralGoelAMD AviralGoelAMD commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Change

The 2D direct-launch lane in parity_unified_attention.py now builds the production spec (_tiled_spec_from_problem) for any cohort by default; the hand-tuned "aspirational ceiling" (num_warps=4 + half-local-PV) moves behind an explicit --variant hlpv.

  • Drops the _d256_gfx950_fast import/branch from the harness — new cohorts need no harness change.
  • Collapses the two duplicated build/compile/launch/time tails into one.
  • Parity probes (PARITY_KQ_SWIZZLE / PARITY_KQ_PAD) are now cohort-agnostic.
  • Net −28 lines (139 +, 167 −).

Verification (gfx950 / MI355X, 2D direct-launch, --skip-triton)

Compared refactored prod and --variant hlpv against this branch's current behavior, by kernel-name identity (noise-free) + timing:

scenario prod (new default) --variant hlpv (= old default) original
D256 sq4096 bf16 395 us (w2+interleave+ksb) 904 us (w4 base) 393 us
combo d64 64x8 bf16 55 us (w4+hlpv…) 48 us (same kernel) 48 us
combo d64 16x2 bf16 40 us (w4+hlpv) 43 us (w4 base) 43 us
prefill d128 b16 fp16 24 us (w2) 39 us (w4+mfma32) 37 us
  • prod reproduces D256 — byte-identical kernel name to this branch (395 vs 393 us).
  • --variant hlpv reproduces the old ceiling default on every non-D256 shape tested.
  • prod does change what non-D256 lanes report (production geometry ≠ the hand-tuned ceiling) — and in every tested case production is equal-or-faster than the ceiling (d128 24 vs 37 us; combo 16x2 40 vs 43 us; combo 64x8 same kernel). Production has caught up to / surpassed the stale ceiling; --variant hlpv keeps it measurable for any shape where you suspect it hasn't.

it moves the non-D256 default numbers, but production ≥ ceiling on every shape measured (4 shapes; not exhaustive — --variant hlpv is the escape hatch).

Relates to AICK-1495 (S7).

@AviralGoelAMD
AviralGoelAMD requested review from a team as code owners July 15, 2026 17:42
@AviralGoelAMD
AviralGoelAMD force-pushed the users/avirgoel/ck/d256-parity-prod-default branch from dcf2665 to 49c138f Compare July 15, 2026 19:23
@AviralGoelAMD
AviralGoelAMD changed the base branch from users/avirgoel/ck/d256-gfx950-opt to users/avirgoel/ck/d256-parity-cleanup July 15, 2026 19:23
@AviralGoelAMD
AviralGoelAMD force-pushed the users/avirgoel/ck/d256-parity-prod-default branch 6 times, most recently from ea166bd to 55fe9b5 Compare July 15, 2026 21:12
…; ceiling behind --variant hlpv

Stacked on the builders/common hoist + PARITY_*-env->CLI cleanup. Addresses the
branch with a variant switch. The 2D direct-launch lane now builds the
PRODUCTION spec (`_tiled_spec_from_problem`) for ANY cohort by default -- a new
cohort is measured with its real geometry with no per-cohort branch. The
hand-tuned "aspirational ceiling" (num_warps=4 + half-local-PV) moves behind an
explicit `--variant hlpv`.

- Drops the `_d256_gfx950_fast` per-cohort branch from the harness.
- Collapses the two duplicated build/compile/launch/time tails into one.
- The shared tail consumes the CLI-param probes (--kq-swizzle / --kq-pad /
  --probe-occupancy) and the hoisted print_occupancy from the cleanup commit.

GPU-verified on gfx950 (MI355X), 2D direct-launch:
- prod default reproduces D256 (byte-identical kernel name; 391 us @
  prefill_d256_gqa8_b16_sq4096) and the combo d64/b32 64x8 (54 us);
- --variant hlpv reproduces the old ceiling (combo 64x8 47.7 us).
@AviralGoelAMD
AviralGoelAMD force-pushed the users/avirgoel/ck/d256-parity-prod-default branch from 55fe9b5 to 4531287 Compare July 15, 2026 21:25
Base automatically changed from users/avirgoel/ck/d256-parity-cleanup to users/avirgoel/ck/d256-gfx950-opt July 15, 2026 21:28
@bartekxk
bartekxk requested a review from Copilot July 21, 2026 09:07

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

Refactors the gfx950 unified-attention parity/benchmark harness so the 2D direct-launch lane measures production-selected 2D geometry by default, while preserving the historical “aspirational ceiling” 2D spec behind an explicit --variant hlpv flag. This keeps new cohorts self-service (no harness edits) and makes parity probes cohort-agnostic.

Changes:

  • Add --variant {prod,hlpv} and plumb it into the 2D direct-launch lane (prod default uses _tiled_spec_from_problem for any cohort).
  • Remove the D256-special-cased fast-path import/branch and consolidate duplicated build/compile/launch/timing logic into a single shared tail.
  • Apply KQ parity probes (--kq-swizzle, --kq-pad) uniformly regardless of cohort/spec source.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1448 to +1450
else:
spec = _tiled_spec_from_problem(problem)
cache_tag = ("prod_direct",)
@AviralGoelAMD

Copy link
Copy Markdown
Contributor Author

cherry picked into parent PR.

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.

2 participants