Repository navigation
CUFFT: Support Float16 - #2430
Conversation
|
Are the |
|
Ah, rats. That's what the |
|
Yep. But it's probably as easy as adding Lines 288 to 290 in a90cba1 |
| @cenum cudaXtCopyType_t::UInt32 begin | ||
| LIB_XT_COPY_HOST_TO_DEVICE = 0 | ||
| LIB_XT_COPY_DEVICE_TO_HOST = 1 | ||
| LIB_XT_COPY_DEVICE_TO_DEVICE = 2 | ||
| end | ||
|
|
||
| const cudaLibXtCopyType = cudaXtCopyType_t | ||
|
|
||
| @cenum libFormat_t::UInt32 begin | ||
| LIB_FORMAT_CUFFT = 0 | ||
| LIB_FORMAT_UNDEFINED = 1 | ||
| end | ||
|
|
||
| const libFormat = libFormat_t | ||
|
|
||
| struct cudaXtDesc_t | ||
| version::Cint | ||
| nGPUs::Cint | ||
| GPUs::NTuple{64,Cint} | ||
| data::NTuple{64,Ptr{Cvoid}} | ||
| size::NTuple{64,Csize_t} | ||
| cudaXtState::Ptr{Cvoid} | ||
| end | ||
|
|
||
| const cudaXtDesc = cudaXtDesc_t | ||
|
|
||
| struct cudaLibXtDesc_t | ||
| version::Cint | ||
| descriptor::Ptr{cudaXtDesc} | ||
| library::libFormat | ||
| subFormat::Cint | ||
| libDescriptor::Ptr{Cvoid} | ||
| end | ||
|
|
||
| const cudaLibXtDesc = cudaLibXtDesc_t |
There was a problem hiding this comment.
These probably shouldn't live in the CUFFT wrappers, but I don't see any other uses right now.
|
Thanks for the feedback and the pointers so far. Current state: Trying to track down allocation errors |
|
This is ready for a review. I understand that the changes are larger than expected. I essentially removed all (internal) support for the previous This required changing how the plans for the AbstractFFTs interface are represented. There is now a single type |
|
CI fails because it times out before it reaches the CUFFT tests. |
That seems suspicious... CI doesn't print which tests it's working in, so given that the master branch works fine, I'd suspect the hang being in cuFFT. |
|
Hangs locally too: |
|
The culprit seems like a very short stack overflow? MWE: using CUDA
using CUDA.CUFFT
import FFTW
using AbstractFFTs
X = rand(Int, 8)
fftw_X = rfft(X)
d_X = CuArray(X)
p = plan_rfft(d_X)
d_Y = p * d_X |
|
Thanks. The problem was that the input array was converted to the output type of the plan instead of the expected input type. (Those differ for real->complex transforms.) |
|
AFAIU, the Xt libraries are intended for multiGPU workloads (and sometimes post their own problems because of that, see e.g. #2320). The documentation doesn't seem to explicitly state that using cufftXt without cufftXtSetGPUs implies that it's otherwise equivalent to the single-GPU API. Or did I miss anything? |
|
@maleadt I scoured the documentation for cuFFT (https://docs.nvidia.com/cuda/cufft/index.html). I find these statements:
That is, it seems that using multiple GPUs is possible with both The problem to which you point seems to be related to memory allocation. I don't change this in my PR – memory continues to be allocated by creating |
That doesn't matter: The problem is that our memory allocations are asynchronous, and cublasXt accesses memory in a way that complicates use of asynchronous memory. I'm wary that cufftXt has the same issues, necessitating manual synchronization as noted in #2320 (comment). It might be that this only manifests when only effectively using multiple GPUs, but I'd be more comfortable if there was a guarantee that single-GPU cufftXt calls are stream ordered. I'll ping NVIDIA about this. |
|
Alright, it was confirmed to me that cuFFT, it be through the Xt APIs or the regular ones, does identical ordering of operations against the stream set by the user. So this shouldn't pose any problem. |
To support
Float16or other, more generic scenarios, cuFFT plans need to be created viacufftXtMakePlanMany. I think this function provides a superset of all other plan-generating functions.The main difference to the previous APIs is that the transformation is not described via a
cufftType, but directly instead via its input and output types.