Skip to content

fix(rocfft): fix plan creation for multi-kernel Bluestein with scale factor - #13098

Merged
regan-amd merged 4 commits into
developfrom
users/regan-amd/scale_bluestein_multi_kernel
Oct 7, 2026
Merged

regan-amd merged 4 commits into
developfrom
users/regan-amd/scale_bluestein_multi_kernel

Conversation

@regan-amd

@regan-amd regan-amd commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

JIRA ID: AIFFT-554

rocfft_plan_create fails when a non-unit scale factor is set on a transform whose last kernel writing user output is the multi-kernel Bluestein CS_KERNEL_RES_MUL (e.g., 1D complex transforms of large prime lengths such as 196597 or 1000003). Runtime compilation of that kernel fails with error: redefinition of parameter 'scale_factor'.

Technical Details

  • bluestein_multi_rtc() (rtc_bluestein_gen.cpp) declared its own scale_factor kernel parameter. The kernel never reads it, and RTCKernelBluesteinMulti::get_launch_args never passes it.
  • When a scale factor is set, store ops already append a scale_factor parameter to the store node (StoreOpsVisitor::visit_Function), apply it to every global store, and pass its value at launch (StoreOps::append_args). The kernel therefore ended up with two parameters of the same name.
  • Fix: remove the redundant declaration and the extra parameter. Nothing changes for unscaled plans: the dropped trailing parameter was never supplied or read.

Test Plan

  • Add length 196597 to callback_sizes in accuracy_test_callback.cpp. On gfx1201 it takes the multi-kernel Bluestein path (PAD_MUL ... RES_MUL) in single and double precision. Because the scaling and callback suites share this size list, the change covers:
    • scale factor 7.23 on complex and real transforms, single and double, in-place and out-of-place, interleaved and planar;
    • sampled funcptr and JIT callback cases.

Test Result

On gfx1201 (RX 9070):

  • Without the fix: 5/9 new scaling cases fail (all complex ones) with rocfft_plan_create failed.
  • With the fix: all 11 generated length-196597 cases pass (scaling and callback, --test_prob 1).
  • Manual checks with --scalefactor on length 1000003: all transform types, interleaved and planar, single and double, and JIT callback + scale.

Submission Checklist

Result scaling on 1D multi-kernel Bluestein plans (PAD_MUL ... RES_MUL)
fails in rocfft_plan_create.  Add length 196597, which uses that path
regardless of architecture or precision, to the callback/scaling sizes.
The multi-kernel Bluestein generator declared its own scale_factor
kernel parameter, which the launcher never passes and the kernel never
reads.  When a scale factor is set, store ops append another
scale_factor parameter to the RES_MUL kernel, and runtime compilation
fails with "redefinition of parameter 'scale_factor'".  Remove the
redundant declaration; store ops already add and pass it as needed.
@therock-pr-bot

therock-pr-bot Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocfft/clients/tests/accuracy_test_callback.cpp, projects/rocfft/library/src/rtc_bluestein_gen.cpp; no test file found
🔎 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 Oct 5, 2026 •

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13098      +/-   ##
===========================================
- Coverage    71.93%   70.92%   -1.01%     
===========================================
  Files         2863     2944      +81     
  Lines       472701   485345   +12644     
  Branches     69827    70075     +248     
===========================================
+ Hits        340000   344209    +4209     
- Misses      109118   117236    +8118     
- Partials     23583    23900     +317     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 49.02% <ø> (-0.31%) ⬇️ Carriedforward from 85b776c
TensileLite-Unit 83.28% <ø> (-0.01%) ⬇️ Carriedforward from 85b776c
hipBLAS 90.62% <ø> (ø) Carriedforward from 85b776c
hipBLASLt 38.65% <ø> (-0.26%) ⬇️ Carriedforward from 85b776c
hipCUB 82.68% <ø> (ø) Carriedforward from 85b776c
hipDNN 86.64% <ø> (+0.01%) ⬆️ Carriedforward from 85b776c
hipFFT 54.32% <ø> (+10.89%) ⬆️ Carriedforward from 85b776c
hipRAND 76.12% <ø> (ø) Carriedforward from 85b776c
hipSOLVER 68.96% <ø> (ø) Carriedforward from 85b776c
hipSPARSE 86.99% <ø> (ø) Carriedforward from 85b776c
rocBLAS 48.25% <ø> (ø) Carriedforward from 85b776c
rocFFT 47.25% <ø> (-0.37%) ⬇️
rocRAND 57.42% <ø> (-0.03%) ⬇️ Carriedforward from 85b776c
rocSOLVER 76.83% <ø> (ø) Carriedforward from 85b776c
rocSPARSE 74.75% <ø> (+0.12%) ⬆️ Carriedforward from 85b776c
rocThrust 78.45% <ø> (-14.70%) ⬇️ Carriedforward from 85b776c

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

Files with missing lines Coverage Δ
projects/rocfft/library/src/rtc_bluestein_gen.cpp 96.32% <ø> (-0.02%) ⬇️

... and 186 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 October 6, 2026 19:05
@regan-amd
regan-amd requested review from a team as code owners October 6, 2026 19:05
@regan-amd
regan-amd enabled auto-merge (squash) October 7, 2026 15:28
@regan-amd
regan-amd merged commit fc6b9d6 into develop Oct 7, 2026
94 of 95 checks passed
@regan-amd
regan-amd deleted the users/regan-amd/scale_bluestein_multi_kernel branch October 7, 2026 23:27
@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