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

Undefined behaviour when initializing plan #134

Closed
DejvBayer opened this issue Sep 12, 2023 · 3 comments
Closed

Undefined behaviour when initializing plan #134

DejvBayer opened this issue Sep 12, 2023 · 3 comments

Comments

@DejvBayer
Copy link

Hi,

when function setConfigurationVkFFT is called from initializeVkFFT in module [vkFFT_InitializeApp.h] https://github.com/DTolm/VkFFT/blob/master/vkFFT/vkFFT/vkFFT_AppManagement/vkFFT_InitializeApp.h), there is line that should reset the app configuration, however it is commented out, so the configuration just works with some random values that lead to segfault. Shouldn't it be uncommented?

static inline VkFFTResult setConfigurationVkFFT(VkFFTApplication* app, VkFFTConfiguration inputLaunchConfiguration)  {
	VkFFTResult resFFT = VKFFT_SUCCESS;
	//app->configuration = {};// inputLaunchConfiguration;                              <- this line
	if (inputLaunchConfiguration.doublePrecision != 0)	app->configuration.doublePrecision = inputLaunchConfiguration.doublePrecision;
	if (inputLaunchConfiguration.doublePrecisionFloatMemory != 0)	app->configuration.doublePrecisionFloatMemory = inputLaunchConfiguration.doublePrecisionFloatMemory;

        /* ... */
}

Thanks.

Dave

@DTolm
Copy link
Owner

DTolm commented Sep 12, 2023

Hello,

Originally I thought the user should zero-initialize the app struct before initialization as to not allow potential memory leaks (for example, if initializeVkFFT is called twice on the same app without freeing resources first).

Now I see that it is possible to add a full app memset to zero in the deleteVkFFT call and add a check for app bytes being zero in the initializeVkFFT call. This should be a better approach compared to setting configuration to zero.

I will add this in the next update.

Best regards,
Dmitrii

@DejvBayer
Copy link
Author

Hi Dmitrii,

thank you for your reply. I am sorry, it was my mistake, I accidentally forgot to zero initialize the app object when I moved from OpenCL to CUDA. After zero initialization everything works fine.

Once again thank you very much.

Best regards,
David

DTolm added a commit that referenced this issue Sep 25, 2023
-Added double-double support in VkFFT. Requires cpu initialization in full quad precision, so only supports gcc for now. Potentially possible to add full FP128 support or some other FP128 library (like mpir) in the future.
-Data has to be stored in double-double before VkFFT kernels calls (no fp128<->double-double conversion on the GPU yet).
-Full 1e-32 precision, but same range as FP64. See Library for Double-Double and Quad-Double Arithmetic by Y Hida for more information on double-double.
-Reuqires FMA contraction to be disabled (due to ab-cd contraction rounding mismatch). Doesn't work on Vulkan as I haven't found how to do that yet.
-Fixed warnings (#138)
-Added proper check for app to be zero before initializeVkFFT call and zeroing on deletion (#134)
-Added an option to provide staging buffer in application and VkGPU handle (#129)
-Added guards for build type (#128)
-Fixed missing deallocation calls for the inverse Bluestein axes. Fixed the buffer layout size in Vulkan in some cases.
-Refactored the code generator and container struct layout for better handling complex numbers (-5k loc).
-Added more precision tests and benchmarks.
-Will be merged in the main branch after more testing and update to the documentation.
@DTolm
Copy link
Owner

DTolm commented Sep 25, 2023

Hello,

I have added the checks for the app being zero and freeing the resources on being deleted. It will be soon merged into the master branch, thanks for suggesting it!

Best regards,
Dmitrii

@DTolm DTolm mentioned this issue Oct 23, 2023
DTolm added a commit that referenced this issue Oct 23, 2023
-Added double-double support in VkFFT. Requires cpu initialization in full quad precision, so only supports gcc with quadmath dependency for now. Potentially possible to add full FP128 support or some other FP128 library (like mpir) in the future.
-Data has to be stored in double-double before VkFFT kernels calls (no fp128<->double-double conversion on the GPU yet).
-Full 1e-32 precision, but same range as FP64. See Library for Double-Double and Quad-Double Arithmetic by Y Hida for more information on double-double.
-Double-double requires FMA contraction to be disabled (due to ab-cd contraction rounding mismatch). Doesn't work on Vulkan as I haven't found how to do that yet.
-Added DST I-IV support.
-Fixed warnings (#138)
-Added proper check for app to be zero before initializeVkFFT call and zeroing on deletion (#134)
-Added an option to provide a staging buffer in the application and VkGPU handle (#129)
-Added guards for build type (#128)
-Changed default innermost stride for real buffers in out-of-place R2C from size[0]+2 to size[0] (#139)
-Allow specifying glslang version (#135)
-Improved instruction count and accuracy for radix-7.
-Fixed missing deallocation calls for the inverse Bluestein axes. Fixed the buffer layout size in Vulkan in some cases.
-Refactored the code generator and container struct layout for better handling complex numbers (-5k loc).
-Added more precision tests and benchmarks.
@DTolm DTolm closed this as completed Oct 23, 2023
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