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

[FEA] Add initial CMakeLists.txt #198

Closed
wants to merge 172 commits into from

Conversation

trxcllnt
Copy link

@trxcllnt trxcllnt commented Apr 15, 2022

edit: Closing in favor of #323

This PR adds a CMakeLists.txt file for configuring and building with CMake:
$ cmake \
    -S . \
    -B build \
    -G Ninja \
    -D CMAKE_CUDA_ARCHITECTURES=NATIVE \
    -D Legion_MAX_DIM=3 \
    -D Legion_MAX_FIELDS=256 \
    -D Legion_USE_CUDA=ON \
    -D Legion_USE_LLVM=ON \
    -D Legion_USE_HDF5=OFF \
    -D Legion_USE_OpenMP=ON \
    -D Legion_USE_GASNet=OFF \
    -D Legion_USE_Python=OFF \
    -D Legion_CUDA_DYNAMIC_LOAD=ON \
    -D Legion_BOUNDS_CHECKS=ON \
    -D Legion_HIJACK_CUDART=OFF
$ cmake --build build

The CMakeLists.txt uses CPM and rapids-cmake to find dependencies already installed on the system, or download+build them from source. CPM will ensure dependencies (such as legion) are built as part of building legate.core.

With a properly configured dependency tree, CMake's find_package can also use a dependency's build directory as a package root:

# Build legion from the following local directory
cmake -S . -B build -D CPM_Legion_SOURCE=/path/to/legion

# Or use legion's build directory as the package root
cmake -S . -B build -D Legion_ROOT=/path/to/legion/build

Opening as a draft PR while I implement and test all the functionality presently in install.py.

Depends on https://gitlab.com/StanfordLegion/legion/-/merge_requests/502 edit: 502 has been merged

@manopapad
Copy link
Contributor

manopapad commented Apr 15, 2022

Exciting! A couple of points to keep in mind, as you make changes to the build system:

Legion has a cmake workflow, that you possibly want to plug in. Note, however, that the Legion make and cmake workflows are not necessarily at parity. Legate has only been using the make workflow, so we may have added some features to that workflow that don't exit in the cmake one.

Please make sure that builds can be made incrementally, and dependencies will carry over all the way from Legion to legate.core to cunumeric. For example:

  • legion-build-dir/foo.o should not be updated if legion-src-dir/foo.c and any referenced header files have not changed.
  • legion-install-dir/lib/liblegion.so should not be updated if none of the object files comprising it have changed.
  • legion-install-dir/include/foo.h should not be updated if legion-src-dir/foo.h has not changed.
  • Header file dependencies should be recorded. I don't know how cmake does this, but in the Legion & legate Makefiles I used -MMD and similar compiler options.

Apologies if this all sounds obvious, but it wasn't trivial to get all of this working properly with make, and supporting multiple compilers with different supported -MMD-like options.

Note that Legion, Realm and legate.core record some of their configuration choices in a header file (legion_defines.h, realm_defines.h and config.mk respectively), e.g. whether this is a release or debug build. Everything downstream (recursively) depends on these header files, so it is critical that these are not updated if the build options have not changed.

@trxcllnt trxcllnt marked this pull request as ready for review April 26, 2022 21:38
@trxcllnt trxcllnt changed the title [WIP] Add initial CMakeLists.txt [FEA] Add initial CMakeLists.txt Apr 26, 2022
* Add required realm/legion preincludes
* Make compile definitions part of the public legate_core interface
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@trxcllnt trxcllnt requested a review from ipdemes May 20, 2022 17:10
@trxcllnt trxcllnt changed the base branch from branch-22.05 to branch-22.07 May 25, 2022 20:19
@trxcllnt trxcllnt requested a review from jjwilke July 25, 2022 19:14
@@ -17,6 +23,7 @@ dependencies:
- pyarrow>=5
- scipy
- typing_extensions
- llvm-openmp
Copy link

Choose a reason for hiding this comment

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

not sure if it's worth documenting where this requirement comes from inline in the file with a comment

@trxcllnt trxcllnt changed the base branch from branch-22.07 to branch-22.10 August 15, 2022 17:38
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.

5 participants