CUDA 13.x/MSVC: Add /Zc:preprocessor - #487
Conversation
This issue was detected by GPT 5.6 Sol while reviewing microsoft/vcpkg#53264 See also NVIDIA/cccl#6791 where NVIDIA works around this problem themselves but several outstanding versions of CUDA 13.x need the workaround. The generator expression `"$<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>"` was suggested by GPT 5.6 Sol. I considered adding to your existing `target_compile_options` call rather than adding another one but I wasn't sure how you would want that formatted with the existing `PRIVATE` options that are currently blocked together.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #487 +/- ##
=======================================
Coverage 96.64% 96.64%
=======================================
Files 35 35
Lines 2653 2653
=======================================
Hits 2564 2564
Misses 89 89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
stotko
left a comment
stotko
left a comment
There was a problem hiding this comment.
Thanks for the fix! Could you incorporate the suggested changes?
|
|
||
| target_compile_features(stdgpu PUBLIC cxx_std_17) | ||
|
|
||
| target_compile_options(stdgpu PUBLIC $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>) |
There was a problem hiding this comment.
The macros defined in stdgpu are designed to work with both conforming C++ preprocessors and the older non-standard MSVC variant. Since this primarily affects CCCL and thrust, this flag (including a version check) should belong to the thrust::thrust target in cmake/Findthrust.cmake
There was a problem hiding this comment.
I will move it there but you should be aware that at least through CUDA 13.2 the official bits don't have this fixed yet.
There was a problem hiding this comment.
Thanks. Since the compilation issues are limited to recent CUDA versions, i.e. CCCL 3.2+ (CUDA 13.2+), the flag should be applied conditionally to avoid regressions on older versions. If this is also needed for these, we can adapt the condition accordingly.
|
|
||
| add_library(thrust::thrust INTERFACE IMPORTED) | ||
| set_target_properties(thrust::thrust PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${THRUST_INCLUDE_DIRS}") | ||
| target_compile_options(thrust::thrust INTERFACE $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>) |
There was a problem hiding this comment.
| target_compile_options(thrust::thrust INTERFACE $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>) | |
| if(THRUST_VERSION VERSION_GREATER_EQUAL "3.2.0") | |
| target_compile_options(thrust::thrust INTERFACE $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>) | |
| endif() |
There was a problem hiding this comment.
NVIDIA/cccl#5166 Suggests it needs to be 2.8.0 and affected CUDA 12.9 as well.
|
|
||
| target_compile_features(stdgpu PUBLIC cxx_std_17) | ||
|
|
||
| target_compile_options(stdgpu PUBLIC $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>) |
There was a problem hiding this comment.
Thanks. Since the compilation issues are limited to recent CUDA versions, i.e. CCCL 3.2+ (CUDA 13.2+), the flag should be applied conditionally to avoid regressions on older versions. If this is also needed for these, we can adapt the condition accordingly.
|
No, thank you! |
2 participants
This issue was detected by GPT 5.6 Sol while reviewing microsoft/vcpkg#53264
See also NVIDIA/cccl#6791 where NVIDIA works around this problem themselves but several outstanding versions of CUDA 13.x need the workaround.
The generator expression
"$<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/Zc:preprocessor>"was suggested by GPT 5.6 Sol. I considered adding to your existingtarget_compile_optionscall rather than adding another one but I wasn't sure how you would want that formatted with the existingPRIVATEoptions that are currently blocked together.