Skip to content
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

Cmake cleanup #766

Merged
merged 10 commits into from
Feb 4, 2022
Merged

Cmake cleanup #766

merged 10 commits into from
Feb 4, 2022

Conversation

oleksandr-pavlyk
Copy link
Collaborator

Cleaned up cmake scripts, and transitioned from using Unix makefiles generator on Linux, and Ninja on Windows to using Ninja for both platforms.

This PR accomplishes that incremental rebuilds is very fast if no native source files changed.

Generator is changed in all scripts/ drivers, in conda-recipe and in all workflows.

Instead of using add_custom_command for a TARGET which does not specify
the required PRE_BUILD/PRE_LINK/POST_LINK argument, use add_custom_command
with OUTPUT to perform the copy, record files coped and create a custom
target that depend on those.

All Cython files then depend on that target.
Ninja parallalizes the build and hence works faster to build the project,
it also does not have the bug that make has cause it to unconditionally
recompile and relink extension libraries saving significant iterative
development time.
@github-actions
Copy link

github-actions bot commented Feb 4, 2022

  -- Add an option to specify custom gtest installation.
  -- FindLLVMCov offers a way to specify where to look
     for llvm-cov. If `bin-llvm` is provided, set the
     `LLVM_TOOL_DIR` envar as well to help FindLLVMCov.
     Otherwise, just adding to PATH is not enough.
  -- Some formatting fixes to coverage workflow.
@diptorupd
Copy link
Contributor

@oleksandr-pavlyk I was looking at why the coverage generation workflow is failing. I have not identified it, but made some clean ups to the gen_coverage.py script to be able to replicate the error locally.

@diptorupd
Copy link
Contributor

diptorupd commented Feb 4, 2022

Following is the output from cmake --build . --target lcov-genhtml. The fishy line is /home/diptorupd/.conda/envs/dpctl-devel/bin/ninja llvm-cov. Something is broken in the llvm-cov target that was not getting caught previously.

(dpctl-devel) diptorupd@diptorupd-workstation:~/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build$ cmake --build . --target lcov-genhtml

[1/1] cd /home/diptorupd/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build/libsyclinterface/test...clinterface/tests/dpctl.lcov --output-directory /home/diptorupd/Desktop/devel/dpctl/dpctl-c-api-coverage

FAILED: libsyclinterface/tests/CMakeFiles/lcov-genhtml /home/diptorupd/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build/libsyclinterface/tests/CMakeFiles/lcov-genhtml 

cd /home/diptorupd/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build/libsyclinterface/tests &&

/home/diptorupd/.conda/envs/dpctl-devel/bin/ninja llvm-cov &&

/opt/intel/oneapi/compiler/2022.0.1/linux/bin-llvm/llvm-cov export -format=lcov /home/diptorupd/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build/libsyclinterface/tests/dpctl_c_api_tests -instr-profile=dpctl.profdata /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_service.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_context_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_device_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_device_manager.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_device_selector_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_event_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_kernel_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_platform_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_platform_manager.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_program_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_queue_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_queue_manager.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_sycl_usm_interface.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../source/dpctl_utils.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../helper/source/dpctl_error_handlers.cpp /home/diptorupd/Desktop/devel/dpctl/libsyclinterface/tests/../helper/source/dpctl_utils_helper.cpp > dpctl.lcov && 
/usr/bin/genhtml /home/diptorupd/Desktop/devel/dpctl/_skbuild/linux-x86_64-3.9/cmake-build/libsyclinterface/tests/dpctl.lcov --output-directory /home/diptorupd/Desktop/devel/dpctl/dpctl-c-api-coverage

@oleksandr-pavlyk
Copy link
Collaborator Author

The issue was that lcov-genhtml was emulate what DEPENDS gen-lcov should be doing with use of COMMAND ${CMAKE_MAKE_EXECUTABLE} gen-lcov instead. This was not sitting right with Ninja.

@coveralls
Copy link
Collaborator

Coverage Status

Coverage decreased (-0.3%) to 81.331% when pulling 5505657 on cmake-cleanup into 00bc959 on master.

@diptorupd diptorupd merged commit 9d16d71 into master Feb 4, 2022
@diptorupd diptorupd deleted the cmake-cleanup branch February 4, 2022 15:31
@github-actions
Copy link

github-actions bot commented Feb 4, 2022

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants