Skip to content

fix(rocfft): fix inner-batched fused-Bluestein memory access faults at runtime - #8571

Closed
Abuudiii wants to merge 17 commits into
developfrom
users/Abuudiii/gpu-mem-access-fault
Closed

Abuudiii wants to merge 17 commits into
developfrom
users/Abuudiii/gpu-mem-access-fault

Conversation

@Abuudiii

@Abuudiii Abuudiii commented Jun 17, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

JIRA ID : AIFFT-13

Some inner-batched complex 1D FFTs on the Bluestein path were crashing the GPU at runtime with a memory access fault, and returned wrong results once the crash was avoided. The fused Bluestein path assumes the batch is the slowest-moving dimension (batch distance ≥ FFT strides). Inner-batched layouts violate that, the batch is interleaved inside each transform, so the fused path can't represent the padded per-batch convolution distance, and both faults and miscomputes result.

Technical Details

  • tree_node_bluestein.cpp (DecideBlueType): Detect inner-batched layouts (a batch distance smaller than an FFT stride) and route them to the non-fused multi-kernel Bluestein path instead of the fused path. The non-fused path already handles these layouts correctly, so this fixes both the crash and the numerics.

  • The detection compares each FFT stride against the batch distance, so it also covers inner-batched layouts where the distance is greater than 1 but still smaller than a stride (e.g. istride=4, idist=2), which a simple idist==1 check would miss.

Test Plan

rocfft-test --gtest_filter=manual.vs_fftw -g 1 across inner-batched Bluestein cases (prime/large lengths × batch sizes × placements), an istride=4 idist=2 case, and non-inner-batched baselines for regressions.

Test Result

  • No GPU memory access faults across the sweep.
  • Inner-batched cases now match the FFTW reference (verified len 4097/4128/4167/4182 at various batches, plus istride=4/idist=2).
  • Non-inner-batched FFTs and normal Bluestein unchanged, no regressions.

Submission Checklist

@Abuudiii Abuudiii self-assigned this Jun 17, 2026
@Abuudiii
Abuudiii requested a review from a team as a code owner June 17, 2026 22:55
@Abuudiii
Abuudiii marked this pull request as draft June 17, 2026 22:55
@codecov-commenter

codecov-commenter commented Jun 17, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
projects/rocfft/shared/arithmetic.h 18.18% 8 Missing and 1 partial ⚠️
...rojects/rocfft/library/src/tree_node_bluestein.cpp 25.00% 1 Missing and 2 partials ⚠️

❌ Your project check has failed because the head coverage (49.40%) 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    #8571       +/-   ##
============================================
- Coverage    70.64%   69.69%    -0.95%     
============================================
  Files         2939     2524      -415     
  Lines       483952   362315   -121637     
  Branches     69768    50845    -18923     
============================================
- Hits        341874   252515    -89359     
+ Misses      118213    91187    -27026     
+ Partials     23865    18613     -5252     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP ?
TensileLite-Unit ?
hipBLAS 90.81% <ø> (+0.19%) ⬆️ Carriedforward from 1daf2ac
hipBLASLt 34.71% <ø> (-0.88%) ⬇️ Carriedforward from 1daf2ac
hipCUB 82.68% <ø> (ø) Carriedforward from 1daf2ac
hipDNN 85.90% <ø> (-0.79%) ⬇️ Carriedforward from 1daf2ac
hipFFT 50.17% <ø> (-4.15%) ⬇️ Carriedforward from 1daf2ac
hipRAND 76.12% <ø> (ø) Carriedforward from 1daf2ac
hipSOLVER 69.18% <ø> (+0.22%) ⬆️ Carriedforward from 1daf2ac
hipSPARSE 86.55% <ø> (-0.44%) ⬇️ Carriedforward from 1daf2ac
rocBLAS 48.06% <ø> (-0.19%) ⬇️ Carriedforward from 1daf2ac
rocFFT 49.40% <20.00%> (+2.29%) ⬆️
rocRAND 57.04% <ø> (-0.38%) ⬇️ Carriedforward from 1daf2ac
rocSOLVER 76.92% <ø> (+0.09%) ⬆️ Carriedforward from 1daf2ac
rocSPARSE 72.38% <ø> (-2.38%) ⬇️ Carriedforward from 1daf2ac
rocThrust 91.36% <ø> (+13.02%) ⬆️ Carriedforward from 1daf2ac

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

Files with missing lines Coverage Δ
...rojects/rocfft/library/src/tree_node_bluestein.cpp 88.44% <25.00%> (-0.21%) ⬇️
projects/rocfft/shared/arithmetic.h 35.59% <18.18%> (-3.99%) ⬇️

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

@Abuudiii
Abuudiii marked this pull request as ready for review June 25, 2026 20:58
@Abuudiii Abuudiii changed the title rocfft: fix inner-batched fused-Bluestein memory access faults at runtime (WIP) rocfft: fix inner-batched fused-Bluestein memory access faults at runtime Jun 25, 2026
@Abuudiii Abuudiii changed the title (WIP) rocfft: fix inner-batched fused-Bluestein memory access faults at runtime rocfft: fix inner-batched fused-Bluestein memory access faults at runtime Jun 29, 2026
@Abuudiii Abuudiii changed the title rocfft: fix inner-batched fused-Bluestein memory access faults at runtime fix(rocfft): fix inner-batched fused-Bluestein memory access faults at runtime Jun 29, 2026
@therock-pr-bot

therock-pr-bot Bot commented Jul 6, 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

therock-pr-bot Bot commented Jul 6, 2026 •

Copy link
Copy Markdown

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

@malcolmroberts

Copy link
Copy Markdown
Contributor

It would be very helpful for this PR if there was a check in the fused Bluestein plan creation path which checked whether the assumptions for creating this plan were not met. That way, if the check added in this PR is removed, then something fails at plan generation time, and then we have an indication of where to fix it.

@Abuudiii
Abuudiii requested a review from malcolmroberts July 16, 2026 16:07
@github-actions

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 Stale PR has no activity for 25+ days and removed Stale PR has no activity for 25+ days labels Aug 19, 2026
@Abuudiii
Abuudiii marked this pull request as draft August 24, 2026 16:48
@Abuudiii
Abuudiii marked this pull request as ready for review August 27, 2026 21:46
@github-actions

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 Stale PR has no activity for 25+ days and removed Stale PR has no activity for 25+ days labels Sep 23, 2026
@Abuudiii
Abuudiii requested a review from a team September 29, 2026 19:46
@regan-amd

Copy link
Copy Markdown
Contributor

#13020 fixes this issue without adding technical debt. My suggestion is to prefer the former solution.

@Abuudiii Abuudiii closed this Oct 5, 2026
@Abuudiii Abuudiii reopened this Oct 6, 2026
@JeniferC99

Copy link
Copy Markdown
Contributor

Cancelling the remaining gfx950 jobs on this run so the TheRock submodule bump PRs can be prioritized on the gfx950 queue.

This change does not target gfx950, and gfx950 was the only work still outstanding. Jobs that already finished are unchanged. Re-run failed jobs if you need gfx950 results before merge.

@regan-amd

Copy link
Copy Markdown
Contributor

#13020 fixed the issue

@regan-amd regan-amd closed this Oct 12, 2026
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.

5 participants