[DFT][cuFFT] Fix "Invalid strides" for shapes with a unit last dimension (#631) - #747
Open
zjin-lcf wants to merge 1 commit into
Open
[DFT][cuFFT] Fix "Invalid strides" for shapes with a unit last dimension (#631)#747zjin-lcf wants to merge 1 commit into
zjin-lcf wants to merge 1 commit into
Conversation
When a DFT descriptor has a dimension of extent 1 (e.g. {3,1}, {2,3,1}),
the default strides of that dimension tie with its neighbour's stride. The
cuFFT commit used std::min_element to locate the smallest-stride (innermost)
dimension, which returns the first tied element and thus mis-selects an outer
dimension as innermost. This triggered a spurious dimension swap and made the
check_stride_validity() pre-check reject a perfectly valid transform with
"Invalid strides", even though the underlying cufftPlanMany call succeeds.
Search for the smallest stride in reverse so ties resolve to the innermost
(highest-index) dimension, avoiding the incorrect swap. Also capture the
selected index before erase() to avoid using an invalidated iterator.
Fixes uxlfoundation#631.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #631.
oneapi::math::dfton the cuFFT backend throwsdft/backends/cufft/commit: Invalid stridesfor multi-dimensional transforms whose last dimension has extent 1 (e.g.{3,1},{2,1},{5,1},{2,3,1}). The same descriptors work on the Intel (MKL) backends, and the equivalent transform works when calling cuFFT directly.Root cause
In
src/dft/backends/cufft/commit.cpp, the backend usesstd::min_elementto find the dimension with the smallest stride (which becomes the cuFFTistride/ostride, i.e. the innermost dimension).When a dimension has extent 1, its default stride ties with its neighbour's stride.
std::min_elementreturns the first element among ties, so an outer dimension is mis-identified as the innermost one. This triggers the dimension-swap heuristic, producing a dimension/stride ordering that thecheck_stride_validity()pre-check then rejects — computinginner_n (e.g. 3) <= inner_stride (1)as false for both directions and throwingInvalid strides. The underlyingcufftPlanManycall that would have been issued is actually valid.There is also latent undefined behaviour: the swap logic computes the selected index from
a_minaftervec_a.erase(a_min)has invalidated that iterator. This is why the failure is compiler-dependent (it reproduces withicpx, but a recent open-source DPC++/clang build happens to take a different, passing path).Fix
erase(), removing the invalidated-iterator UB.No behavioural change for shapes without unit dimensions: for strictly decreasing default strides the reverse search selects the same (last) element as before.
Test plan
Validated end-to-end on an NVIDIA sm_103 device, CUDA 13.2 / cuFFT 12.2, using an open-source DPC++ toolchain (intel/llvm nightly with the CUDA backend). Built oneMath with
-DENABLE_CUFFT_BACKEND=ON -DTARGET_DOMAINS=dft.With the fix, the reproducer from #631 and related shapes all pass:
cufftPlan2d(3,1)and the exactcufftPlanManyparams oneMath generates) succeed on the B300 — the failure is in the pre-check, not cuFFT.{3,1},{2,1},{5,1},{2,3,1}; fixed logic ACCEPTS them; regression shapes unchanged.Suggestion for maintainers: consider adding a DFT test case covering shapes with a unit last dimension to guard against regressions.