Skip to content

fix(hipblaslt): lay out E with lde and stride_e in the CPU reference - #13215

Merged
tony-davis merged 2 commits into
developfrom
users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde
Oct 13, 2026
Merged

tony-davis merged 2 commits into
developfrom
users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde

Conversation

@tony-davis

@tony-davis tony-davis commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-32139

Targets develop. It removes the quarantine that the fast_check stack (#13015, AIHPBLAS-5017, now merged) added for these cases.

Motivation

hipblaslt-test checks the GPU's answer against a reference computed on the CPU. For epilogues with an auxiliary output, E (the pre-activation result some training workloads save for the backward pass), the reference writes its own copy of E, and gradient epilogues read E back.

The library lays E out with its own leading dimension (lde) and batch stride (stride_e). The reference used D's instead: it wrote E at D's position, j * ldd + i, and started each batch's E at D's batch offset. Whenever lde differs from ldd, unit_check compared the GPU's correct E against a reference laid out on the wrong grid, and failed it. This affects the test client only; the library is not involved.

Technical Details

  • epilogue_func (in clients/common/include/testing_matmul.hpp) takes lde and indexes E with j * lde + i, for both the write and the gradient read. D still uses ldd.
  • The caller offsets each batch's E by stride_e instead of stride_d.
  • matmul_fast_check_aux_padded_ldd_ROCM32139 comes out of known_bugs.yaml and becomes the regression test. It now also runs with two batches, which the stride_e change needs, and with a padded lde (140, where M is 131 and ldd is 151). The padded-lde size was covered by a fast_check-only copy while the case was quarantined; that copy is removed.

Test Plan

On gfx942 and gfx950, run the formerly quarantined case and every pre_checkin case with an aux output:

hipblaslt-test --gtest_filter='*ROCM32139*:*pre_checkin*fast_check*aux*:*pre_checkin*_AUX*:*pre_checkin*Aux*-*known_bug*'

once with this branch's client, once with the client rebuilt without the testing_matmul.hpp change, and once with only the stride_e change reverted.

Test Result

ROCm 10.2.0 (TheRock 10.2.0a20260928), built from this branch:

  • gfx942 (MI300A): 1,738 passed, 0 failed. With the fix reverted, the 4 variants of the formerly quarantined case fail (lde 131 and 140, one batch and two).
  • gfx942, with only the stride_e change reverted: the 2 two-batch variants fail and the 2 one-batch variants pass, so each half of the fix has a test that fails without it.
  • gfx950 (MI350X): 170 passed, 0 failed. With the fix reverted, the same 4 variants fail.
  • Math CI precheckin passed against develop.

Submission Checklist

Risk level

Low (1/5): test-client only. The reference changes only when E's leading dimension or batch stride differs from D's, where it was wrong; library and device behavior are unchanged.

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project check has failed because the head coverage (42.73%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13215      +/-   ##
===========================================
- Coverage    71.99%   71.86%   -0.12%     
===========================================
  Files         2879     2876       -3     
  Lines       478104   475289    -2815     
  Branches     70852    70231     -621     
===========================================
- Hits        344165   341563    -2602     
+ Misses      109972   109948      -24     
+ Partials     23967    23778     -189     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 49.67% <ø> (ø) Carriedforward from b56f244
TensileLite-Unit 83.44% <ø> (-0.11%) ⬇️ Carriedforward from b56f244
hipBLAS 90.62% <ø> (ø) Carriedforward from b56f244
hipBLASLt 42.73% <ø> (ø)
hipCUB 82.68% <ø> (ø) Carriedforward from b56f244
hipDNN 86.66% <ø> (-0.15%) ⬇️ Carriedforward from b56f244
hipFFT 42.58% <ø> (+0.23%) ⬆️ Carriedforward from b56f244
hipRAND 76.12% <ø> (ø) Carriedforward from b56f244
hipSOLVER 68.96% <ø> (ø) Carriedforward from b56f244
hipSPARSE 86.99% <ø> (ø) Carriedforward from b56f244
rocBLAS 48.17% <ø> (-0.03%) ⬇️ Carriedforward from b56f244
rocFFT 46.51% <ø> (-0.74%) ⬇️ Carriedforward from b56f244
rocRAND 57.45% <ø> (-0.14%) ⬇️ Carriedforward from b56f244
rocSOLVER 76.83% <ø> (ø) Carriedforward from b56f244
rocSPARSE 74.07% <ø> (-0.24%) ⬇️ Carriedforward from b56f244
rocThrust 93.15% <ø> (ø) Carriedforward from b56f244

*This pull request uses carry forward flags. Click here to find out more.
see 126 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tony-davis tony-davis changed the title fix(hipblaslt): ROCM-32139 lay out E with lde and stride_e in the CPU reference fix(hipblaslt): lay out E with lde and stride_e in the CPU reference Oct 7, 2026
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/AIHPBLAS-5017-side-operands branch 2 times, most recently from dccd0c0 to a007b01 Compare October 7, 2026 20:03
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde branch from 392af76 to 13305ea Compare October 7, 2026 20:24
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/AIHPBLAS-5017-side-operands branch from a007b01 to 519e7a1 Compare October 7, 2026 21:05
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde branch from 13305ea to 3030497 Compare October 7, 2026 21:24
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/AIHPBLAS-5017-side-operands branch 2 times, most recently from 1e40a94 to aa4b65c Compare October 8, 2026 17:03
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde branch from 3030497 to 3e6b887 Compare October 8, 2026 18:08
An error occurred while trying to automatically change base from users/todavis_amdeng/AIHPBLAS-5017-side-operands to users/todavis_amdeng/AIHPBLAS-5016-exactness-large-k October 9, 2026 03:32
… reference

The client CPU reference's epilogue (epilogue_func in testing_matmul.hpp) wrote the aux
output E, and read it back for gradient epilogues, at D's position (j * ldd + i), and
started each batch's E at D's batch offset (stride_d). The library lays E out with lde
and stride_e, so whenever lde differs from ldd, unit_check failed a correct GPU result.

Pass lde to epilogue_func and index E with it, and offset each batch's E by stride_e.

matmul_fast_check_aux_padded_ldd_ROCM32139 comes out of known_bugs.yaml and becomes the
regression test. It now also runs with two batches, which the stride_e change needs, and
with a padded lde, which its fast_check-only copy covered while it was quarantined; that
copy is removed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tony-davis
tony-davis force-pushed the users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde branch from 3e6b887 to b56f244 Compare October 9, 2026 16:16
@tony-davis
tony-davis changed the base branch from users/todavis_amdeng/AIHPBLAS-5017-side-operands to develop October 9, 2026 16:16
@therock-pr-bot

therock-pr-bot Bot commented Oct 9, 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

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

Copy link
Copy Markdown

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

@tony-davis
tony-davis marked this pull request as ready for review October 12, 2026 17:34
@tony-davis
tony-davis requested a review from a team as a code owner October 12, 2026 17:34
@tony-davis
tony-davis requested a review from newling October 12, 2026 17:34
@tony-davis
tony-davis requested a review from talumbau October 12, 2026 19:34

@newling newling 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.

Looks good to me. I left one suggestion about argument names that might be invalid, or better done as a NFC follow-up.

@tony-davis
tony-davis merged commit f6ccd04 into develop Oct 13, 2026
323 of 331 checks passed
@tony-davis
tony-davis deleted the users/todavis_amdeng/ROCM-32139-cpu-ref-e-lde branch October 13, 2026 17:46
tony-davis added a commit that referenced this pull request Oct 13, 2026
Resolve the matmul_gtest.yaml conflict with #13215. Each PR removed its
own fast_check-only copies from the shared block, so with both fixes in,
the whole block comes out.

Co-authored-by: Cursor <cursoragent@cursor.com>
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

tony-davis added a commit that referenced this pull request Oct 14, 2026
Resolve the matmul_gtest.yaml conflict by taking develop's removal of the
matmul_fast_check_only_* cases, which became unnecessary once the client CPU
reference was fixed (#13214, #13215).

Co-authored-by: Cursor <cursoragent@cursor.com>
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