Repository navigation
[hipfftw tests] Use reference_fft_data in hipfftw tests + avoid H2H copies for host-residing results - #7189
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (76.92%) 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 #7189 +/- ##
========================================
Coverage 64.48% 64.49%
========================================
Files 2656 2656
Lines 419138 419138
Branches 62289 62289
========================================
+ Hits 270279 270282 +3
+ Misses 128384 128372 -12
- Partials 20475 20484 +9
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
from
May 8, 2026 03:23
ee5f227 to
d342e2e
Compare
regan-amd
marked this pull request as ready for review
May 8, 2026 17:27
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
2 times, most recently
from
May 28, 2026 00:23
58af0a6 to
a090b7e
Compare
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
2 times, most recently
from
June 4, 2026 20:17
fc8c180 to
72fd154
Compare
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
from
June 11, 2026 15:30
f541fbd to
3b17db3
Compare
evetsso
approved these changes
Jun 23, 2026
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
from
June 23, 2026 20:01
3b17db3 to
b8c1270
Compare
…opies for host-residing results
…llocation to avoid even more copies.
regan-amd
force-pushed
the
users/regan-amd/reference_in_hipfftw_tests
branch
from
July 7, 2026 21:27
c1fb677 to
869f43f
Compare
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
regan-amd
enabled auto-merge (squash)
July 8, 2026 21:51
1 task done
regan-amd
added a commit
that referenced
this pull request
Aug 8, 2026
…quired (#10494) ## Motivation JIRA ID: ROCM-28393 Fix a possible crash (access violation) in hipfftw functional validation tests, due to test-side bugs introduced in #7189. A crash occurs non-deterministically when consecutive tests share the same FFT configuration (allowing cache reuse), and manifests as a NULL-plan dereference inside by `fftw_execute_dft`. Copy operations from cached reference-results to test buffers were also considering an incorrect (possible too-large) batch size. ## Technical Details `hipfftw_functional_validation::SetUp` unconditionally called `initialize_input` and `launch_async_compute` on the `reference_fft_data_t` object, even when its constructor successfully picked up cached results from a previous test. Since `execute_cpu_fft` destroys the FFTW plan after use, re-launching the async compute on cached data passes a NULL plan to `fftw_execute_dft`, causing the access violation. The fix guards both calls behind `needs_input_initialization()` and `needs_computing()`, skipping re-computation when the cache already holds valid results for the test instance of interest. ## Test Plan Current test base suffice. Manually tested on configurations found to fail/crash. ## Test Result Tests pass. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
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.
Motivation & technical details
The
reference_fft_datastructure introduced in #5590 can be used as is in hipfftw tests, simplifying and unifying logic across functional tests and alleviating some code duplications. This PR suggests to do that. Reference FFTW calculations systematically use default out-of-place data layouts as a result of this change. This PR also alleviates some unnecessary copies of host-residing data, upon completion of hipfftw plan execution in tests.This results in a noticeable speedup for the hipfftw functional changes (about twice as fast on a gfx90a machine).
Test Plan
Current tests suffice.
Test Result
Tests pass.
Submission Checklist