Skip to content

test(rocfft): cover planning decisions, buffer assignment, and twiddle reuse - #11272

Closed
Abuudiii wants to merge 5 commits into
developfrom
users/Abdullah/internal-suite-tests
Closed

Abuudiii wants to merge 5 commits into
developfrom
users/Abdullah/internal-suite-tests

Conversation

@Abuudiii

Copy link
Copy Markdown
Contributor

Motivation

Added more tests to the internal test suite for the solution map reader, the kernel cache, kernel names, stride arithmetic, or planning.

Technical Details

Adds 48 tests:

  • data_layout_test.cpp - stride and offset arithmetic
  • function_map_key_test.cpp - kernel identifiers are unique and stable
  • kernel_name_test.cpp - kernel names are unique and round trip
  • solution_map_test.cpp - tuning files read and write, bad files error out
  • rtc_test.cpp - kernel cache store, get and serialize
  • plan_test.cpp - decomposition, scheme choice, Bluestein selection, transpose fusion, buffer assignment
  • repo_test.cpp - twiddle and chirp tables are reused, not rebuilt

The planning code was not in the test binary. tests/CMakeLists.txt now builds those sources into it and links hip::device. powX.cpp stays out because it carries device code, so rocfft_stub.cpp supplies its three functions behind ROCFFT_BUILD_INTERNAL_TEST. The shipping library is unchanged.

library/src/CMakeLists.txt moves the ROCFFT_SOLUTION_MAP_DIR default above

  • rtc_test.cpp - kernel cache store, get and serialize
  • plan_test.cpp - decomposition, scheme choice, Bluestein selection, transpose fusion, buffer assignment
  • repo_test.cpp - twiddle and chirp tables are reused, not rebuilt

The planning code was not in the test binary. tests/CMakeLists.txt now builds those sources into it and links hip::device. powX.cpp stays out because it carries device code, so rocfft_stub.cpp supplies its three functions behind ROCFFT_BUILD_INTERNAL_TEST. The shipping library is unchanged.

library/src/CMakeLists.txt moves the ROCFFT_SOLUTION_MAP_DIR default above add_subdirectory( tests ) so the tests can find the shipped solution maps.

Three tests failed against existing code, so the fixes are included:

  • data_descriptor.h - the field search had no end check and read past the end of the data when a field was missing. SeekField bounds it and throws. OptionalFieldParser handles fields that may be absent.
  • function_map_key.h - gcn_arch_name is optional now, since the shipped .dat files predate it. New files still include it and still round trip.
  • rtc_cache.cpp - CREATE TABLE in connect_db gets the same try and catch get_stmt and store_stmt already use, so a corrupt cache file is skipped instead of failing rocfft_setup.

Test Plan

Built and ran rocfft-internal-test on MI300X, gfx942, ROCm 7.2.4.

Test Result

48/48 new tests build and pass.

The library and all four stub consumers build and link clean.

Submission Checklist

@Abuudiii Abuudiii self-assigned this Aug 25, 2026
@Abuudiii
Abuudiii requested a review from a team as a code owner August 25, 2026 21:01
@therock-pr-bot

therock-pr-bot Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@Abuudiii
Abuudiii marked this pull request as draft August 25, 2026 21:42
@Abuudiii
Abuudiii marked this pull request as ready for review August 28, 2026 18:45
@malcolmroberts

Copy link
Copy Markdown
Contributor

It sounds like there are several testing objectives for this PR. Would it be possible to separate these into separate PRs?

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

This would probably be better split-up.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Oct 4, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.

@github-actions github-actions Bot closed this Oct 10, 2026
Abuudiii added a commit that referenced this pull request Oct 26, 2026
## Motivation

Split from #11272. `data_layout` stride and offset arithmetic had no
direct tests.

## Technical Details

- `data_layout_test.cpp` - constructor validation, packed default
strides, offsets, in-place real/complex layouts and axis ordering
- `shared/ptrdiff.h` - adds the missing `#include <algorithm>` for
`std::any_of`; it did not build without it once the test compiled
`data_layout.cpp`

## Test Plan

Built this branch alone on develop and ran every `rocfft-internal-test`
case in its own process on MI210 (gfx90a), ROCm 7.1.0.

## Test Result

15/15 new tests pass, 19/19 total.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Abuudiii added a commit that referenced this pull request Oct 29, 2026
…ch_name (#12664)

## Motivation

Split from #11272. Reading a solution map with a missing field walks
past the end of the buffer, and every shipped `.dat` file is missing
`gcn_arch_name`.

## Technical Details

- `data_descriptor.h` - `SeekField` bounds the field search and throws
when the field is absent. A truncated or unterminated list now throws
too. `OptionalFieldParser` handles fields that may be absent.
- `function_map_key.h` - `gcn_arch_name` is optional. New files still
write it and still round trip.
- `solution_map_test.cpp` - shipped maps load, write then read, and
missing, empty or truncated files error out.
- `library/src/CMakeLists.txt` - moves the `ROCFFT_SOLUTION_MAP_DIR`
default above `add_subdirectory( tests )` so the tests find the shipped
maps.

## Test Plan

Built this branch alone on develop and ran every `rocfft-internal-test`
case in its own process on MI210 (gfx90a), ROCm 7.1.0.

## Test Result

5/5 new tests pass, 9/9 total. Without the fix,
`shipped_solution_maps_are_readable` and `solution_map_truncated_file`
segfault.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
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