Skip to content

[rocjitsu] Fix zero-selector FLOAT32 buffer stores and enable guitar-hero coverage - #13988

Merged
kuhar merged 13 commits into
developfrom
users/newling/rocjitsu-guitar-hero-ci-fixes
Oct 30, 2026
Merged

kuhar merged 13 commits into
developfrom
users/newling/rocjitsu-guitar-hero-ci-fixes

Conversation

@newling

@newling newling commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Untyped buffer stores with a single-channel FLOAT32 format and all four descriptor DST_SEL fields set to zero are rejected by the emulator. The guitar-hero instruction corpus intentionally exercises this and other known emulator bugs, including fixes already under review when the cases were added. Enabling the newer corpus exposed the CI failures tracked in #12583. Native gfx1100 and gfx1201 accept this descriptor and route it like the A8 selector view.

This PR resolves the qualified zero-selector view to A8 before packing the store. W supplies the memory channel when present. When W is absent, the existing missing-channel policy supplies X on gfx1201 and zero on gfx1100. These measurements cover gfx1100 and gfx1201; the routing and zero-fill policy on the other supported targets are extrapolated. Both buffer entry points use one helper for qualification and selector resolution. Shared image packing keeps its existing behavior.

Regressions use distinct X/Y/Z/W values, including packed D16 components, and cover all nine ordinary/D16 store forms with sparse EXEC. Direct packing covers both missing-channel policies. Boundary checks through both buffer APIs reject zero selectors for single-channel integer, narrower float and multichannel formats without changing output bytes or issuing a store. Integer and multichannel image-store regressions preserve the image behavior.

The other relevant emulator fixes have landed: #13705 supplies typed integer minmax lowering, floating-dot DPP availability and per-source FP8/BF8 selection; #13691 supplies the DOT arithmetic and accumulator modifiers. This PR retains both native DOT4 fixture sets and decoded coverage of arithmetic, accumulator modifiers, plain/DPP operand handling, wave sizes, host rounding modes, sparse EXEC and overlapping registers. Related generator coverage is consolidated, and the DOT arithmetic and DPP availability contracts are documented.

Both workflow corpus references advance from e191fc2 to ebb4004d, enabling the guitar-hero cases. Bounded apt retries handle transient dependency-download failures. The separate VMEM/GDS completion work and newer corpus pin remain in #12583.

Validation: the rebased Clang Release and ASan/UBSan builds pass the affected buffer/image, DOT/WMMA and integer regressions, including the captured arithmetic cases. The affected Python suites and changed-file hooks pass. All ten guitar-hero cases pass with the exact pinned corpus and the workflow's ROCm SDK version.

Appendix: omitting the large SGEMM corpus case from UBSan lanes

The corpus case rocblas_sgemm.large_2048x2048 on gfx1201 takes about 141 s in the Clang ASan+UBSan lane without vptr checks and 360-457 s with them, which exceeds the lane's 360 s first-pass timeout. On the parent commit, the asan-ubsan lane failed on this case: its gfx1201 pass took 374 s and failed when this case timed out, and the case passed only in a 458 s rerun. The final commit adds --ubsan to run-corpus-tests.sh, which deselects the cases listed in ubsan_excluded_cases from the simulator suites, and the workflow passes it for both UBSan lanes. Those lanes still run the smaller rocBLAS SGEMM cases, and Release still runs the large one.

To check that nothing else was dropped, the table compares the corpus test summaries (passed / skipped) of run 41003966356 on this commit with run 40995568287 on its parent. The counts are identical in the ubsan-gcc and asan-ubsan lanes, apart from the parent's asan-ubsan timeout described above.

Arch Before After
gfx942 81 / 23 81 / 22
gfx950 75 / 11 75 / 10
gfx1100 80 / 6 80 / 5
gfx1201 118 / 7 117 / 7
gfx1250 112 / 52 112 / 51

Exactly one item is removed per architecture. On gfx1201 it is a passing test; on the other four architectures the case was already skipped, so one skip disappears. No other passed count changes, and the case name no longer appears in either UBSan log. The native ctest counts are unchanged, with all 9845 tests passing in ubsan-gcc and all 9857 in asan-ubsan. After the change, the gfx1201 pass takes 44 s in asan-ubsan instead of 374 s. The release lane still passes the large case on gfx1201, and the tsan lane also passes. For comparison, the parent run's ubsan-gcc job passed the large case in its gfx1201 pass, which took 114 s.

