Skip to content

test(rocke): re-bless the differential golden on develop - #14501

Merged
adickin-amd merged 1 commit into
developfrom
fix/rocke-rebless-differential-golden
Oct 28, 2026
Merged

adickin-amd merged 1 commit into
developfrom
fix/rocke-rebless-differential-golden

Conversation

@CongMa13

Copy link
Copy Markdown
Contributor

Fixes #14474

Summary: For human reviewers

The golden file llvm_gfx_all.json is stale. The checks rocke_differential_golden_ll and rocke_differential_golden_canonical fail on develop.

This PR records the golden again on f37a7056c79. The PR changes only llvm_gfx_all.json.

What changed

Review request

  1. Confirm the authors of feat(rocke): make the C++ engine consult the arch-domain table, and add error mode #13723 and fix(rocke): one valid K=128 f8f6f4 MFMA lowering for fp8, fp6 and fp4 #13976 meant the IR changes.
  2. Confirm the author of perf(rocke): build one K/V resource per page on the gfx942 2D i64 path #14349 meant the gfx942_attention_tiled_2d change. The probable source is perf(rocke): build one K/V resource per page on the gfx942 2D i64 path #14349.

Verification

  • Before the change, the golden check failed on develop for llvm20 and llvm22.
  • After the change, run_diff.py --mode ll --python-only --check-golden --flavor llvm20 --only target_intrinsics,moe_fused_mega_fp8 prints GOLDEN OK.
  • The full check for each flavor, the canonical check and the byte-identity gate did not run. CI runs them.

Summary: For AI reviewers

  • The diff changes one JSON file. No emitter code changes.
  • The command run_diff.py --record-golden wrote the file with --python-only.
  • Check the changed hashes only against the three families above.

🤖 Generated with Claude Code

The golden in llvm_gfx_all.json was recorded on a base without #13723
and #13976. Both PRs change the emitted IR. The checks
rocke_differential_golden_ll and rocke_differential_golden_canonical
fail on develop.

Re-record the golden on f37a705 for llvm20, llvm22, llvm23 and the
canonical IR mode. The golden also moves for
gfx942_attention_tiled_2d, which no earlier PR blessed.

Fixes #14474

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@CongMa13
CongMa13 requested a review from a team as a code owner October 27, 2026 17:50
@therock-pr-bot

therock-pr-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 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.

@cgmillette

Copy link
Copy Markdown
Contributor

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #14501 — re-bless the differential golden on develop (CongMa13) — 🟢 ACCEPT: exact, fully attributed re-bless

Prior item Status
Reviewers 🟢 None yet: no reviews or threads, only the policy bot
Issue #14474 (BrianHarrisonAMD, 10-27): #13723 left target_intrinsics[0,1,2,4,7,9,10,11] (ll) and [7,9,10,11] (canonical) unblessed; #13976 left moe_fused_mega_fp8[1..5] (ll) unblessed 🟢 Matches exactly (ran). The PR changes 63 anchors and nothing else:
• target_intrinsics: 8 changed + 1 added per ll flavor, 4 + 1 in canonical;
• moe_fused_mega_fp8: 5 changed per ll flavor;
• gfx942_attention_tiled_2d: 4 added per section.
No family is added or removed
Author's request 1: confirm that #13723 and #13976 meant the IR changes 🟢 Each change appears exactly at its PR's merge and matches that PR's stated intent (ran: every config emitted at each develop commit from before #13723 to today, llvm22; today's hashes equal the PR's anchors).
• #13723: each config now lowers for a target that has its intrinsics (the new ARCHES table). Configs 0, 1, 2 and 7 move to gfx1250 and config 4 to gfx1201, so their amdgpu-flat-work-group-size minimum drops from 64 to 32, as wave32 needs. Config 0 also gains gfx1250's s.wait.asynccnt. s.wait.event moves out of config 7 into a new config inserted at index 9. That shifts old configs 9–11 to 10–12 with identical hashes, so the real output changes are 0, 1, 2, 4, 7 and the new 9.
• #13976: in moe_fused_mega_fp8 configs 1–5, the only difference is that the K=128 scale MFMA is now called by its overload-mangled name (…16x16x128.f8f6f4.v8i32.v8i32). Undoing that suffix leaves identical IR. Config 0 has no K=128 MFMA and is unchanged
Author's request 2: confirm that the gfx942_attention_tiled_2d change came from #14349 🟢 Confirmed. The change only adds configs: 0–6 hash identically at every commit, and 7–10 first appear at #14349's merge (ran). They are #14349's new i64 K/V parity configs, which our follow-up on #14349 found byte-identical across the two engines at llvm20/22/23
Bots / CI 🟡 The policy bot, pre-commit, clang-tidy and Component CI pass. TheRock Multi-Arch is still building, so the hipkernelprovider shards that run the two golden CTests have not run yet (read)
Our checks 🟢
• The PR merges cleanly with develop (5 behind) (ran).
• We ran the exact CTest commands (run_diff.py … --python-only --check-golden: ll at llvm20/22/23, then canonical). On today's develop both fail, reproducing #14474. With the PR merged into today's develop, both print GOLDEN OK, well inside the 300 s CTest timeout (ran).
• Byte identity is unaffected: the PR changes only the golden
# Sev file:line Finding
N1 🟡 golden/llvm_gfx_all.json (merge order) develop broke because #13974's golden was recorded before #13723 and #13976 landed: develop does not require an up-to-date base. Any other rocKE PR that changes emitter output and merges before this one breaks the check again → land this next. If develop's rocKE tree moves first, re-run the two --check-golden commands on the merge before merging

