Skip to content

C API build script improvements to build dpctl with a custom DPC++ and other changes #319

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

Merged
merged 3 commits into from
May 20, 2021

Conversation

diptorupd
Copy link
Contributor

@diptorupd diptorupd commented Mar 11, 2021

Closes #259
Closes #340

  • FindDPCPP.cmake is now renamed to FindIntelSycl.cmake.
  • llvm-cov and llvm-profdata now have their own cmake modules
    and the minimum version required is 11.0.0 for these tools.
  • A new CMAKE flag DPCTL_CUSTOM_DPCPP_INSTALL_DIR is now available
    that makes it possible to build the DPCTLSyclInterface with a
    custom dpcpp compiler.
  • Formatting changes to CMakeLists.txt files.
  • The sources in dpctl-capi/helper are now included in coverage reports.

@diptorupd diptorupd force-pushed the build_system_fixes branch 2 times, most recently from 36eaaa6 to 4e40659 Compare March 13, 2021 01:36
@diptorupd diptorupd requested review from PokhodenkoSA and removed request for oleksandr-pavlyk March 13, 2021 01:37
@diptorupd diptorupd mentioned this pull request Mar 15, 2021
8 tasks
@oleksandr-pavlyk
Copy link
Contributor

What about Windows? Should we have dpctl-capi/dbg_build.bat as well?

@diptorupd
Copy link
Contributor Author

diptorupd commented Mar 22, 2021

What about Windows? Should we have dpctl-capi/dbg_build.bat as well?

I added the script for windows. For some reason, I can build but not run the gtests on Windows. The dpctl_utils_helper.h does not use the __declspec(dllimport). I will fix that in a separate PR.

@diptorupd diptorupd force-pushed the build_system_fixes branch from b64da3b to fbffc14 Compare March 23, 2021 23:06
@diptorupd
Copy link
Contributor Author

diptorupd commented Mar 23, 2021

@PokhodenkoSA how can the C API gtests be run on Windows? I am using the dbg_build.bat script in this PR. If I do ninja check I can successfully build the tests, but after that get an error. Most likely the issue is the the DPCTLSyclInterface.dll is not found. It will be good to apply a fix if it is a small one and have the C API tests run on Windows and CI.

@PokhodenkoSA
Copy link
Contributor

have the C API tests run on Windows and CI.

On Linux too. I did not find gtest run on Linux CI.

@PokhodenkoSA
Copy link
Contributor

@diptorupd what is the status of the task?

@diptorupd diptorupd modified the milestones: 0.7.0, Next Release Apr 16, 2021
@diptorupd diptorupd force-pushed the build_system_fixes branch 5 times, most recently from 056086c to ab80882 Compare May 6, 2021 03:02
@diptorupd
Copy link
Contributor Author

@oleksandr-pavlyk I cleaned this PR and removed most of the changes to the Python package build scripts (there are still few cosmetic ones). The changes here are only related to our CMake scripts to build DPCTLSyclInterface using custom dpcpp. Once this one is merged we can do the rest of the clean up in #426 and complete this round of refactoring.

This was referenced May 6, 2021
@diptorupd diptorupd requested a review from oleksandr-pavlyk May 6, 2021 03:11
@oleksandr-pavlyk
Copy link
Contributor

oleksandr-pavlyk commented May 6, 2021

The cmake module finder for LLVMCov seems to have a problem:

-- Found Lcov: ~/.conda/envs/idp/bin/lcov
CMake Error at ~/.conda_local/envs/idp/share/cmake-3.19/Modules/FindPackageHandleStandardArgs.cmake:218 (message):
  Could NOT find LLVMCov (missing: LLVMCov_FOUND) (Required is at least
  version "11")
Call Stack (most recent call first):
  ~/.conda_local/envs/idp/share/cmake-3.19/Modules/FindPackageHandleStandardArgs.cmake:582 (_FPHSA_FAILURE_MESSAGE)
  cmake/modules/FindLLVMCov.cmake:65 (find_package_handle_standard_args)
  CMakeLists.txt:188 (find_package)


-- Configuring incomplete, errors occurred!

and the same time:

(idp) [07:37:34 nuc04 dpctl-capi]$ llvm-cov --version
LLVM (http://llvm.org/):
  LLVM version 12.0.0git
  Optimized build.
  Default target: x86_64-unknown-linux-gnu
  Host CPU: skylake
(idp) [07:37:40 nuc04 dpctl-capi]$ which llvm-cov
/localdisk/intel/oneapi/compiler/2021.2.0/linux/bin/llvm-cov

@oleksandr-pavlyk
Copy link
Contributor

Linux CI failed with

  -- Found IntelSycl: /opt/intel/oneapi/compiler/latest/linux (Required is at least version "2021.2.0")
08:12:24
  CMake Error at $PREFIX/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
08:12:24
    Could NOT find LevelZero (missing: LEVEL_ZERO_LIBRARY)

@oleksandr-pavlyk
Copy link
Contributor

The CMake runs fine now with nightly build of intel/llvm@sycl using CMake 3.19 after the change I pushed to FindIntelSycl.cmake

Using CMake 3.20 I run into trouble with finding GTest::mock. It expected libgmock.so in Python environment, which is not there.

Building of program_interface generates 10 __SYCL2020_DEPRECATED warnings, but that is expected.

The linking phase fails with message:

[  3%] Linking CXX executable dpctl_c_api_tests
/usr/bin/ld: cannot find /localdisk/workspace/repos/llvm/build/lib/clang/13.0.0/lib/linux/libclang_rt.profile-x86_64.a: No such file or directory
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)

