Skip to content

fix(hipblaslt): refuse the unsupported epilogue aux scale - #13522

Merged
tony-davis merged 3 commits into
developfrom
users/todavis_amdeng/ROCM-32138-refuse-aux-scale
Oct 14, 2026
Merged

tony-davis merged 3 commits into
developfrom
users/todavis_amdeng/ROCM-32138-refuse-aux-scale

Conversation

@tony-davis

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-32138

Targets develop. It removes the quarantine that the fast_check stack (#13021, AIHPBLAS-5018, now merged) added for this defect.

Motivation

HIPBLASLT_MATMUL_DESC_EPILOGUE_AUX_SCALE_POINTER (and GemmInputs::setScaleAux in the extension API) let a caller set a scale for the epilogue's auxiliary output, E. No kernel applies it. The library stored the pointer, passed it down to TensileLite, and returned success, so E came out unscaled and the caller had no way to know. Implementing the scale would mean new kernel arguments and codegen across every epilogue that writes E. I think refusing it is the right fix until someone needs it.

Technical Details

  • hipblasLtMatmulDescSetAttribute returns HIPBLAS_STATUS_NOT_SUPPORTED, with a log message, for a non-null aux scale pointer. Setting it to null still succeeds. The check is in hipblaslt.cpp, because rocblaslt has no status that maps to NOT_SUPPORTED; the file already returns it directly elsewhere.
  • In the extension API, GemmInputs::setScaleAux returns void, so Gemm::setProblem and GroupedGemm::setProblem refuse a non-null aux scale instead, the same way. The shorter setProblem overloads forward to these two, and the device UserArguments struct has no aux scale field, so these are the only ways in.
  • The doc comments in hipblaslt.h and hipblaslt-ext.hpp now say the aux scale is not supported and what setting it returns.
  • matmul_fast_check_aux_scale_e_ROCM32138 comes out of matmul_gtest.yaml and known_bugs.yaml. It set the aux scale, which now fails at the set call, so its premise is gone. It was the only YAML case with scaleE: 1, and no bench flag sets it. The client's scaleE plumbing in testing_matmul.hpp stays, unused.
  • The new regression test is AlgoErrors.smoke_AuxScaleIsRefused. It checks that the C API refuses a non-null pointer and accepts null, and that both extension setProblem calls refuse one. aux_matmul_set_get_attr expected success for a non-null pointer; it now expects NOT_SUPPORTED, and success for null.

Any caller that sets this attribute, even to a pointer to 1.0, now gets an error where it used to get silently unscaled E. That's on purpose. A follow-up could implement the scale if a user needs it.

Test Plan

On gfx942 and gfx950:

hipblaslt-test --gtest_filter='AlgoErrors.*:*pre_checkin*aux*'

once with this branch's library, and once with the same test binary against a develop library without this change.

Test Result

ROCm 10.2.0 (TheRock 10.2.0a20260928), built from this branch for gfx942 and gfx950:

  • gfx942 (MI300X): 497 tests pass, including the new smoke test and every pre_checkin aux case. gfx950 (MI355X): the same filter runs 209 tests, and all pass.
  • Without this change (same test binary, develop library at 24bdd07), on both: 2 tests fail. The smoke test's C-API set, Gemm::setProblem and GroupedGemm::setProblem each return HIPBLAS_STATUS_SUCCESS where HIPBLAS_STATUS_NOT_SUPPORTED is expected, and pre_checkin_aux_matmul_set_get_attr_f16_r fails at the same set call.
  • A gfx950 (MI350X) run is still queued. Automated GitHub checks are pending.

Submission Checklist

Risk level

Low (2/5): host-side checks on one attribute and one extension input. Callers that set the aux scale now get an error, but they never got a scaled E.

No kernel applies an aux scale, so a non-null
HIPBLASLT_MATMUL_DESC_EPILOGUE_AUX_SCALE_POINTER or GemmInputs::setScaleAux
was silently ignored and E came out unscaled. Refuse it with
HIPBLAS_STATUS_NOT_SUPPORTED and a log message instead; null stays allowed.

Remove the quarantined matmul_fast_check_aux_scale_e_ROCM32138 case, whose
premise is gone; AlgoErrors.smoke_AuxScaleIsRefused is the regression test.

ROCM-32138

Co-authored-by: Cursor <cursoragent@cursor.com>
@therock-pr-bot

therock-pr-bot Bot commented Oct 12, 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 Oct 12, 2026 •

Copy link
Copy Markdown

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

@tony-davis
tony-davis requested a review from newling October 12, 2026 22:49
@tony-davis
tony-davis marked this pull request as ready for review October 12, 2026 22:50
@tony-davis
tony-davis requested review from a team as code owners October 12, 2026 22:50
@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.64865% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...cts/hipblaslt/library/src/amd_detail/hipblaslt.cpp 94.74% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13522      +/-   ##
===========================================
- Coverage    74.64%   71.99%   -2.65%     
===========================================
  Files         2881     2879       -2     
  Lines       480122   478672    -1450     
  Branches     71209    70933     -276     
===========================================
- Hits        358358   344611   -13747     
- Misses       97497   110041   +12544     
+ Partials     24267    24020     -247     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 49.67% <ø> (-0.03%) ⬇️ Carriedforward from ca2f6fd
TensileLite-Unit 83.55% <ø> (-0.01%) ⬇️ Carriedforward from ca2f6fd
hipBLAS 90.62% <ø> (ø) Carriedforward from ca2f6fd
hipBLASLt 43.59% <98.65%> (+0.29%) ⬆️
hipCUB 82.68% <ø> (ø) Carriedforward from ca2f6fd
hipDNN 86.80% <ø> (-<0.01%) ⬇️ Carriedforward from ca2f6fd
hipFFT 42.35% <ø> (+0.35%) ⬆️ Carriedforward from ca2f6fd
hipRAND 76.12% <ø> (ø) Carriedforward from ca2f6fd
hipSOLVER 68.96% <ø> (-0.56%) ⬇️ Carriedforward from ca2f6fd
hipSPARSE 86.99% <ø> (ø) Carriedforward from ca2f6fd
rocBLAS 48.21% <ø> (-20.52%) ⬇️ Carriedforward from ca2f6fd
rocFFT 47.25% <ø> (+1.55%) ⬆️ Carriedforward from ca2f6fd
rocRAND 57.59% <ø> (+0.33%) ⬆️ Carriedforward from ca2f6fd
rocSOLVER 76.83% <ø> (ø) Carriedforward from ca2f6fd
rocSPARSE 74.31% <ø> (-2.74%) ⬇️ Carriedforward from ca2f6fd
rocThrust 93.15% <ø> (ø) Carriedforward from ca2f6fd

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...blaslt/library/include/hipblaslt/hipblaslt-ext.hpp 0.00% <ø> (ø)
...ts/hipblaslt/library/include/hipblaslt/hipblaslt.h 50.00% <ø> (ø)
...hipblaslt/library/src/amd_detail/hipblaslt-ext.cpp 65.88% <100.00%> (+3.50%) ⬆️
...cts/hipblaslt/library/src/amd_detail/hipblaslt.cpp 68.76% <94.74%> (+0.87%) ⬆️

... and 415 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.

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

LGTM, just one nit about error messages

tony-davis and others added 2 commits October 14, 2026 10:42
Resolve known_bugs.yaml and matmul_gtest.yaml: develop already removed the
ROCM-32062, ROCM-32139 and ROCM-32277 entries, and this PR removes the
ROCM-32138 entry and its quarantined case.

Co-authored-by: Cursor <cursoragent@cursor.com>
Include the sizes, ops, types and the pointer in the error, and name the
call site (Gemm, GroupedGemm, MatmulDesc) so each message is unique.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tony-davis
tony-davis enabled auto-merge (squash) October 14, 2026 15:22
@tony-davis
tony-davis merged commit a0dbbe6 into develop Oct 14, 2026
137 of 144 checks passed
@tony-davis
tony-davis deleted the users/todavis_amdeng/ROCM-32138-refuse-aux-scale branch October 14, 2026 23:32
@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.

2 participants