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

Introduce a single CMakeLists.txt #2118

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 4 additions & 22 deletions .github/workflows/buildAndTestCMake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ jobs:
# Only run scheduled CI on main repo
if: (github.repository == 'openxla/stablehlo' || github.event_name != 'schedule')
name: "cmake-build ${{ github.event_name == 'schedule' && '(llvm-project@HEAD)' || ''}}"
env:
LLVM_PROJECT_DIR: "llvm-project"
LLVM_BUILD_DIR: "llvm-build"
STABLEHLO_BUILD_DIR: "stablehlo-build"
STABLEHLO_PYTHON_BUILD_DIR: "stablehlo-python-build"
strategy:
fail-fast: false
runs-on: ${{ github.repository == 'openxla/stablehlo' && 'ubuntu-22.04-64core' || 'ubuntu-22.04' }}
Expand All @@ -66,14 +61,6 @@ jobs:
with:
llvm-version: ${{ steps.llvm-version.outputs.version }}

- name: Configure and Build LLVM
shell: bash
run: |
./build_tools/github_actions/ci_build_cmake_llvm.sh "$LLVM_PROJECT_DIR" "$LLVM_BUILD_DIR"
env:
CMAKE_BUILD_TYPE: Release
MLIR_ENABLE_BINDINGS_PYTHON: ON

- name: Fix kernel mmap rnd bits
# Asan in llvm 14 provided in ubuntu 22.04 is incompatible with
# high-entropy ASLR in much newer kernels that GitHub runners are
Expand All @@ -84,16 +71,11 @@ jobs:
- name: Build and Test StableHLO (with AddressSanitizer)
shell: bash
run: |
./build_tools/github_actions/ci_build_cmake.sh "$LLVM_BUILD_DIR" "$STABLEHLO_BUILD_DIR"
env:
CMAKE_BUILD_TYPE: Release
STABLEHLO_ENABLE_BINDINGS_PYTHON: OFF
STABLEHLO_ENABLE_SANITIZER: address
cmake --preset debug
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have some kind of release preset? debug feels like the wrong setting for CI.

cmake --build ./build --target check-stablehlo-ci
Comment on lines +74 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CMake has a concept of a "workflow" that can do this but there is some weird history to CMakePresets [1]

[1] https://gitlab.kitware.com/cmake/cmake/-/issues/22538

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I believe build is sufficient (instead of ./build).


- name: Build and Test StableHLO (with Python bindings)
shell: bash
run: |
./build_tools/github_actions/ci_build_cmake.sh "$LLVM_BUILD_DIR" "$STABLEHLO_BUILD_DIR"
env:
CMAKE_BUILD_TYPE: Release
STABLEHLO_ENABLE_BINDINGS_PYTHON: ON
cmake --preset debug-python
cmake --build ./build --target check-stablehlo-ci
66 changes: 61 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,69 @@ if(STABLEHLO_EXTERNAL_PROJECT_BUILD)
list(APPEND CMAKE_MODULE_PATH "${MLIR_MAIN_SRC_DIR}/cmake/modules")
elseif(NOT STABLEHLO_BUILD_EMBEDDED)
message(STATUS "Building StableHLO with an installed MLIR")
find_package(MLIR REQUIRED CONFIG)

# These defaults are moderately important to us, but the user *can*
# override them (enabling some of these brings in deps that will conflict,
# so ymmv).
# https://github.com/openxla/iree/blob/f3b6bcd79b24ef4a9b355eb3f3496ffafcbd0881/build_tools/cmake/iree_llvm.cmake#L127
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider giving some context about what this link represents in the comment. It seems the lines below were mostly taken from there?

set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
set(LLVM_INCLUDE_BENCHMARKS OFF CACHE BOOL "")
set(LLVM_APPEND_VC_REV OFF CACHE BOOL "")
set(LLVM_ENABLE_IDE ON CACHE BOOL "")
set(LLVM_ENABLE_BINDINGS OFF CACHE BOOL "")
# LLVM defaults to building all targets. We always enable targets that we need
# as we need them, so default to none. The user can override this as needed,
# which is fine.
set(LLVM_TARGETS_TO_BUILD "" CACHE STRING "")

# We enable LLVM projects as needed. The user can override this.
set(LLVM_ENABLE_PROJECTS "" CACHE STRING "")
set(LLVM_EXTERNAL_PROJECTS "" CACHE STRING "")

# Unconditionally enable mlir.
list(APPEND LLVM_ENABLE_PROJECTS mlir)