Utility: 🟢 Positive — unblocks develop's golden checks, red on develop's hipkernelprovider runs since 05:14 UTC; exact and fully attributed.


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

First-round review: a local qwen3.6 model made the independent first pass. The orchestrator (Claude Opus 5.5) verified it. The checks were:

The orchestrator owns this merge-worthiness call. It flags whether a closer look is needed; it is not an approval or a rejection.

@shumway
shumway enabled auto-merge (squash) October 27, 2026 18:18
jakpiase added a commit that referenced this pull request Oct 28, 2026
…nd moe_fused_mega_fp8

rocke_differential_golden_ll and rocke_differential_golden_canonical fail
on develop, and so on this branch: #13723 changed the target_intrinsics
IR and the moe_fused_mega_fp8 LLVM IR after #13974 blessed the golden.

Re-record only those two families (llvm20, llvm22, llvm23 and canonical)
with run_diff.py --python-only --record-golden --only. The hashes match
the ones in #14501, which re-blesses the same entries on develop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adickin-amd pushed a commit that referenced this pull request Oct 28, 2026
…tor root (#14553)

## Summary

The hip-kernel-provider census now passes the descriptor loader an
absolute, normalized root. Before, it passed a path that climbs out of
the test tree and back down. On Windows, that climb pushed the gfx1151
WMMA attention decode pack (#14412) over the 259-character `MAX_PATH`
limit. The pack's descriptors never loaded, and
`TestGfx1151WmmaAttentionDecodePacks` failed on the Windows gfx110X
lane.

JIRA ID : ALMIOPEN-3093

## Problem

On develop, TheRock Multi-Arch CI's Windows gfx110X hip-kernel-provider
shards fail these four census cases:

- `hkp-census-gfx1100-TestGfx1151WmmaAttentionDecodePacks`
- `hkp-census-gfx1101-TestGfx1151WmmaAttentionDecodePacks`
- `hkp-census-gfx1102-TestGfx1151WmmaAttentionDecodePacks`
- `hkp-census-gfx1103-TestGfx1151WmmaAttentionDecodePacks`

The error in each is:

```
descriptor set 'hipkernel:Gfx1151WmmaAttentionDecode' did not load
```

Because these cases fail, the required **Multi-Arch CI Summary** check
is red on every PR that runs the hip-kernel-provider tests. Develop's
own post-merge run
[40611307596](https://github.com/ROCm/rocm-libraries/actions/runs/40611307596)
fails the same cases.

**Cause.** The census entry names its shard relative to its own
directory:

```
census/<arch>/../../../../arch_content/...
```

Windows resolves every descriptor file as that directory plus the climb,
so the climb counts against `MAX_PATH` even though the shard itself sits
well within it. The decode pack has the longest file names in the tree.
Its paths come to about 264-267 characters. The other packs come to
about 253-257.

## Change

`dnn-providers/hip-kernel-provider/src/tests/main.cpp` (census mode
only):
- After the existing check that the root is a directory, the census
computes `std::filesystem::absolute(root).lexically_normal()`.
- It sets `HIPDNN_DESCRIPTOR_DIR` to that value before any suite loads.
- If `absolute()` fails, the census keeps the original root, so the old
behavior is the fallback.

This is the same hunk that #14111 already carries. That PR found it and
validated it on a TheRock Multi-Arch run that passed on Windows gfx110X.
It is split out here so it can land ahead of #14111. When #14111
rebases, its copy of the hunk becomes a no-op.

## Risks and mitigations

| Risk | Mitigation |
|---|---|
| A census now loads a different tree | The normalized path names the
same directory, so the set of files loaded is identical. Only the
spelling of the path changes. |
| Something other than the census is affected | The change is inside
census mode in the test binary's `main()`. The plugin, the product
loader and the non-census test runs are untouched. |

## Testing

- The hunk is identical to the one in #14111, whose TheRock Multi-Arch
run passed the Windows gfx110X census.
- This PR's own TheRock Multi-Arch run should turn the four
`TestGfx1151WmmaAttentionDecodePacks` census cases green on Windows
gfx110X.

## Landing order

This PR unblocks the Windows census for:
- #14483 (ALMIOPEN-3024)
- #14358 and #14111, which land after #14483

The separate rocKE golden mismatch on the same shards is fixed by
#14501.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@adickin-amd

Copy link
Copy Markdown
Contributor

issueing override merge

@adickin-amd
adickin-amd disabled auto-merge October 28, 2026 14:07
@adickin-amd
adickin-amd merged commit a3b5d0c into develop Oct 28, 2026
339 of 363 checks passed
@adickin-amd
adickin-amd deleted the fix/rocke-rebless-differential-golden branch October 28, 2026 14:07
AviralGoelAMD added a commit that referenced this pull request Oct 28, 2026
…arry

Resolve the differential golden conflict with #14501 by regeneration:
take develop's file, then re-record the attention_tiled_2d and
attention_unified families (ll llvm20/22/23 + ir canonical, python-only).
Other families keep develop's entries.
jakpiase added a commit that referenced this pull request Oct 28, 2026
No textual conflicts; llvm_gfx_all.json auto-merged (develop re-bless #14501
plus this PR's stem cases).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jakpiase added a commit that referenced this pull request Oct 28, 2026
No textual conflicts. Develop brings rocke changes from #14426 (grouped
1x1 implicit-GEMM fwd addressing) and #14501 (differential golden
re-bless); IR golden checked after the merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

[rocke] rocke_differential_golden_{ll,canonical} fail on develop: golden stale after #13723 and #13976

4 participants