Skip to content

feat(hipfft): add hipfftXtSetWorkArea for multi-device external work buffers - #11434

Merged
regan-amd merged 6 commits into
developfrom
users/regan-amd/hipfft_ext_workareas
Sep 3, 2026
Merged

regan-amd merged 6 commits into
developfrom
users/regan-amd/hipfft_ext_workareas

Conversation

@regan-amd

Copy link
Copy Markdown
Contributor

Motivation

Multi-device hipFFT plans previously had no way to use externally-managed work areas. The only option was auto-allocation, which doesn't allow the caller to control VRAM usage or reuse buffers across plans. The cuFFT API provides cufftXtSetWorkArea for this purpose; hipFFT now matches it.

JIRA ID: AIFFT-523

Technical Details

  • Add hipfftXtSetWorkArea(hipfftHandle, void**) to the public API (AMD and NVIDIA backends), allowing users to supply per-device work buffers for multi-device plans.
  • hipfftSetWorkArea now delegates to hipfftXtSetWorkArea and rejects single-process, multi-device plans explicitly.
  • Refactor hipfftGetSize*/hipfftEstimate* to use a shared hipfftHandle_t::make_size_querying_plan_from() helper, eliminating duplicated create/destroy boilerplate and ensuring multi-device device contexts are propagated to size queries.
  • Fix multi-GPU workspace size estimation: hipfftGetSize now writes one size per device (previously only the first entry was populated).
  • Extract hipfftHandle_wrapper_t and hipfftLibXtDesc_wrapper_t RAII typedefs into a shared header (hipfft_object_wrapper.h) used by both the library and test clients.
  • Enable the previously-disabled various_multi_gpu test suite with auto-allocation disabled in single-process, multi-device tests.

Test Plan

  • hipfftxt_test.cpp: Updated make_plan to disable auto-allocation and attach work buffers from a grow-to-max pool via hipfftXtSetWorkArea. This exercises the new API on every parameterized multi-device test case.
  • multi_device_test.cpp: Enabled various_multi_gpu suite (previously DISABLED_) which tests library-decided splits with auto-allocation disabled.
  • Existing single-device tests remain unchanged and validate no regression on the hipfftSetWorkArea path.

Test Result

CI tests pass.
The changes were also verified manually using

  • 8x gfx90a (rocFFT backend)
  • 8x V100 (cuFFT backend)
  • 4x H100 (cuFFT backend)

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Aug 28, 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

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.81081% with 29 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
projects/hipfft/library/src/amd_detail/hipfft.cpp 63.38% 17 Missing and 9 partials ⚠️
projects/hipfft/shared/hipfft_object_wrapper.h 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11434      +/-   ##
===========================================
+ Coverage    63.78%   64.49%   +0.71%     
===========================================
  Files         2825     2826       +1     
  Lines       469112   469096      -16     
  Branches     69265    69252      -13     
===========================================
+ Hits        299193   302505    +3312     
+ Misses      148872   145505    -3367     
- Partials     21047    21086      +39     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 39.04% <ø> (ø) Carriedforward from e72781d
TensileLite-Unit 75.53% <ø> (ø) Carriedforward from e72781d
hipBLAS 90.62% <ø> (ø) Carriedforward from e72781d
hipBLASLt 35.22% <ø> (-<0.01%) ⬇️ Carriedforward from e72781d
hipCUB 82.68% <ø> (ø) Carriedforward from e72781d
hipDNN 86.95% <ø> (ø) Carriedforward from e72781d
hipFFT 43.38% <60.81%> (+0.12%) ⬆️
hipRAND 76.12% <ø> (ø) Carriedforward from e72781d
hipSOLVER 68.92% <ø> (ø) Carriedforward from e72781d
hipSPARSE 86.99% <ø> (ø) Carriedforward from e72781d
rocBLAS 0.28% <ø> (ø) Carriedforward from e72781d
rocFFT 55.95% <ø> (+10.03%) ⬆️
rocRAND 56.91% <ø> (ø) Carriedforward from e72781d
rocSOLVER 76.83% <ø> (ø) Carriedforward from e72781d
rocSPARSE 74.61% <ø> (ø) Carriedforward from e72781d
rocThrust 91.60% <ø> (ø) Carriedforward from e72781d

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

Files with missing lines Coverage Δ
projects/hipfft/library/include/hipfft/hipfft.h 100.00% <ø> (ø)
projects/hipfft/shared/hipfft_object_wrapper.h 0.00% <0.00%> (ø)
projects/hipfft/library/src/amd_detail/hipfft.cpp 34.77% <63.38%> (+0.51%) ⬆️

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

@regan-amd
regan-amd marked this pull request as ready for review August 29, 2026 16:06
@regan-amd
regan-amd requested review from a team as code owners August 29, 2026 16:06

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

Looks like we're going to conflict. :(

@regan-amd

Copy link
Copy Markdown
Contributor Author

Looks like we're going to conflict. :(

Yeah, I'll rebase after #10875 is merged, it shouldn't be too hard to resolve the conflicts

@malcolmroberts
malcolmroberts self-requested a review September 2, 2026 16:45
Add hipfftXtSetWorkArea API for setting externally-managed work areas
on single-process, multi-device plans.  The pre-existing hipfftSetWorkArea
now delegates to it, rejecting SPMD plans.

Fix hipfftGetSize* work area estimates to account for the plan's
pre-initialization state (device contexts, scale factor, communicator)
via a new make_size_querying_plan_from helper that copies that state
into a temporary plan, instead of estimating from a bare plan.

Document hipfftSetWorkArea, hipfftXtSetWorkArea, and hipfftGetSize
in the RST API reference (previously missing).
Disable auto-allocation in make_plan and allocate work buffers
explicitly via gpubuf (which honors VRAM limits).  Buffers are
attached to plans via hipfftXtSetWorkArea.

A static grow-to-max pool keyed by device ID
(spmd_hipfft_params::work_pool) avoids redundant allocations
across test instances; TearDownTestSuite clears it.
…-GPU

Allocate each work area buffer on its corresponding GPU by setting the
device via rocfft_scoped_device before calling alloc(). Previously all
buffers were allocated on whichever device happened to be active.

Also remove the redundant buf.free() call since alloc() already frees
internally.
@regan-amd
regan-amd force-pushed the users/regan-amd/hipfft_ext_workareas branch from 8f50528 to a0a1e24 Compare September 2, 2026 18:18
@regan-amd
regan-amd enabled auto-merge (squash) September 2, 2026 18:18
@regan-amd
regan-amd merged commit 80bf655 into develop Sep 3, 2026
107 of 110 checks passed
@regan-amd
regan-amd deleted the users/regan-amd/hipfft_ext_workareas branch September 3, 2026 06:17
@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.

4 participants