# Setup LLVM lib and bin directories.
set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/llvm-project/bin)
set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/llvm-project/lib)
set(LLVM_TOOLS_BINARY_DIR ${CMAKE_BINARY_DIR}/llvm-project/bin)
Comment on lines +97 to +118
Copy link
Contributor Author

Choose a reason for hiding this comment

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

having to set these here are pretty ugly; we can discuss if this is an adequate trade-off.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this does and/or why it's needed. I believe this sets where compiled LLVM binaries/libraries will go? I'm assuming LLVM sets this already? Are the values it sets inappropriate for us?


list(APPEND CMAKE_MESSAGE_INDENT " ")
set(_BUNDLED_LLVM_CMAKE_SOURCE_SUBDIR "llvm-project/llvm")
add_subdirectory("${_BUNDLED_LLVM_CMAKE_SOURCE_SUBDIR}" "llvm-project" EXCLUDE_FROM_ALL)
get_directory_property(LLVM_VERSION_MAJOR DIRECTORY "${_BUNDLED_LLVM_CMAKE_SOURCE_SUBDIR}" LLVM_VERSION_MAJOR)
if (NOT LLVM_VERSION_MAJOR)
message(SEND_ERROR "Failed to read LLVM_VERSION_MAJOR property on LLVM directory. Should have been set since https://github.com/llvm/llvm-project/pull/83346.")
endif()
list(POP_BACK CMAKE_MESSAGE_INDENT)

# Set some CMake variables that mirror things exported in the find_package
# world. Source of truth for these is in an installed LLVMConfig.cmake,
# MLIRConfig.cmake, LLDConfig.cmake (etc) and in the various standalone
# build segments of each project's top-level CMakeLists.
set(LLVM_CMAKE_DIR "${CMAKE_BINARY_DIR}/llvm-project/lib/cmake/llvm")
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")
# TODO: Fix MLIR upstream so it doesn't spew into the containing project
# binary dir. See mlir/cmake/modules/CMakeLists.txt
# (and other LLVM sub-projects).
set(MLIR_CMAKE_DIR "${CMAKE_BINARY_DIR}/lib/cmake/mlir")
if(NOT EXISTS "${MLIR_CMAKE_DIR}/AddMLIR.cmake")
message(SEND_ERROR "Could not find AddMLIR.cmake in ${MLIR_CMAKE_DIR}: LLVM sub-projects may have changed their layout. See the mlir_cmake_builddir variable in mlir/cmake/modules/CMakeLists.txt")
endif()
list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")

message(STATUS "Using MLIRConfig.cmake in: ${MLIR_DIR}")
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/bin)
set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/lib)
list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_DIR}")

set(LLVM_INCLUDE_DIRS
${CMAKE_SOURCE_DIR}/${_BUNDLED_LLVM_CMAKE_SOURCE_SUBDIR}/include
${CMAKE_BINARY_DIR}/llvm-project/include
)
set(MLIR_INCLUDE_DIRS
${CMAKE_SOURCE_DIR}/llvm-project/mlir/include
${CMAKE_BINARY_DIR}/llvm-project/tools/mlir/include
)
else()
message(STATUS "Building StableHLO embedded in another project")
endif()
Expand Down
22 changes: 7 additions & 15 deletions CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,27 @@
"LLVM_ENABLE_ASSERTIONS": "ON",
"LLVM_ENABLE_LLD": "ON",
"STABLEHLO_ENABLE_BINDINGS_PYTHON" : "OFF",
"STABLEHLO_ENABLE_SPLIT_DWARF": "ON",
"CMAKE_CXX_COMPILER_LAUNCHER": "ccache",
"CMAKE_CXX_COMPILER": "clang++",
"CMAKE_C_COMPILER_LAUNCHER": "ccache",
"CMAKE_C_COMPILER": "clang",
"CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
"MLIR_DIR": "${sourceDir}/llvm-build/lib/cmake/mlir"
"CMAKE_PLATFORM_NO_VERSIONED_SONAME": "ON",
"LLVM_VERSION_SUFFIX": "",
"LLVM_USE_SPLIT_DWARF": "ON",
"STABLEHLO_ENABLE_SPLIT_DWARF": "ON",
"STABLEHLO_ENABLE_SANITIZER": "address"
}
},
{
"name": "debug-python",
"displayName": "Debug w/ python bindings",
"inherits": "debug",
"cacheVariables": {
"MLIR_ENABLE_BINDINGS_PYTHON": "ON",
"STABLEHLO_ENABLE_BINDINGS_PYTHON" : "ON",
"STABLEHLO_ENABLE_SANITIZER": "OFF"
}
}
],
"buildPresets": [
{
"name": "debug",
"displayName": "Build Debug",
"configurePreset": "debug"
},
{
"name": "debug-python",
"displayName": "Build Debug w/ python bindings",
"configurePreset": "debug-python"
}
]
]
}
84 changes: 15 additions & 69 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,13 @@ Here's how to build the StableHLO repo on Linux or macOS:

