From d1a8bf20e37f34cdb423a5e3f027fbb29558644c Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 28 Jan 2022 06:46:24 -0600 Subject: [PATCH 1/4] CYTHON_TRACE must be set when generating coverage Setting of this proprocessor variable went missing in the move to cmake + scikit-build, resulting in coverage not going through Cython files anymore. This change should restore that, and thus improve the coverage percentage. --- dpctl/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dpctl/CMakeLists.txt b/dpctl/CMakeLists.txt index 275fe83f80..1d730e9933 100644 --- a/dpctl/CMakeLists.txt +++ b/dpctl/CMakeLists.txt @@ -111,6 +111,10 @@ function(build_dpctl_ext _trgt _src _dest) add_library(${_trgt} MODULE ${_generated_src}) target_include_directories(${_trgt} PRIVATE ${NumPy_INCLUDE_DIR} ${DPCTL_INCLUDE_DIR}) add_dependencies(${_trgt} _build_time_create_dpctl_include) + if (DPCTL_GENERATE_COVERAGE) + target_compile_definitions(${_trgt} PRIVATE CYTHON_TRACE=1 CYTHON_PROFILE=1) + target_compile_options(${_trgt} PRIVATE -fno-sycl-use-footer) + endif() target_link_libraries(${_trgt} DPCTLSyclInterface) target_link_options(${_trgt} PRIVATE "LINKER:${DPCTL_LDFLAGS}") python_extension_module(${_trgt}) From b124c9474e5418d99bb0970adbd29a394cf5d687 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Fri, 28 Jan 2022 18:16:06 -0600 Subject: [PATCH 2/4] Renamed copy_generated_headers.cmake -> copy_existing.cmake --- dpctl/CMakeLists.txt | 6 +++--- .../{copy_generated_headers.cmake => copy_existing.cmake} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename dpctl/cmake/{copy_generated_headers.cmake => copy_existing.cmake} (100%) diff --git a/dpctl/CMakeLists.txt b/dpctl/CMakeLists.txt index 1d730e9933..9ab20afbf3 100644 --- a/dpctl/CMakeLists.txt +++ b/dpctl/CMakeLists.txt @@ -112,7 +112,7 @@ function(build_dpctl_ext _trgt _src _dest) target_include_directories(${_trgt} PRIVATE ${NumPy_INCLUDE_DIR} ${DPCTL_INCLUDE_DIR}) add_dependencies(${_trgt} _build_time_create_dpctl_include) if (DPCTL_GENERATE_COVERAGE) - target_compile_definitions(${_trgt} PRIVATE CYTHON_TRACE=1 CYTHON_PROFILE=1) + target_compile_definitions(${_trgt} PRIVATE CYTHON_TRACE=1 CYTHON_TRACE_NOGIL=1) target_compile_options(${_trgt} PRIVATE -fno-sycl-use-footer) endif() target_link_libraries(${_trgt} DPCTLSyclInterface) @@ -128,11 +128,11 @@ function(build_dpctl_ext _trgt _src _dest) COMMAND ${CMAKE_COMMAND} -DSOURCE_FILE=${_generated_public_h} -DDEST=${CMAKE_CURRENT_SOURCE_DIR} - -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_generated_headers.cmake + -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_existing.cmake COMMAND ${CMAKE_COMMAND} -DSOURCE_FILE=${_generated_api_h} -DDEST=${CMAKE_CURRENT_SOURCE_DIR} - -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_generated_headers.cmake + -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_existing.cmake DEPENDS ${_trgt} VERBATIM COMMENT "Copying Cython-generated headers to destination" diff --git a/dpctl/cmake/copy_generated_headers.cmake b/dpctl/cmake/copy_existing.cmake similarity index 100% rename from dpctl/cmake/copy_generated_headers.cmake rename to dpctl/cmake/copy_existing.cmake From ccacd4af9cf18fa80d989f82e54b2d1feba75966 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Sun, 30 Jan 2022 12:15:52 -0600 Subject: [PATCH 3/4] Changes for Cython.Coverage to work with scikit-build When cython converts .pyx files to cxx it is now provided a working directory option. This makes sure that generated `__pyx_f` array lists sources relative to project directory. When generating coverage, cmake adds a target to copy generated .cxx files to the location of .pyx file. --- dpctl/CMakeLists.txt | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/dpctl/CMakeLists.txt b/dpctl/CMakeLists.txt index 9ab20afbf3..69292f897e 100644 --- a/dpctl/CMakeLists.txt +++ b/dpctl/CMakeLists.txt @@ -1,6 +1,8 @@ find_package(PythonExtensions REQUIRED) find_package(NumPy REQUIRED) + +set(CYTHON_FLAGS "-w ${CMAKE_SOURCE_DIR}") find_package(Cython REQUIRED) if(WIN32) @@ -135,8 +137,21 @@ function(build_dpctl_ext _trgt _src _dest) -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_existing.cmake DEPENDS ${_trgt} VERBATIM - COMMENT "Copying Cython-generated headers to destination" + COMMENT "Copying Cython-generated headers to dpctl" ) + if (DPCTL_GENERATE_COVERAGE) + set(_copy_cxx_trgt "${_trgt}_copy_cxx") + add_custom_target( + ${_copy_cxx_trgt} ALL + COMMAND ${CMAKE_COMMAND} + -DSOURCE_FILE=${_generated_src} + -DDEST=${CMAKE_CURRENT_SOURCE_DIR} + -P ${CMAKE_SOURCE_DIR}/dpctl/cmake/copy_existing.cmake + DEPENDS ${_trgt} + VERBATIM + COMMENT "Copying Cython-generated source to dpctl" + ) + endif() install(TARGETS ${_trgt} LIBRARY DESTINATION ${_dest}) endfunction() From 6fab7027ea9eba6459774224098b2297885b4c44 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Sun, 30 Jan 2022 13:08:12 -0600 Subject: [PATCH 4/4] Added *.cxx to dpctl/.gitignore file --- dpctl/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/dpctl/.gitignore b/dpctl/.gitignore index 0c03f1ed6a..3e23a8af25 100644 --- a/dpctl/.gitignore +++ b/dpctl/.gitignore @@ -1,5 +1,6 @@ *.so *.cpp +*.cxx *.c *.h memory/*.h