Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise not to overwrite user's data #177

Open
DejvBayer opened this issue Jun 13, 2024 · 2 comments
Open

Promise not to overwrite user's data #177

DejvBayer opened this issue Jun 13, 2024 · 2 comments

Comments

@DejvBayer
Copy link

DejvBayer commented Jun 13, 2024

Hi,

I have noticed that most of the parameters passed to VkFFT are passed by address, for example:

void* buffer = ...;

VkFFTConfiguration config{};
config.buffer = &buffer;

The problem is that the buffer member is defined as void** and if I have a constant array of pointers, the compiler emits warning that I cast void* const* to void**. I would suggest changing all Type** members to Type* const* unless the pointers is modified by VkFFT. It is the same story for VkFFTLaunchParams. Example:

typedef struct {
        // ...
#if(VKFFT_BACKEND==0)
	VkBuffer* buffer;           // -> const VkBuffer* buffer;
	VkBuffer* tempBuffer;       // -> const VkBuffer* tempBuffer;
	VkBuffer* inputBuffer;      // -> const VkBuffer* inputBuffer;
	VkBuffer* outputBuffer;     // -> const VkBuffer* outputBuffer;
	VkBuffer* kernel;           // -> const VkBuffer* kernel;
#elif(VKFFT_BACKEND==1)
	void** buffer;              // -> void* const* buffer;
	void** tempBuffer;          // -> void* const* tempBuffer;
	void** inputBuffer;         // -> void* const* inputBuffer;
	void** outputBuffer;        // -> void* const* outputBuffer;
	void** kernel;              // -> void* const* kernel;
#elif(VKFFT_BACKEND==2)
	void** buffer;              // -> void* const* buffer;
	void** tempBuffer;          // -> void* const* tempBuffer;
	void** inputBuffer;         // -> void* const* inputBuffer;
	void** outputBuffer;        // -> void* const* outputBuffer;
	void** kernel;              // -> void* const* kernel;
#elif(VKFFT_BACKEND==3)
	cl_mem* buffer;             // -> const cl_mem* buffer;
	cl_mem* tempBuffer;         // -> const cl_mem* tempBuffer;
	cl_mem* inputBuffer;        // -> const cl_mem* inputBuffer;
	cl_mem* outputBuffer;       // -> const cl_mem* outputBuffer;
	cl_mem* kernel;             // -> const cl_mem* kernel;
#elif(VKFFT_BACKEND==4)
	void** buffer;              // -> void* const* buffer;
	void** tempBuffer;          // -> void* const* tempBuffer;
	void** inputBuffer;         // -> void* const* inputBuffer;
	void** outputBuffer;        // -> void* const* outputBuffer;
	void** kernel;              // -> void* const* kernel;
#elif(VKFFT_BACKEND==5)
	MTL::Buffer** buffer;       // -> MTL::Buffer* const* buffer;
	MTL::Buffer** tempBuffer;   // -> MTL::Buffer* const* tempBuffer;
	MTL::Buffer** inputBuffer;  // -> MTL::Buffer* const* inputBuffer;
	MTL::Buffer** outputBuffer; // -> MTL::Buffer* const* outputBuffer;
	MTL::Buffer** kernel;       // -> MTL::Buffer* const* kernel;
#endif
        // ...
} VkFFTConfiguration;

One more idea: for input buffers it may be convinient to use const void* const* to indicate that the input data are never overwritten (only for CUDA, HIP and LevelZero).

Thanks a lot!

David

DTolm added a commit that referenced this issue Jun 13, 2024
@DTolm
Copy link
Owner

DTolm commented Jun 13, 2024

Hello,

This is a good idea, I have added it (to all user buffers except tempBuffer, this one is usually allocated and managed by VkFFT). However, it complicates things a bit. For example, cuLaunchKernel expects void** as an argument (https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__EXEC.html#group__CUDA__EXEC_1gb8f3dc3031b40da29d5f9a7139e52e15), so it will just move casting to void** inside VkFFT.

Best regards,
Dmitrii

@DejvBayer
Copy link
Author

I know about it, however I believe this is a better solution. Thanks for the change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants