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

vkFFT backend module #162

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

vkFFT backend module #162

wants to merge 7 commits into from

Conversation

DejvBayer
Copy link

@DejvBayer DejvBayer commented Mar 8, 2024

This merge request introduces vkFFT_Backend module that defines VKFFT_BACKEND_* macro for each backend ID and VKFFT_BACKEND_IS_* macro defined as (VKFFT_BACKEND == VKFFT_BACKEND_*). It makes the backend's specific sections more explicit than comparing VKFFT_BACKEND to some value.

Also a check if VKFFT_BACKEND macro is defined is added to the module.

@DejvBayer
Copy link
Author

More changes:

  • CMake version is upgraded to version 3.17
    • we can use CUDAToolkit instead of depricated CUDA package
    • unity build can be used to significantly reduce compile times
    • clear up whole file, divided into sections, some comments were added
  • chosen VKFFT_BACKEND can be passed to CMake via number (as it was) or by string, e. g. -DVKFFT_BACKEND=CUDA
  • CUDA language is completely removed from the project as it was unnecessary
    • all .cu benchmarks and precision files were renamed to .cpp
    • enable_language was removed from CMake as well as CUDA language configurations

@DTolm
Copy link
Owner

DTolm commented Mar 22, 2024

Hello,

The changes to VKFFT_BACKEND look cosmetic to me but if they can make the code more clear to others I am happy to add them. However, instead of adding the new file, all definitions should be made in the vkFFT.h file - it is an entry point to the whole library and library-wide definitions are stored in it.
As for the CMakeLists changes, I need to look further into them and I think they are better done as a separate pr.

Best regards,
Dmitrii

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

Successfully merging this pull request may close these issues.

None yet

2 participants