Import test for CUDA project #1285

luncliff · 2019-08-28T16:13:03Z

Summary

This PR adds a new test(fmt-cuda-test) for the library.
It imports fmt in the CUDA(.cu) source file and build it with C++ 14 standard.

find_package(CUDA 9.0)
if(CUDA_FOUND)
  add_subdirectory(cuda-test)
  add_test(NAME cuda-test COMMAND fmt-in-cuda-test)
endif()

Related Issues

Compile error under visual c++ 2017 and CUDA 10.1 #1149 CUDA, C++14, and GCC 6+ #1080(CUDA)

Note

The change of each files...

test/CMakeLists.txt

Try to find if the environment has CUDA and add a new test for it(fmt-cuda-test).

find_package(CUDA 9.0)
if(CUDA_FOUND)
  add_test(...) # ... see the change below ...
endif()

The structure followed that of the find-package-test.

test/cuda-test/CMakeLists.txt

The original issue(#1149) was about the case when NVCC's host compiler is MSVC.
However, the case can't be tested always since Windows CI services don't support CUDA build/test.

Instead, the file contains detailed comments. I wish future visitors can read it and solve their issue with it.

test/cuda-test/cpp14.cc

C++ source code for CMake target which requires CUDA compilation

test/cuda-test/cuda-cpp14.cu

CUDA source code for CMake target. This file is a peer of test/cuda-test/cpp14.cc.

`<fmt/core.h>`

Reviewed #1273.
Following 744302a, the header file checks for NVCC and CUDA source code.

__NVCC__: if the compiler is from NVIDIA
__CUDACC__: if the source code is CUDA(.cu) file

License Agreement

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

Current test is only for Windows(cl.exe). Need to test more with the other host compilers... * Activate the test when `find_package(CUDA)` worked * The test runs with C++14 Detailed comments in 'test/cuda-test'

* checks both `__NVCC__` and `__CUDACC__` More comments for CMake and CUDA source file.

The header file checks 2 things. * __NVCC__: if the compiler is from NVIDIA * __CUDACC__: if the source code is CUDA(.cu) file Since we can't sure all users prefer latest, Version for `find_pacakge(CUDA)` is downgraded to 9.0. This is the minimum version for C++14 in CUDA

luncliff · 2019-08-28T16:21:45Z

Tested Environment

OS/System
- Microsoft Windows 10 Pro
- OS Version/SDK: 10.0.18362 N/A Build 18362
- System Model: XPS 15 9550
- System Type: x64-based PC
Tools
- Visual Studio 2017 Community (15.7.13)
- CMake 3.15.1 (from Chocolatey)
CUDA Version: 10.1

Test Run

After its build, the log from the CTest should be like the following.

The test takes some time since it actually invokes NVCC and performs linking to generate the final executable.

PS D:\fmt\build> ctest --output-on-faulure
Test project D:/fmt/build
      Start  1: assert-test
...
      Start 18: cuda-test
18/18 Test #18: cuda-test ........................   Passed   11.53 sec

6% tests passed, 17 tests failed out of 18

Total Test time (real) =  11.99 sec
...

vitaut

Thanks for the PR!

vitaut · 2019-08-29T00:40:03Z

include/fmt/core.h

 // Workaround broken [[deprecated]] in the Intel compiler.
 #ifdef __INTEL_COMPILER
 #  define FMT_DEPRECATED_ALIAS
 #else
 #  define FMT_DEPRECATED_ALIAS FMT_DEPRECATED
 #endif

+// Workaround broken [[deprecated]] for the NVCC (CUDA with C++14)
+#if defined(__NVCC__) || defined(__CUDACC__)


I suggest merging this into the above #ifdef:

#ifdef __INTEL_COMPILER || defined(__NVCC__) || defined(__CUDACC__)

Sure. can I wrap the Intel compiler's macro in defined? (for consistencty?)

#if defined(__INTEL_COMPILER) || defined(__NVCC__) || defined(__CUDACC__) # define FMT_DEPRECATED_ALIAS #else # define FMT_DEPRECATED_ALIAS FMT_DEPRECATED #endif

vitaut · 2019-08-29T00:45:58Z

test/CMakeLists.txt

+#
+find_package(CUDA 9.0)
+if(CUDA_FOUND)
+  add_test(cuda-test ${CMAKE_CTEST_COMMAND}


Can we not invoke cmake recursively and add cuda_add_executable etc. here? We already found the CUDA package here and don't need to find the FMT package since we are within fmt.

What I intended was to show the full example for importing in CUDA project. test/CMakeLists.txt is quite long so I'm afraid it may confuse future readers.
We can consider 2 alternatives.

include(fmt-cuda-test.cmake) + add_test: we can consider include to shorten test/CMakeLists.txt

add_subdirectory + add_test: I think this is the best

# # Activate CUDA related tests if we can find CUDA from CMake. This is optional. # For version selection, see https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#cpp14-language-features # find_package(CUDA 9.0) if(CUDA_FOUND) add_subdirectory(cuda-test) add_test(NAME cuda-test COMMAND fmt-in-cuda-test) endif()

I agree that add_subdirectory is a better option.

vitaut · 2019-08-29T00:46:33Z

test/cuda-test/CMakeLists.txt

+#
+set_target_properties(fmt-in-cuda-test
+PROPERTIES
+    CXX_STANDARD 14 # Notice this is for C++ code


This seems redundant since you set cxx_std_14 compiler feature.

I will remove the part (CXX_STANDARD)

vitaut · 2019-08-29T00:55:15Z

test/cuda-test/CMakeLists.txt

+get_target_property(cuda_standard 
+    fmt-in-cuda-test CUDA_STANDARD
+)
+message(STATUS "cuda_standard:          ${cuda_standard}")


Case mismatch?

Oh, that was my mistake :)

vitaut · 2019-08-29T00:55:21Z

test/cuda-test/CMakeLists.txt

+message(STATUS "cuda_standard:          ${cuda_standard}")
+
+get_target_property(cuda_standard_required
+    fmt-in-cuda-test CUDA_STANDARD_REQUIRED


vitaut · 2019-08-29T00:57:35Z

test/cuda-test/cuda-cpp14.cu

+#include <cuda.h>
+#include <iostream>
+
+using namespace std;


nit: Let's just qualify cout.

vitaut · 2019-08-31T01:17:48Z

Merged with minor tweaks in 345ba07. Thanks again.

luncliff · 2019-08-31T03:15:51Z

Appreciate for the acceptance :)

luncliff added 4 commits August 28, 2019 09:58

test cuda: import fmt in CUDA source code

ba82f54

Current test is only for Windows(cl.exe). Need to test more with the other host compilers... * Activate the test when `find_package(CUDA)` worked * The test runs with C++14 Detailed comments in 'test/cuda-test'

test cuda: add more comment / macro check

be27926

* checks both `__NVCC__` and `__CUDACC__` More comments for CMake and CUDA source file.

Merge branch 'master' into cuda

bfa245e

vitaut requested changes Aug 29, 2019

View reviewed changes

test cuda: applied PR reviews

30c5209

vitaut closed this Aug 31, 2019

luncliff mentioned this pull request Aug 31, 2019

fmt 6.0.0 warnings under gcc-8.3.0 (+ CUDA nvcc) #1288

Closed

luncliff deleted the cuda branch August 31, 2019 07:36

luncliff mentioned this pull request Sep 20, 2019

Making CUDA test work with CMAKE_MSVC_RUNTIME_LIBRARY #1317

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import test for CUDA project #1285

Import test for CUDA project #1285

luncliff commented Aug 28, 2019 •

edited

Loading

luncliff commented Aug 28, 2019 •

edited

Loading

vitaut left a comment

vitaut Aug 29, 2019

luncliff Aug 29, 2019 •

edited

Loading

vitaut Aug 29, 2019

luncliff Aug 29, 2019 •

edited

Loading

vitaut Aug 29, 2019

vitaut Aug 29, 2019

luncliff Aug 29, 2019

vitaut Aug 29, 2019

luncliff Aug 29, 2019

vitaut Aug 29, 2019

vitaut Aug 29, 2019

luncliff Aug 29, 2019

vitaut commented Aug 31, 2019

luncliff commented Aug 31, 2019

Import test for CUDA project #1285

Import test for CUDA project #1285

Conversation

luncliff commented Aug 28, 2019 • edited Loading

Summary

Related Issues

Note

License Agreement

luncliff commented Aug 28, 2019 • edited Loading

Tested Environment

Test Run

vitaut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luncliff Aug 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luncliff Aug 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaut commented Aug 31, 2019

luncliff commented Aug 31, 2019

luncliff commented Aug 28, 2019 •

edited

Loading

luncliff commented Aug 28, 2019 •

edited

Loading

luncliff Aug 29, 2019 •

edited

Loading

luncliff Aug 29, 2019 •

edited

Loading