Skip to content

fix(rocfft): fall back to gather-scatter when opt multi-device build throws - #11996

Merged
regan-amd merged 2 commits into
developfrom
users/regan-amd/duct_tape_mgpu
Sep 14, 2026
Merged

regan-amd merged 2 commits into
developfrom
users/regan-amd/duct_tape_mgpu

Conversation

@regan-amd

Copy link
Copy Markdown
Contributor

Motivation

BuildOptMultiDevicePlan could let an exception escape to rocfft_plan_create_internal (e.g. get_most_balanced_proc_pair failing to factor an unbalanceable pencil grid), which returned rocfft_status_failure instead of falling through to the single-device plan path.

JIRA ID: AIFFT-539

Technical Details

  • Give it the same function-try-block exception handling that BuildMultiDevicePlan already had, so it logs the exception, discards any partial plan state, and returns false to let plan creation fall back as intended.
  • Factor the shared handler into rocfft_plan_t::discard_failed_multi_device_plan and reuse it from both functions to align the two implementations and avoid duplication.

Test Plan

Added a multi-GPU ad-hoc regression token that exercises the throw.

Test Result

Tests pass for the added ad-hoc token on a system with 8 gfx90a (plan creation failed prior).

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 11, 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 Sep 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 101 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
projects/rocfft/library/src/plan.cpp 0.00% 101 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11996      +/-   ##
===========================================
+ Coverage    70.23%   70.25%   +0.03%     
===========================================
  Files         2810     2810              
  Lines       462535   462543       +8     
  Branches     68105    68104       -1     
===========================================
+ Hits        324828   324951     +123     
+ Misses      114187   114091      -96     
+ Partials     23520    23501      -19     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 46.40% <ø> (ø) Carriedforward from 2e146c4
TensileLite-Unit 76.07% <ø> (ø) Carriedforward from 2e146c4
hipBLAS 90.62% <ø> (ø) Carriedforward from 2e146c4
hipBLASLt 35.27% <ø> (ø) Carriedforward from 2e146c4
hipCUB 82.68% <ø> (ø) Carriedforward from 2e146c4
hipDNN 87.02% <ø> (ø) Carriedforward from 2e146c4
hipFFT 43.07% <ø> (ø) Carriedforward from 2e146c4
hipRAND 76.12% <ø> (ø) Carriedforward from 2e146c4
hipSOLVER 68.92% <ø> (ø) Carriedforward from 2e146c4
hipSPARSE 86.99% <ø> (ø) Carriedforward from 2e146c4
rocBLAS 48.31% <ø> (ø) Carriedforward from 2e146c4
rocFFT 47.52% <0.00%> (+0.36%) ⬆️
rocRAND 56.90% <ø> (ø) Carriedforward from 2e146c4
rocSOLVER 76.83% <ø> (ø) Carriedforward from 2e146c4
rocSPARSE 74.61% <ø> (ø) Carriedforward from 2e146c4
rocThrust 91.60% <ø> (ø) Carriedforward from 2e146c4

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

Files with missing lines Coverage Δ
projects/rocfft/library/src/include/plan.h 26.42% <ø> (ø)
projects/rocfft/library/src/plan.cpp 27.98% <0.00%> (-0.03%) ⬇️

... and 30 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 September 12, 2026 00:48
@regan-amd
regan-amd requested review from a team as code owners September 12, 2026 00:48
Base automatically changed from users/regan-amd/fft_bump_post_10_1 to develop September 12, 2026 05:25
…throws

BuildOptMultiDevicePlan could let an exception escape to
rocfft_plan_create_internal (e.g. get_most_balanced_proc_pair failing to
factor an unbalanceable pencil grid), which returned
rocfft_status_failure instead of falling through to the single-device
plan path.

Give it the same function-try-block exception handling that
BuildMultiDevicePlan already had, so it logs the exception, discards any
partial plan state, and returns false to let plan creation fall back as
intended. Factor the shared handler into
rocfft_plan_t::discard_failed_multi_device_plan and reuse it from both
functions to align the two implementations and remove duplication.

Add a multi-GPU adhoc regression token that exercises the throw, plus a
changelog entry.
…throw invariant

Remove the duplicated doc comment on discard_failed_multi_device_plan (kept on the header declaration) and document that BuildOptMultiDevicePlan must throw rather than return false past the first plan-state mutation, since BuildMultiDevicePlan no longer clears partial state on entry.
@regan-amd
regan-amd force-pushed the users/regan-amd/duct_tape_mgpu branch from 236148c to 844940d Compare September 12, 2026 05:25
@regan-amd
regan-amd merged commit 45b014a into develop Sep 14, 2026
113 of 115 checks passed
@regan-amd
regan-amd deleted the users/regan-amd/duct_tape_mgpu branch September 14, 2026 17:40
@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.

2 participants