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: add USE_SYSTEM_* options; support find_package(pthreadpool) #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ConnorBaker
Copy link

@ConnorBaker ConnorBaker commented Jul 3, 2023

Similar to the work done in pytorch/pytorch#37137, this adds the following CMake options:

  • USE_SYSTEM_LIBS
  • USE_SYSTEM_GOOGLEBENCHMARK
  • USE_SYSTEM_GOOGLETEST
  • USE_SYSTEM_FXDIV

This is particularly useful in the context of Nix, where we can build these libraries once and then re-use them elsewhere to avoid rebuilding vendors dependencies.

Additionally, adds a CMake configuration file to make it possible for CMake-based projects to use find_package to consume this library.

@ConnorBaker ConnorBaker force-pushed the feat/use-system-libs branch 4 times, most recently from c3cb073 to a531c1b Compare July 3, 2023 02:33
@ConnorBaker ConnorBaker marked this pull request as ready for review July 4, 2023 02:40
@ConnorBaker ConnorBaker force-pushed the feat/use-system-libs branch 7 times, most recently from d7c1425 to 8fc7c96 Compare July 4, 2023 19:54
@ConnorBaker ConnorBaker force-pushed the feat/use-system-libs branch from 8fc7c96 to f320027 Compare July 9, 2023 17:58
@ConnorBaker
Copy link
Author

Updated to reflect comment from pytorch/cpuinfo#153 (comment).

@Maratyszcza if you have the time, would you mind reviewing this?

@petrhosek
Copy link

I'd be interested in seeing this merged as well since we need this change for the https://github.com/google/ml-compiler-opt project. @Maratyszcza would it be possible to take a look?

@@ -0,0 +1,5 @@
@PACKAGE_INIT@

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this also needs the following prior to this line to make sure FXdiv is present:

Suggested change
@PACKAGE_INIT@
include(CMakeFindDependencyMacro)
find_dependency(FXdiv)
@PACKAGE_INIT@

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.

2 participants