but it may have to do with my setup more than changes in this PR.

@oleksandr-pavlyk
Copy link
Contributor

Linux CI failed with

  -- Found IntelSycl: /opt/intel/oneapi/compiler/latest/linux (Required is at least version "2021.2.0")
08:12:24
  CMake Error at $PREFIX/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
08:12:24
    Could NOT find LevelZero (missing: LEVEL_ZERO_LIBRARY)

@PokhodenkoSA Should these be libraries be expected in CI ? Should we be setting L0_LIB_DIR and L0_INCLUDE_DIR to help CMake find them in some non-standard location?

@diptorupd diptorupd force-pushed the build_system_fixes branch from fbfb3ae to 2df9735 Compare May 10, 2021 03:26
@diptorupd
Copy link
Contributor Author

Linux CI failed with

  -- Found IntelSycl: /opt/intel/oneapi/compiler/latest/linux (Required is at least version "2021.2.0")
08:12:24
  CMake Error at $PREFIX/share/cmake-3.14/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
08:12:24
    Could NOT find LevelZero (missing: LEVEL_ZERO_LIBRARY)

@PokhodenkoSA Should these be libraries be expected in CI ? Should we be setting L0_LIB_DIR and L0_INCLUDE_DIR to help CMake find them in some non-standard location?

I do not think that is the problem here. ze_loader is definitely installed on our CI system (see the build logs for the other PRs). It is some change I inadvertently introduced that is messing things up. I see from the logs that the C++ compiler selected to build libDPCTLSyclInterface is GCC as opposed to dpcpp as in other builds. I will investigate further and fix it. But may be a while before I get to this as it.

@diptorupd diptorupd force-pushed the build_system_fixes branch 4 times, most recently from 797a6e7 to 3cae7db Compare May 14, 2021 17:19
@diptorupd
Copy link
Contributor Author

diptorupd commented May 16, 2021

@oleksandr-pavlyk

I added the changes we discussed on Friday. The new GetLevelZeroHeaders.cmake replaces the FindLevelZero.cmake module. The new module clones the Level Zero repo and checks out the latest tag to get the headers in the build directory and builds libDPCTLSyclInteface with those headers. I think the changes will help us build on Windows too with Level Zero program compilation support once I make few other changes.

Few more changes:

  • Removed the automatic setting of CMAKE_C_COMPILER and CMAKE_CXX_COMPILER inside FindIntelSycl.cmake. It is considered an anti-pattern to set these variables inside a cmake script (Ref: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#how-do-i-use-a-different-compiler)
  • I added another cmake module GetProjectVersion.cmake that uses Git tags to figure out the version number. I was already doing something similar inside the docs sub-directory. Extracted the code into a reusable module.
  • Added API version and soname to the generated libDPCTLSyclInteface shared library.

I will fix the Windows build with Level zero support and add an action to test the build with the public dpcpp binaries.

@diptorupd diptorupd changed the title Changes to build scripts to enable building dpctl with a custom DPC++ C API build script improvements to build dpctl with a custom DPC++ and other changes May 16, 2021
@diptorupd
Copy link
Contributor Author

diptorupd commented May 16, 2021

@oleksandr-pavlyk Now this needs your expertise :) The issue with the Linux CI is that post adding soname to libDPCTLSyclInterface things are breaking when importing the Cython extensions.

@diptorupd
Copy link
Contributor Author

@oleksandr-pavlyk Tremendous!!

diptorupd added 3 commits May 19, 2021 19:16
…ompiler.

   - FindDPCPP.cmake is now renamed to FindIntelSycl.cmake.
   - llvm-cov and llvm-profdata now have their own cmake modules
     and the minimum version required is 11.0.0 for these tools.
   - CMAKE_C_COMPILER and CMAKE_CXX_COMPILER are now set by the
     FindIntelSycl.cmake module and do not need to be set manually.
   - A new CMAKE flag DPCTL_CUSTOM_DPCPP_INSTALL_DIR is now available
     that makes it possible to build the DPCTLSyclInterface with a
     custom dpcpp compiler.
   - Formatting changes to CMakeLists.txt files.
   - A new helper script to build the DPCTLSyclInterface library using a
     custom dpcpp.
   - All version checking for CMake modules requirements uses LESS_EQUAL
     instead of EQUAL.
   - Replaced FindLevelZero cmake module with GetLevelZeroHeaders module which
     checks out Level zero repo to satisfy dependency of DPCTL on Level-zero
     headers when configured
   - Added GetProjectVersion cmake module to get version and semantic version
     information for Git repos.
Now that Level zero is being checkout out, the location of the .h file
has changed and is controlled via include directory path.
@diptorupd diptorupd force-pushed the build_system_fixes branch from 5ed4140 to b7bd9b9 Compare May 20, 2021 00:16
@diptorupd diptorupd requested review from oleksandr-pavlyk and removed request for PokhodenkoSA May 20, 2021 04:40
@diptorupd diptorupd merged commit d227ab9 into IntelPython:master May 20, 2021
@diptorupd diptorupd deleted the build_system_fixes branch May 20, 2021 13:10
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.

Level Zero on Windows Introduce versioning in soname for libDPCTLSyclInterface
3 participants