-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Remove ban on using string_cast with CUDA host code #1041
Conversation
string_cast.hpp merely detects whether the current compiler is NVCC (originally based on `if defined(__CUDACC__)` in glm/simd/platform.h) and throws an error if it is. This means string_cast.hpp cannot be included in any header which might ever be used in a CUDA project. Of course, glm::to_string can't be used in device (GPU) code. However, the current approach to stop this is both incorrect and unnecessary. __CUDACC__ will be defined in both host and device code compilation, and glm::to_string can obviously be used in host code. The correct define is __CUDA_ARCH__ (will be defined only if compiling device code). However, there's no problem if glm::to_string is defined (the header is included) while compiling device code, as long as it's not actually used in the device code. So, throwing an error if __CUDA_ARCH__ is defined would still prevent string_cast.hpp from being included in CUDA projects. There's actually no need for any manual check to see if glm::to_string is being used in device code, because the compiler will already check for that. It returns a std::string, which itself can't be used in device code, so it's unlikely a developer would try. And if they did, there would be errors that both glm::to_string and all the needed std::string constructors, stream operators, etc. are host-only functions.
Actually developers tried... Why not using CUDA_ARCH instead to warn the developers if they try? |
Perhaps it wasn't clear from above. It's perfectly legal to The current state throws an error if string_cast.hpp is included during either CPU or GPU compilation, regardless of whether Your proposal would throw a warning if string_cast.hpp is included during GPU compilation only, regardless of whether If the question is, "why would you ever include a header you can't use?", the answer is, because you're including the header to be used in the CPU code within the same file or project. The header is still included while the GPU code is being compiled. The user could put ifdefs around the include based on A CUDA program using headers which define CPU-only objects is completely normal; developers having to not use those objects in GPU code is completely normal. There's no need for a special warning or any kind of special treatment of this situation in GLM at all. |
But using CUDA_ARCH, wouldn't it be possible to include the files anywhere, no warning, but the code it not present when compile with GPU code? Fundamentally, I would use CUDA_ARCH like ARM or x86 instrinsics but here without implementation at all. |
Yes, that's a possibility. However, the result of that would be, if a developer tried to use Again, there is no need for any kind of special treatment of a CPU-only function that doesn't work in GPU code in CUDA. This is completely normal and every CUDA developer is used to this. |
Ok, let's try then! |
string_cast.hpp merely detects whether the current compiler is NVCC (originally based on
if defined(__CUDACC__)
in glm/simd/platform.h) and throws an error if it is. This means string_cast.hpp cannot be included in any header which might ever be used in a CUDA project.Of course,
glm::to_string
can't be used in device (GPU) code. However, the current approach to stop this is both incorrect and unnecessary.__CUDACC__
will be defined in both host and device code compilation, andglm::to_string
can obviously be used in host code. The correct define is__CUDA_ARCH__
(will be defined only if compiling device code). However, there's no problem ifglm::to_string
is defined (the header is included) while compiling device code, as long as it's not actually used in the device code. So, throwing an error if__CUDA_ARCH__
is defined would still prevent string_cast.hpp from being included in CUDA projects.There's actually no need for any manual check to see if
glm::to_string
is being used in device code, because the compiler will already check for that. It returns a std::string, which itself can't be used in device code, so it's unlikely a developer would try. And if they did, there would be errors that bothglm::to_string
and all the needed std::string constructors, stream operators, etc. are host-only functions.