Repository navigation
fix(hipblaslt): pin gfx942 build-only test to its target arch - #12086
Conversation
…arch test_compile (and test_use_cache) in test_build_only.py hardcode ISA gfx942 in their embedded config and are meant to build regardless of host architecture, but tensile_args forwards any --gpu-targets set by the harness (e.g. tox injects --gpu-targets gfx1250v0 on gfx1250v0 hosts), which takes priority over the config's own ISA in Tensile.py. On such hosts the test silently compiled for gfx1250v0 instead of gfx942, and gfx1250's WMMA v3 ISA doesn't support the config's (16,16,16,1) MFMA-only instruction shape, causing an assertion failure unrelated to the actual gfx1250 kernel work under test. Pin --gpu-targets gfx942 explicitly after *tensile_args so it always wins, regardless of what the harness forwards. Fixes #12083 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (46.40%) 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 #12086 +/- ##
===========================================
- Coverage 70.55% 70.55% -0.00%
===========================================
Files 2810 2810
Lines 462535 462575 +40
Branches 68105 68113 +8
===========================================
+ Hits 326312 326328 +16
- Misses 112680 112699 +19
- Partials 23543 23548 +5
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
tony-davis
left a comment
There was a problem hiding this comment.
Nice fix, and a genuinely tricky one to track down. I checked the --gpu-targets precedence claim directly in Tensile.py: it's a store action, so the flag you appended after *tensile_args does win over anything the harness forwards. That matches the behavior you described.
A couple of things I dug into while reviewing:
- I looked at the currently-failing checks on this PR.
FFM gfx1250/HW gfx1250are the gfx1250 bring-up lane, not required, so I didn't weigh those. mci/rocm-libraries/precheckin(hipblaslt)is also red right now, but I pulled the actual Jenkins log for the failing stage (Test ubuntu24-therock && gfx950). It's a GPU memory fault (HSA_STATUS_ERROR_MEMORY_FAULT, core dump) insidehipblaslt-teston gfx950, in an unrelatedmatmul_testcase. Nothing to do with this diff, so I'm treating it as a pre-existing gfx950 hardware flake.
Also spot-checked that your fix doesn't break legitimate --gpu-targets forwarding elsewhere (the tdmpluslds_tdm_gfx1250.yaml check in your test plan). Good catch adding that.
One tiny, non-blocking thought for later: the docstring explains why the pin is needed but not that --gpu-targets is last-wins on the CLI, which is the fact that makes this fix reliable rather than incidental. Not asking for a change, just flagging it in case it's useful context for the next person who touches this file.
|
LGTM. Failing FFM and HW tests are unrelated. |
TheRock Submodule Bump ActivityNewest first
|
JIRA ID : AIHPBLAS-4510 ## Motivation - 4 gfx1250 TensileLite `common` configs fail on the current gfx1250 HW. - xfail them so the per-PR and nightly gfx1250 lanes stay green while the gaps are tracked. - The separate gfx942 `test_compile` failure is fixed in #12086 ## Technical Details - Mark the currently-failing gfx1250 configs `xfail-gfx1250` - Fix `findAvailableArchs` (`config_helpers.py`) to strip a trailing version suffix arch — without this the marks never fire on a versioned runner and the configs stay red. - FFM path uses HSA_MODEL_MEMFILE to skip any HW specific `xfail-<arch>`, as these tests failing on HW pass on FFM. - Add 3 tests in `Tensile/Tests/common/test_config_helpers.py` covering the arch-suffix strip and the FFM-gate suppression. ## Test Plan - 3 new common tests in `test_config_helpers.py` - FFM tests pass as expected - Full gfx1250 HW xfail behavior is exercised once #12164 's workflow runs post-merge against a rebuilt artifact. ## Test Result - [x] `pytest Tensile/Tests/common/test_config_helpers.py` → 7 passed (4 existing + 3 new). - [x] FFM tests pass as expected - https://github.com/ROCm/rocm-libraries/pull/12178/checks?check_run_id=105181618286 - gfx1250 HW result pending the post-merge #12164 run. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. ## Risk level - Low — only adds xfail marks to already-failing gfx1250 configs plus a one-line arch-normalization with regression tests; no change to any passing test. `xfail_strict` turns an unexpected pass red, so a fixed config can't stay silently masked. --------- Co-authored-by: Tony Davis <tony.davis@amd.com>
JIRA ID : AIHPBLAS-4748
Motivation
Fixes #12083.
Tensile/Tests/common/flags/gfx942/test_build_only.py::test_compilefails ongfx1250v0-revision hosts withAssertionError: Unhandled WMMA: (16, 16, 16, 1), even though its embedded config hardcodesISA: [[9,4,2]](gfx942) and the test is documented to run on any machine as a hardware-independent,--build-onlycompile check.Root cause:
tensile_args(Tensile/Tests/conftest.py) forwards pytest's--gpu-targetsoption into every test'sTensile.Tensile(...)invocation when the harness sets one (e.g.tox.iniinjects--gpu-targets gfx1250v0wheneverinvoke get-gpu-revision-targetreportsgfx1250v0).Tensile.pygives CLI--gpu-targetspriority over the config's ownISA:field, so the test's gfx942 target was silently overridden to gfx1250v0. gfx1250's WMMA v3 ISA only supports K-depths of 4/32/64/128 (Tensile/Common/Utilities.py::wmmaV3InputVgprLayout), so the config's(16, 16, 16, 1)MFMA-only instruction shape (valid on gfx942) hit theassert False, f"Unhandled WMMA: {wmma}"branch — a false-positive CI/local-test failure unrelated to the actual gfx1250 kernel work being tested.Technical Details
Pin
--gpu-targets gfx942explicitly in bothtest_compileandtest_use_cache, appended after*tensile_argsin the argument list passed toTensile.Tensile(...).Tensile.py's--gpu-targetsargparse option uses the defaultstoreaction, so the last occurrence on the command line wins — this guarantees the test always builds for gfx942 regardless of what the harness forwards, without touching the legitimate--gpu-targetsforwarding behavior other tests rely on.test_use_cachewas not actually exposed to this bug today (it's guarded by its ownskipif(not has_arch("gfx942")), so it only ever runs on real gfx942 hardware where any forwarded--gpu-targetswould already say gfx942), but it makes the same "always gfx942" claim astest_compile, so it gets the same explicit pin for consistency and to guard against future changes to the skip condition or harness behavior.Test Plan
developvia the standard CI flow:tox -e py3 -- Tensile/Tests -m common --durations=0on a 4-GPUgfx1250/gfx1250v0host, both with the default 4 pytest workers and withTENSILE_NUM_PYTEST_WORKERS=1.--gpu-targets gfx1250v0forwarding tox performs:pytest --prebuilt-client=... --global-parameters CheckASMCodeSize=True --tensile-options=--gpu-targets,gfx1250v0 Tensile/Tests/common/flags/gfx942/test_build_only.pygemm/gfx12/tdmpluslds_tdm_gfx1250.yaml) under the same--gpu-targets gfx1250v0forwarding to confirm the fix doesn't affect tests that rely on that forwarding.Test Result
Before fix:
After fix:
Unrelated gfx1250 config test, same environment, still passes:
Submission Checklist
Risk level
Low. The change is scoped to two test functions in a single test file, only adds an explicit
--gpu-targets gfx942argument (highest-precedence override) to calls that were previously letting that value fall through from the harness; it does not change any non-test code path or other tests' behavior.