```sh
# On Linux
sudo apt install cmake ninja-build lld
sudo apt install cmake ninja-build lld ccache

# On macOS
brew install cmake ninja
brew install cmake ninja ccache
```

2. Set the `LLVM_ENABLE_LLD` shell variable depending on your preferences. We
recommend setting it to `ON` on Linux and to `OFF` on macOS.

```sh
[[ "$(uname)" != "Darwin" ]] && LLVM_ENABLE_LLD="ON" || LLVM_ENABLE_LLD="OFF"
```

3. Clone the StableHLO repo and the LLVM repository:
2. Clone the StableHLO repo and the LLVM repository:

```sh
git clone https://github.com/openxla/stablehlo
Expand All @@ -57,81 +50,34 @@ Here's how to build the StableHLO repo on Linux or macOS:

Cloning the LLVM repository may take a few minutes.

4. Make sure you check out the correct commit in the LLVM repository:
3. Make sure you check out the correct commit in the LLVM repository:

```sh
(cd llvm-project && git fetch && git checkout $(cat ../build_tools/llvm_version.txt))
```

You need to do this every time `llvm_version.txt` changes.

5. Configure and build MLIR:

```sh
MLIR_ENABLE_BINDINGS_PYTHON=OFF build_tools/build_mlir.sh ${PWD}/llvm-project/ ${PWD}/llvm-build
```

This will take a considerable amount of time. For example, on a MacBook Pro
with an M1 Pro chip, building MLIR took around 10 minutes at the moment
of writing.

Again, you need to do this every time `llvm_version.txt` changes.

6. Build StableHLO as a standalone library:
4. Build StableHLO as a standalone library and run all the tests:

```sh
mkdir -p build && cd build

cmake .. -GNinja \
-DLLVM_ENABLE_LLD="$LLVM_ENABLE_LLD" \
-DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_ASSERTIONS=ON \
-DSTABLEHLO_ENABLE_BINDINGS_PYTHON=OFF \
-DMLIR_DIR=${PWD}/../llvm-build/lib/cmake/mlir

cmake --build .
```

If you are actively developing StableHLO, you may want the following additional
CMake settings:

```sh
cmake .. -GNinja \
-DSTABLEHLO_ENABLE_LLD=ON \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DLLVM_ENABLE_ASSERTIONS=ON \
-DSTABLEHLO_ENABLE_BINDINGS_PYTHON=OFF \
-DSTABLEHLO_ENABLE_SPLIT_DWARF=ON \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DSTABLEHLO_ENABLE_SANITIZER=address \
-DMLIR_DIR=${PWD}/../llvm-build/lib/cmake/mlir

cmake --build .
```

This will enable debug symbols and ccache, which can speed up incremental
builds. It also creates a GDB index file in the binary to speed up
debugging.

If you build MLIR using the script above it should also set by default
`LLVM_USE_SPLIT_DWARF` which does the majority of the size saving for
the binary and should also be set.

7. Now you can make sure it works by running some tests:

```sh
ninja check-stablehlo-tests
# first configure the build system
cmake --preset debug
# then build the project
cmake --build ./build --target check-stablehlo-ci
```

You should see results like this:

```txt
Testing Time: 5.99s
Passed: 47
Testing Time: 4.13s

Total Discovered Tests: 137
Passed: 137 (100.00%)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This would be accurate if we were running check-stablehlo-tests, but because we are running check-stablehlo-ci 4 test suites need to run.

IMO, check-stablehlo-tests is more appropriate because the other 3 suites (tosa, linalg, testdata) are not as relevant to day-to-day development.

Also I commented about this elsewhere, and IDK if it's because we're using a debug build or something else, but running these tests is usually way faster for me. On main, I can run all 3888 tests in under 10 seconds, but with this branch it takes over a minute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's add a release preset and test against that?
I think starting the debug binaries are expensive.

```

This runs all the tests in `stablehlo/tests/`.
This runs all the tests in `stablehlo/tests/`. You can change the target
to build or test specific parts of the project.

## Python

Expand Down
69 changes: 0 additions & 69 deletions build_tools/build_mlir.sh

This file was deleted.

Loading
Loading