From 58c532f3002f579eee80a06c269e7d5b6b8c6b3e Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 4 Sep 2020 00:32:40 -0400 Subject: [PATCH 1/5] fix: support nvcc and test --- .github/workflows/ci.yml | 21 +++++++++++++ README.md | 1 + include/pybind11/cast.h | 27 ++++++++++++++--- include/pybind11/numpy.h | 7 +++++ tests/CMakeLists.txt | 43 +++++++++++++++++++++++---- tests/test_constants_and_functions.py | 4 ++- tests/test_copy_move.cpp | 13 ++++---- tests/test_virtual_functions.cpp | 4 +-- 8 files changed, 103 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 825631beae..eff57af7d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -287,6 +287,27 @@ jobs: - name: Interface test run: cmake --build build --target test_cmake_build + cuda: + runs-on: ubuntu-latest + name: "🐍 3.8 • CUDA 11 • Ubuntu 20.04" + container: nvidia/cuda:11.0-devel-ubuntu20.04 + + steps: + - uses: actions/checkout@v2 + + - name: Install 🐍 3 + run: apt-get update && apt-get install -y cmake python3-dev python3-pytest + + - name: Configure + run: cmake -S . -B build -DPYBIND11_CUDA_TESTS=ON -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON + + - name: Build + run: cmake --build build -j2 -v + + - name: Python tests + run: cmake --build build --target pytest + + install-classic: name: "🐍 3.5 • Debian • x86 • Install" runs-on: ubuntu-latest diff --git a/README.md b/README.md index bae6cf2b5c..633231f74a 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,7 @@ In addition to the core functionality, pybind11 provides some extra goodies: 4. Intel C++ compiler 17 or newer (16 with pybind11 v2.0 and 15 with pybind11 v2.0 and a [workaround][intel-15-workaround]) 5. Cygwin/GCC (tested on 2.5.1) +6. NVCC (CUDA 11 tested) ## About diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 5711004df9..6773fdc150 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1006,6 +1006,27 @@ template using is_std_char_type = any_of< std::is_same /* std::wstring */ >; + +template::value, int> = 0> +bool invalid_convert_integral(U) { + return false; +} + +template::value && !std::is_unsigned::value, int> = 0> +bool invalid_convert_integral(U value) { + return sizeof(U) != sizeof(T) + && ( value < (U) (std::numeric_limits::min)() + || value > (U) (std::numeric_limits::max)() + ); +} + +template::value && std::is_unsigned::value, int> = 0> +bool invalid_convert_integral(U value) { + return sizeof(U) != sizeof(T) + && value > (U) (std::numeric_limits::max)(); +} + + template struct type_caster::value && !is_std_char_type::value>> { using _py_type_0 = conditional_t; @@ -1036,10 +1057,8 @@ struct type_caster::value && !is_std_char_t bool py_err = py_value == (py_type) -1 && PyErr_Occurred(); - // Protect std::numeric_limits::min/max with parentheses - if (py_err || (std::is_integral::value && sizeof(py_type) != sizeof(T) && - (py_value < (py_type) (std::numeric_limits::min)() || - py_value > (py_type) (std::numeric_limits::max)()))) { + // Only compare > 0 if not unsigned (warns on some compilers, like nvcc) + if (py_err || invalid_convert_integral(py_value)) { bool type_error = py_err && PyErr_ExceptionMatches( #if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION) PyExc_SystemError diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 674450a631..0192a8b17b 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1483,7 +1483,14 @@ struct vectorize_arg { template struct vectorize_helper { + +// NVCC for some reason breaks if NVectorized is private +#ifdef __CUDACC__ +public: +#else private: +#endif + static constexpr size_t N = sizeof...(Args); static constexpr size_t NVectorized = constexpr_sum(vectorize_arg::vectorize...); static_assert(NVectorized >= 1, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 72de21018a..54f13fd543 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -30,6 +30,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/../tools") option(PYBIND11_WERROR "Report all warnings as errors" OFF) option(DOWNLOAD_EIGEN "Download EIGEN (requires CMake 3.11+)" OFF) +option(PYBIND11_CUDA_TESTS "Enable building CUDA tests (requires CMake 3.12+)" OFF) set(PYBIND11_TEST_OVERRIDE "" CACHE STRING "Tests from ;-separated list of *.cpp files will be built instead of all tests") @@ -49,6 +50,14 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) "RelWithDebInfo") endif() +if(PYBIND11_CUDA_TESTS) + enable_language(CUDA) + if(DEFINED CMAKE_CXX_STANDARD) + set(CMAKE_CUDA_STANDARD ${CMAKE_CXX_STANDARD}) + endif() + set(CMAKE_CUDA_STANDARD_REQUIRED ON) +endif() + # Full set of test files (you can override these; see below) set(PYBIND11_TEST_FILES test_async.cpp @@ -104,6 +113,16 @@ if((PYBIND11_TEST_FILES_ASYNC_I GREATER -1) AND (PYTHON_VERSION VERSION_LESS 3.5 list(REMOVE_AT PYBIND11_TEST_FILES ${PYBIND11_TEST_FILES_ASYNC_I}) endif() +# Skip tests for CUDA check: +# /pybind11/tests/test_constants_and_functions.cpp(125): +# error: incompatible exception specifications +list(FIND PYBIND11_TEST_FILES test_constants_and_functions.cpp PYBIND11_TEST_FILES_CAF_I) +if((PYBIND11_TEST_FILES_CAF_I GREATER -1) AND PYBIND11_CUDA_TESTS) + message( + STATUS "Skipping test_constants_and_functions due to incompatible exception specifications") + list(REMOVE_AT PYBIND11_TEST_FILES ${PYBIND11_TEST_FILES_CAF_I}) +endif() + string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}") # Contains the set of test files that require pybind11_cross_module_tests to be @@ -195,7 +214,7 @@ endif() function(pybind11_enable_warnings target_name) if(MSVC) target_compile_options(${target_name} PRIVATE /W4) - elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)") + elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)" AND NOT PYBIND11_CUDA_TESTS) target_compile_options(${target_name} PRIVATE -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated) endif() @@ -203,6 +222,8 @@ function(pybind11_enable_warnings target_name) if(PYBIND11_WERROR) if(MSVC) target_compile_options(${target_name} PRIVATE /WX) + elseif(PYBIND11_CUDA_TESTS) + target_compile_options(${target_name} PRIVATE "SHELL:-Werror all-warnings") elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Intel|Clang)") target_compile_options(${target_name} PRIVATE -Werror) endif() @@ -239,12 +260,22 @@ foreach(t ${PYBIND11_CROSS_MODULE_GIL_TESTS}) endif() endforeach() +# Support CUDA testing by forcing the target file to compile with NVCC +if(PYBIND11_CUDA_TESTS) + set_property(SOURCE ${PYBIND11_TEST_FILES} PROPERTY LANGUAGE CUDA) +endif() + foreach(target ${test_targets}) set(test_files ${PYBIND11_TEST_FILES}) if(NOT "${target}" STREQUAL "pybind11_tests") set(test_files "") endif() + # Support CUDA testing by forcing the target file to compile with NVCC + if(PYBIND11_CUDA_TESTS) + set_property(SOURCE ${target}.cpp PROPERTY LANGUAGE CUDA) + endif() + # Create the binding library pybind11_add_module(${target} THIN_LTO ${target}.cpp ${test_files} ${PYBIND11_HEADERS}) pybind11_enable_warnings(${target}) @@ -354,8 +385,10 @@ add_custom_command( $ ${CMAKE_CURRENT_BINARY_DIR}/sosize-$.txt) -# Test embedding the interpreter. Provides the `cpptest` target. -add_subdirectory(test_embed) +if(NOT PYBIND11_CUDA_TESTS) + # Test embedding the interpreter. Provides the `cpptest` target. + add_subdirectory(test_embed) -# Test CMake build using functions and targets from subdirectory or installed location -add_subdirectory(test_cmake_build) + # Test CMake build using functions and targets from subdirectory or installed location + add_subdirectory(test_cmake_build) +endif() diff --git a/tests/test_constants_and_functions.py b/tests/test_constants_and_functions.py index 36b1aa64b1..b980ccf1cc 100644 --- a/tests/test_constants_and_functions.py +++ b/tests/test_constants_and_functions.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- -from pybind11_tests import constants_and_functions as m +import pytest + +m = pytest.importorskip("pybind11_tests.constants_and_functions") def test_constants(): diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 0f698bdf05..fa044aa375 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -175,14 +175,17 @@ TEST_SUBMODULE(copy_move_policies, m) { m.attr("has_optional") = false; #endif - // #70 compilation issue if operator new is not public + // #70 compilation issue if operator new is not public - body added but not needed on most compilers struct PrivateOpNew { int value = 1; private: -#if defined(_MSC_VER) -# pragma warning(disable: 4822) // warning C4822: local class member function does not have a body -#endif - void *operator new(size_t bytes); + void *operator new(size_t bytes) { + void *ptr = std::malloc(bytes); + if (ptr) + return ptr; + else + throw std::bad_alloc{}; + } }; py::class_(m, "PrivateOpNew").def_readonly("value", &PrivateOpNew::value); m.def("private_op_new_value", []() { return PrivateOpNew(); }); diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 583c1e647e..6dcf294153 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -139,7 +139,7 @@ class NCVirt { std::string print_movable(int a, int b) { return get_movable(a, b).get_value(); } }; class NCVirtTrampoline : public NCVirt { -#if !defined(__INTEL_COMPILER) +#if !defined(__INTEL_COMPILER) && !defined(__CUDACC__) NonCopyable get_noncopyable(int a, int b) override { PYBIND11_OVERLOAD(NonCopyable, NCVirt, get_noncopyable, a, b); } @@ -205,7 +205,7 @@ TEST_SUBMODULE(virtual_functions, m) { .def(py::init()); // test_move_support -#if !defined(__INTEL_COMPILER) +#if !defined(__INTEL_COMPILER) && !defined(__CUDACC__) py::class_(m, "NCVirt") .def(py::init<>()) .def("get_noncopyable", &NCVirt::get_noncopyable) From 11088ddeeee7f4a2f21687e727692b3d26963f1c Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 4 Sep 2020 09:34:49 -0400 Subject: [PATCH 2/5] fixup! fix: support nvcc and test --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eff57af7d8..5d6e10d028 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -295,8 +295,9 @@ jobs: steps: - uses: actions/checkout@v2 + # tzdata will try to ask for the timezone, so set the DEBIAN_FRONTEND - name: Install 🐍 3 - run: apt-get update && apt-get install -y cmake python3-dev python3-pytest + run: apt-get update && DEBIAN_FRONTEND="noninteractive" apt-get install -y cmake python3-dev python3-pytest - name: Configure run: cmake -S . -B build -DPYBIND11_CUDA_TESTS=ON -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON From 37ace8b5503ab61ebe3352f3be8c7f2e93c0b9d4 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 4 Sep 2020 09:50:18 -0400 Subject: [PATCH 3/5] docs: mention what compilers fail --- tests/test_copy_move.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index fa044aa375..34f1c618d5 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -175,7 +175,10 @@ TEST_SUBMODULE(copy_move_policies, m) { m.attr("has_optional") = false; #endif - // #70 compilation issue if operator new is not public - body added but not needed on most compilers + // #70 compilation issue if operator new is not public - simple body added + // but not needed on most compilers; MSVC and nvcc don't like a local + // struct not having a method defined when declared, since it can not be + // added later. struct PrivateOpNew { int value = 1; private: From da1d2dc90ce09df0a0cb569ace0537bfefc0fde3 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 4 Sep 2020 23:18:34 -0400 Subject: [PATCH 4/5] fix: much simpler logic --- include/pybind11/cast.h | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 6773fdc150..d78df64fcc 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1007,26 +1007,6 @@ template using is_std_char_type = any_of< >; -template::value, int> = 0> -bool invalid_convert_integral(U) { - return false; -} - -template::value && !std::is_unsigned::value, int> = 0> -bool invalid_convert_integral(U value) { - return sizeof(U) != sizeof(T) - && ( value < (U) (std::numeric_limits::min)() - || value > (U) (std::numeric_limits::max)() - ); -} - -template::value && std::is_unsigned::value, int> = 0> -bool invalid_convert_integral(U value) { - return sizeof(U) != sizeof(T) - && value > (U) (std::numeric_limits::max)(); -} - - template struct type_caster::value && !is_std_char_type::value>> { using _py_type_0 = conditional_t; @@ -1055,10 +1035,11 @@ struct type_caster::value && !is_std_char_t : (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr()); } + // Python API reported an error bool py_err = py_value == (py_type) -1 && PyErr_Occurred(); - // Only compare > 0 if not unsigned (warns on some compilers, like nvcc) - if (py_err || invalid_convert_integral(py_value)) { + // Check to see if the conversion is valid (integers should match exactly) + if (py_err || (std::is_integral::value && py_value != (py_type) (T) py_value)) { bool type_error = py_err && PyErr_ExceptionMatches( #if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION) PyExc_SystemError From 424ea1a4f315da90d0b7f7aa07574a2476ab593f Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 10 Sep 2020 10:36:30 -0400 Subject: [PATCH 5/5] refactor: slightly faster / clearer --- include/pybind11/cast.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index d78df64fcc..e57761410f 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1039,7 +1039,8 @@ struct type_caster::value && !is_std_char_t bool py_err = py_value == (py_type) -1 && PyErr_Occurred(); // Check to see if the conversion is valid (integers should match exactly) - if (py_err || (std::is_integral::value && py_value != (py_type) (T) py_value)) { + // Signed/unsigned checks happen elsewhere + if (py_err || (std::is_integral::value && sizeof(py_type) != sizeof(T) && py_value != (py_type) (T) py_value)) { bool type_error = py_err && PyErr_ExceptionMatches( #if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION) PyExc_SystemError