@github-actions github-actions Bot added github actions Pull requests that update GitHub Actions code project: rocjitsu labels Oct 22, 2026
@therock-pr-bot

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

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 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.

@newling newling added CI bug fix Helps unblock CI — improves stability or fixes failure. Considered very high priority. High Priority and removed High Priority CI bug fix Helps unblock CI — improves stability or fixes failure. Considered very high priority. labels Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

gfx1250 B0-to-A0 translation baseline

Warning

The PR result is not directly comparable to the latest develop baseline.

Changed provenance:

Field develop PR
corpus_commit e191fc247b8c04f984f41cf2b2a83ff89d6a0854 ebb4004d7633293e94539b3577fb126397b8c6a4

Compared PR head 5aab2e0ae014c3d317c6fb01214575fca640a255 using rocjitsu-test-corpus run 41003966356.

@newling
newling marked this pull request as ready for review October 22, 2026 00:52
@newling
newling requested review from a team and atgutier as code owners October 22, 2026 00:52
@newling
newling requested a review from kuhar October 22, 2026 00:52
@kuhar

kuhar commented Oct 22, 2026

Copy link
Copy Markdown
Member

The newer guitar-hero instruction corpus exposed three deterministic emulator failures in CI for #12583. Investigating those failures also uncovered a related DOT4 arithmetic bug.

FYI, this was by design, I added test cases that covered bugs that I had already fixed locally (either in-review or blocked by other work)...

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just several comments

@newling
newling marked this pull request as draft October 22, 2026 15:12
@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch 2 times, most recently from ec5c16a to ece83e6 Compare October 22, 2026 16:06
@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch from ece83e6 to cf9f4a1 Compare October 23, 2026 16:25
@newling
newling marked this pull request as ready for review October 23, 2026 16:30
@newling
newling requested a review from kuhar October 23, 2026 16:30
@newling newling changed the title [rocjitsu] Fix guitar-hero corpus instruction and sanitizer failures [rocjitsu] Fix zero-selector FLOAT32 buffer stores and enable guitar-hero coverage Oct 23, 2026
@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch from cf9f4a1 to f7990da Compare October 23, 2026 17:41

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added a few comments

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added 2 comments and an inline reply

@newling
newling requested a review from kuhar October 26, 2026 15:21
@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch from f7990da to 77a7ea7 Compare October 26, 2026 15:22

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch 2 times, most recently from d7a8411 to 83a74c0 Compare October 27, 2026 15:19

@newling newling left a comment •

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 ubsan-gcc timeout on rocblas_sgemm.large_2048x2048 (gfx1201) is likely from #14021, which roughly doubles emulation time in GCC UBSan builds, not from this PR.

@newling
newling force-pushed the users/newling/rocjitsu-guitar-hero-ci-fixes branch from 8e29340 to c0f37a4 Compare October 30, 2026 03:07
The corpus case rocblas_sgemm.large_2048x2048 on gfx1201 runs for about
141 s in the Clang ASan+UBSan lane without vptr checks and 360-457 s
with them, past the lane's 360 s first-pass timeout.

run-corpus-tests.sh gains --ubsan, which deselects the cases listed in
ubsan_excluded_cases from every simulator-suite pass. The workflow
passes it for both UBSan lanes. Those lanes still run the smaller
rocBLAS SGEMM cases, and Release still runs the large one.
@kuhar
kuhar merged commit ce3fa32 into develop Oct 30, 2026
171 of 176 checks passed
@kuhar
kuhar deleted the users/newling/rocjitsu-guitar-hero-ci-fixes branch October 30, 2026 21:51
@github-actions

Copy link
Copy Markdown
Contributor

rocjitsu post-commit fuzz passed

The 32-worker multiplexer campaigns for commit ce3fa320bd13 completed without recording a crash or hang.

  • ASan+UBSan (6-minute): 631,655 input executions (1,790.10 inputs/s)
  • TSan (6-minute): 534,516 input executions (1,524.23 inputs/s)

View the workflow run.

@systems-assistant

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

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm project: rocjitsu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants