test: re-enable hipFFT and rocFFT callback tests - #500
Open
mapatel-amd wants to merge 3 commits into
Open
Conversation
These two tests were skipped because rocFFT's default store callback was a
static device function, and HIP CLR could not resolve the resulting
`.static.<hash>` mangled symbol through hipModuleGetGlobal:
Cannot create GlobalVar Obj for symbol:
_ZL31store_cb_default_complex_double.static.4d22c59b5a98cebc
ROCm/rocm-systems#7263 was fixed by ROCm/rocm-libraries@004a100615a
("rocFFT: give default FFT callbacks external linkage to fix
hipModuleGlobal", 2026-07-10) and closed the same day. The default
callbacks are now declared `inline __device__` rather than static, so the
symbol has external linkage and resolves. The fix predates the current
nightly by several weeks.
Drop both manifest entries so the tests run again under ctest and
`make test`.
Co-Authored-By: Claude <noreply@anthropic.com>
The build workflows filter on paths and listed .github/workflows/** but not .github/build_tools/**, so changes to the CI tooling did not start a build. That tooling decides what gets built and tested: skip_manifest.py and generate_skip_tests.py produce the ctest exclude list and the make skip variables, and configure_ci.py builds the job matrix. A pull request that only edits the skip manifest therefore merges with no build or test signal at all. This one is an example: re-enabling the hipFFT and rocFFT callback tests touches only skip_manifest.py, so it ran the linter and nothing else, and had to be dispatched by hand to get coverage. Add .github/build_tools/** to the push and pull_request path filters in both workflows so changes to the skip machinery are exercised by the same matrix they control. Co-Authored-By: Claude <noreply@anthropic.com>
regan-amd
reviewed
Aug 18, 2026
regan-amd
left a comment
Contributor
There was a problem hiding this comment.
ROCm/rocm-libraries@004a100615a is expected to resolve the issue that motivated this test to be disabled in the first place, indeed.
However, the function-pointer callback feature that these samples illustrate are expected to fail on gfx1250 (known issue). Ideally, the re-enabled tests should be skipped/disabled/not tested on gfx1250, if possible.
Collaborator
|
We don't run gfx1250 in our CI workflows so if it works for other archs then it should be fine to reenable them. |
regan-amd
approved these changes
Aug 19, 2026
zichguan-amd
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-enables the
hipfft_callbackandrocfft_callbacktests by removing theirentries from
.github/build_tools/skip_manifest.py.Why they were skipped
rocFFT's default store callback was a static device function, so the compiler
appended a
.static.<hash>suffix to its mangled name. HIP CLR could not resolvethat symbol through
hipModuleGetGlobal, and both examples aborted at runtime:Tracked as
ROCm/rocm-systems#7263.
Why it is safe to re-enable
The issue was fixed by [ROCm/rocm-libraries@004a100615a](https://github.com/ROCm/r
ocm-libraries/commit/004a100615a) — "rocFFT: give default FFT callbacks external
linkage to fix hipModuleGlobal (#8477)", merged 2026-07-10; #7263 was closed
the same day.
The default callbacks are now declared with external linkage rather than static:
No
.static.<hash>suffix, sohipModuleGetGlobalresolves the symbol. Thecommit is an ancestor of
developand predates the current nightly package byroughly five weeks, so the fix is present in what CI tests against.
Changes
Removed both entries from
SKIP_MANIFEST(15 lines, one file). The examplesthemselves are unchanged.
Verification
Generator output for
--channel nightly --target gfx1100 --distro ubuntu-24.04 --install-method tarball-multi-arch:skip_tests.txtSKIP_FROM_TESThipfft_callback,rocfft_callbackAll other manifest entries (rocDecode and the rest) are unaffected.
Empty-exclude-list edge case: these were the only entries carrying a
ctestkey, so
skip_tests.txtis now empty while the workflow still passes--exclude-from-fileunconditionally. Both halves check out:generate_skip_tests.pyopens the file outside theif skip_tests:guard, soit is still created, just zero bytes.
ctest --exclude-from-file <empty file>runs normally and exits 0 (verified onctest 3.28.3).
Not verified on hardware
Every ROCm SDK available locally predates the fix (newest
7.15.0a20260702; thefix landed 2026-07-10), so the examples could not be run directly. CI on the next
nightly is the real check.
If either test still aborts with the
GlobalVar Objerror, the right response isto reopen
ROCm/rocm-systems#7263 with
the new log rather than silently restoring the skip — the manifest entry would
otherwise hide a regression in a fix that is supposed to have shipped.