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.

Sign up for GitHub

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] Enable accepting external stablehlo project #3927

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+19 −6
Open

[cmake] Enable accepting external stablehlo project #3927

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 19 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ option(TORCH_MLIR_ENABLE_STABLEHLO "Add stablehlo dialect" ON)
if(TORCH_MLIR_ENABLE_STABLEHLO)
add_definitions(-DTORCH_MLIR_ENABLE_STABLEHLO)
endif()
# It is possible that both stablehlo and torch_mlir projects are used in some compiler project.
# In this case, we might not want to use stablehlo that is downloaded by torch_mlir (in external/stablehlo folder)
# but instead stablehlo that is part of the top level compiler project.
# TORCH_MLIR_EXTERNAL_STABLEHLO_DIR represents stablehlo directory (<some_path>/stablehlo)
# in top level compiler project that will be included in torch_mlir. It is assumed that top level
# compiler project makes stablehlo targets available (for example with `add_subdirectory`) and
# thus they are not added.
set(TORCH_MLIR_EXTERNAL_STABLEHLO_DIR "" CACHE STRING "Path to stablehlo dir from super project")

option(TORCH_MLIR_OUT_OF_TREE_BUILD "Specifies an out of tree build" OFF)

Expand Down Expand Up @@ -233,12 +241,17 @@ endif()
# project that we don't actually depend on. Further some of those parts
# do not even compile on all platforms.
if (TORCH_MLIR_ENABLE_STABLEHLO)
set(STABLEHLO_BUILD_EMBEDDED ON)
set(STABLEHLO_ENABLE_BINDINGS_PYTHON ON)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/externals/stablehlo
${CMAKE_CURRENT_BINARY_DIR}/stablehlo
EXCLUDE_FROM_ALL)
include_directories(${CMAKE_CURRENT_SOURCE_DIR}/externals/stablehlo)
if (NOT "${TORCH_MLIR_EXTERNAL_STABLEHLO_DIR}" STREQUAL "")
# Only include directories. It is assumed that stablehlo targets are made available by external project.
include_directories(${TORCH_MLIR_EXTERNAL_STABLEHLO_DIR})
Copy link
Member

Choose a reason for hiding this comment

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

I would have assumed that the external project also makes the includes available.

else()
Comment on lines +244 to +247
Copy link
Member

Choose a reason for hiding this comment

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

Actually

Suggested change
if (NOT "${TORCH_MLIR_EXTERNAL_STABLEHLO_DIR}" STREQUAL "")
# Only include directories. It is assumed that stablehlo targets are made available by external project.
include_directories(${TORCH_MLIR_EXTERNAL_STABLEHLO_DIR})
else()
# Only configure StableHLO if it isn't provided from a top-level project
if ("${TORCH_MLIR_EXTERNAL_STABLEHLO_DIR}" STREQUAL "")

should be sufficient. In that case I would probably replace the string with a boolean as it seems that the string isn't used elsewhere.

Depending on how the super project is set up (and pulls in torch-mlir and stablehlo) we could consider to add a TORCH_BUILD_EMBEDDED option, as some parts in the current out-of tree build config might be in responsibility of the super project as well.

set(STABLEHLO_BUILD_EMBEDDED ON)
set(STABLEHLO_ENABLE_BINDINGS_PYTHON ON)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/externals/stablehlo
${CMAKE_CURRENT_BINARY_DIR}/stablehlo
EXCLUDE_FROM_ALL)
include_directories(${CMAKE_CURRENT_SOURCE_DIR}/externals/stablehlo)
endif()
endif()

#-------------------------------------------------------------------------------
Expand Down
Loading