[Do not merge] Initial support for auto updating and building dependent third party projects #211
Conversation
|
@silee2 Please add a description, I want to understand what you are attempting. I too am making some changes to build system, so we should make sure our changes do not collide. Since you had a |
|
@diptorupd Added description. |
|
@SiLee I am unable to leave any review comments, maybe my access is not setup correctly. |
| add_subdirectory(third-party) | ||
|
|
||
| if(NOT MLIR_EXT_USE_LLVM_INSTALL_TREE) | ||
| set(LLVM_DIR ${PROJECT_BINARY_DIR}/third-party/mlir/llvm-project/llvm/lib/cmake/llvm) |
There was a problem hiding this comment.
I do not like checking out third party repos in IMEX repo. The problem is if you use git clean -dfx the folder will be wiped out. In my Python build_locally.py, I allow users to set a WORKING_DIR or use system temp directory. Let us at least have the option of providing a -DCMAKE_THIRD_PARTY_LIB_PATH` or something like that?
There was a problem hiding this comment.
The PR does not remove the option of providing -DCMAKE_THIRD_PARTY_LIB_PATH. It only checks out third party repo if -DCMAKE_THIRD_PARTY_LIB_PATH is not provided.
| if(GPU_ENABLE) | ||
| # Check if using external level-zero | ||
| if(DEFINED LEVEL_ZERO_DIR) | ||
| set(MLIR_EXT_USE_EXT_LEVEL_ZERO ON) |
There was a problem hiding this comment.
Let us move to IMEX as the prefix of all our CMake flags.
| @@ -0,0 +1 @@ | |||
| bb7fff05b801e26c3d7858e03e509d1089914d59 | |||
There was a problem hiding this comment.
Perhaps bind to a released tag for L0.
There was a problem hiding this comment.
Yes. That was the intention but did not have time to update tag parsing routine. I already had a SHA parse code so just used a SHA instead of tag for now. The SHA points to tag v1.5.0
| @@ -70,6 +72,43 @@ macro(apply_llvm_compile_flags target) | |||
| target_compile_definitions(${target} PRIVATE ${LLVM_DEFINITIONS}) | |||
| endmacro() | |||
|
|
|||
| # Check if using external LLVM/MLIR install tree | |||
| if (DEFINED MLIR_DIR) | |||
There was a problem hiding this comment.
We should really not need two flags. If someone is using an LLVM build without building MLIR project, CMake will throw an error and we should at that point inform user what to do.
There was a problem hiding this comment.
Perhaps I am not understanding it right. What does MLIR_DIR point to?
There was a problem hiding this comment.
MLIR_DIR is defined to guide "find_package(MLIR)" work correctly in CMake.
LLVM_DIR is for "find_package(LLVM)"
Both were already used in the code base. Not introduced by this PR.
Actually find_package(MLIR) will call find_package(LLVM) internally. So we can clean it up but that requires more changes.
There was a problem hiding this comment.
I had considered using ExternalProject too. What I did not like was that I needed to first to a make llvm and then make imex.
But, I like the overall direction of the changes. They will not break my Python build script and direct usage of CMake will work as well.
|
LLVM is not built separately. |
Current build setup requires all dependent third party components like LLVM/MLIR and level zero loader to be built/installed separately.
This has two issues
This PR addresses those issues by
The change does not break existing usage cases since the old method of building third party components separately and passing path as CMake config arguments is still supported.
CMake checks if third party components provides a path and only builds components missing path.