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] Make StableHLO build consistent with MLIR-HLO #1612

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

kuhar
Copy link
Member

@kuhar kuhar commented Jun 13, 2023

This applies when stablehlo is built by other projects enabled as an
external llvm project.

This change makes it match the mlir-hlo configuration more closely:
https://github.com/tensorflow/mlir-hlo/blob/51ce2fa22bc9acff37332ad02ef39412f21ffc98/CMakeLists.txt#L113-L130,
and ease the transition from mlir-hlo to stablehlo:
https://discourse.llvm.org/t/sunsetting-the-mlir-hlo-repository/70536.

Tested by embedding stablehlo into IREE.

Issue: #1549

@kuhar kuhar marked this pull request as draft June 13, 2023 15:28
@burmako burmako self-requested a review June 13, 2023 16:32
@burmako burmako self-assigned this Jun 13, 2023
@burmako burmako added the Build label Jun 13, 2023
This applies when stablehlo is built by other projects enabled as an
external llvm project.

This change makes it match the mlir-hlo configuration more closely:
https://github.com/tensorflow/mlir-hlo/blob/51ce2fa22bc9acff37332ad02ef39412f21ffc98/CMakeLists.txt#L113-L130,
and ease the transition from mlir-hlo to stablehlo:
https://discourse.llvm.org/t/sunsetting-the-mlir-hlo-repository/70536.

Tested by embeding stablehlo into IREE.
q
@kuhar kuhar changed the title [CMake] Fix stablehlo configuration when embedded [CMake] Fix stablehlo build configurations Jun 13, 2023
@kuhar kuhar marked this pull request as ready for review June 13, 2023 19:56
@burmako burmako changed the title [CMake] Fix stablehlo build configurations [CMake] Make StableHLO build consistent with MLIR-HLO Jun 13, 2023
@@ -37,19 +37,27 @@ endif()
#-------------------------------------------------------------------------------
# Options and settings
#-------------------------------------------------------------------------------
option(STABLEHLO_BUILD_EMBEDDED "Build StableHLO as part of another project" OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

There are quite a few related boolean variables - STABLEHLO_BUILD_EMBEDDED, STABLEHLO_STANDALONE_BUILD and STABLEHLO_EXTERNAL_PROJECT_BUILD - and I think that not all combinations of them are valid, so it's a bit confusing.

However, given that this PR aims to exactly reproduce the MLIR-HLO setup, I think we should keep these variables as is to first establish the baseline and then think about refining it. I'll maybe look into this in a follow-up PR.

@burmako burmako merged commit 52de2c0 into openxla:main Jun 13, 2023
6 checks passed
burmako pushed a commit that referenced this pull request Jun 23, 2023
To follow up on #1612, this PR further aligns StableHLO and MLIR-HLO
builds to facilitate the transition from mlir-hlo to stablehlo.

Changes in this PR:
  1) Adds Ccache support.
  2) Adds support for LLVM_ENABLE_ZLIB.
  3) Removes STABLEHLO_STANDALONE_BUILD, to better align with MLIR-HLO
     terminology.

Deltas between CMakeLists.txt in this PR and CMakeLists.txt in MLIR-HLO:
  a) Says "StableHLO" instead of "MHLO".
  b) Supports STABLEHLO_ENABLE_STRICT_BUILD (there is no equivalent
     in the MLIR-HLO build).
  c) Doesn't support ${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules yet,
     as well as some MLIR_HLO_FOO_DIR variables needed for that.
     (We have #1549 to follow up on this).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants