This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Add support for out of tree external projects using MLIR
ClosedPublic

Authored by stephenneuendorffer on Mar 11 2020, 10:53 PM.

Details

Summary

LLVM has a documented mechanism for passing configuration information
to an out of tree project using cmake. See
https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project. This
patch adds similar support for MLIR.

Using this requires something like:

cmake_minimum_required(VERSION 3.4.3)
project(SimpleProject)

find_package(MLIR REQUIRED CONFIG)

include_directories(${LLVM_INCLUDE_DIRS})
include_directories(${MLIR_INCLUDE_DIRS})
link_directories(${LLVM_BUILD_LIBRARY_DIR})
add_definitions(${LLVM_DEFINITIONS})

set(CMAKE_MODULE_PATH

${LLVM_CMAKE_DIR}
${MLIR_CMAKE_DIR}
)

include(AddLLVM)
include(TableGen)
include(AddMLIR)

add_executable(test-opt test-opt.cpp)
llvm_update_compile_flags(test-opt)

get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
get_property(conversion_libs GLOBAL PROPERTY MLIR_CONVERSION_LIBS)
message(dialects=${dialect_libs})
set(LIBS

${dialect_libs}
${conversion_libs}
MLIRLoopOpsTransforms
MLIRLoopAnalysis
MLIRAnalysis
MLIRDialect
MLIREDSC
MLIROptLib
MLIRParser
MLIRPass
MLIRQuantizerFxpMathConfig
MLIRQuantizerSupport
MLIRQuantizerTransforms
MLIRSPIRV
MLIRSPIRVTestPasses
MLIRSPIRVTransforms
MLIRTransforms
MLIRTransformUtils
MLIRTestDialect
MLIRTestIR
MLIRTestPass
MLIRTestTransforms
MLIRSupport
MLIRIR
MLIROptLib
LLVMSupport
LLVMCore
LLVMAsmParser
)

target_link_libraries(test-opt ${LIBS})

Diff Detail

Event Timeline

stephenneuendorffer edited the summary of this revision. (Show Details)
mehdi_amini accepted this revision.Mar 14 2020, 8:05 PM
mehdi_amini added inline comments.
mlir/cmake/modules/CMakeLists.txt
46

I don't really get what this is doing and what is the removal path?

stephenneuendorffer marked an inline comment as done.Mar 14 2020, 8:25 PM
stephenneuendorffer added inline comments.
mlir/cmake/modules/CMakeLists.txt
46

It's copy-pasted from llvm. This delivers all the cmake files so that they are available for use in out-of-tree code. I don't
particularly understand the comment either, which appear to date back to this:

commit 81b580b455426dd885fe4858e5bd3e2f22c00f5f
Author: NAKAMURA Takumi <geek4civic@gmail.com>
Date: Sun Feb 9 16:35:40 2014 +0000

Teach LLVMConfig to avoid modifying CMAKE_MODULE_PATH

Do not modify this value on the application's behalf and just ensure API
modules are always available next to the LLVMConfig module.  This is
already the case in the install tree so use file(COPY) to make it so in
the build tree.  Include the LLVM-Config API module from next to the
LLVMConfig location.

Contributed by Brad King.

llvm-svn: 201047

After looking at this again, I think that delivering the .td files isn't necessary for what I'm trying to do (build against a build area, where we still have the full source). It might be necessary for building against an install area, but we can deal with that later, I think. @rriddle do you have any concerns about the current state?

marbre added inline comments.Mar 17 2020, 1:46 PM
mlir/cmake/modules/CMakeLists.txt
50

Seems like this line might be a leftover from the pre-git times. Can it be dropped?

68–69

Outdated due to the updated definition in line 57?

marbre accepted this revision.Mar 17 2020, 1:50 PM
stephenneuendorffer marked an inline comment as done.Mar 17 2020, 2:32 PM
stephenneuendorffer added inline comments.
mlir/cmake/modules/CMakeLists.txt
68–69

It appears that the intention here is to unset the corresponding variables which are used to pass information into the configure_file call. There is an error which is that not all of the *CONFIG* variables are unset.

rriddle accepted this revision.Mar 17 2020, 2:44 PM
This revision is now accepted and ready to land.Mar 17 2020, 2:44 PM
This revision was automatically updated to reflect the committed changes.

Hi, thanks for improving the way one can plugin MLIR ! While trying to rebase our f18 project that uses MLIR after this patch, I had a cmake issue using installed LLVMs out-of-tree, I tracked it to one of the line in this patch (see in-lined comment).

mlir/cmake/modules/MLIRConfig.cmake.in
23

I am now getting a build error here when building out-of-tree projects that use AddMLIR (in my case f18) and are provided an installed LLVM directory path (as opposed to a build directory path):

CMake Error at <my-llvm-install-dir>/lib/cmake/mlir/MLIRConfig.cmake:27 (include):
  include could not find load file:

    <my-llvm-install-dir>/lib/cmake/mlir/MLIRTargets.cmake
Call Stack (most recent call first):
  CMakeLists.txt:93 (find_package)

Providing an llvm build directory path to f18 cmake command, I do not have that issue (<my-llvm-build-dir>/lib/cmake/mlir/MLIRTargets.cmake always exists for me, but not <my-llvm-install-dir>/lib/cmake/mlir/MLIRTargets.cmake).

The issue seems to be that MLIRTargets.cmake is installed conditionally (depending on MLIR_HAS_EXPORTS), but it is expected to exists unconditionally here.

Would it make sens to make this line something like :

if (EXISTS "@MLIR_CONFIG_EXPORTS_FILE@")
  include("@MLIR_CONFIG_EXPORTS_FILE@")
endif()

or to always install MLIRTargets ?

Hi, thanks for improving the way one can plugin MLIR ! While trying to rebase our f18 project that uses MLIR after this patch, I had a cmake issue using installed LLVMs out-of-tree, I tracked it to one of the line in this patch (see in-lined comment).

This code is partially copied from clang., and it looks like clang has the same issue, if CLANG_HAS_EXPORTS is not set. The big difference is that there is currently no mechanism to set MLIR_HAS_EXPORTS, so this will always get triggered. I think your EXISTS check is probably the best fix. I'll put together a patch.

Hi, thanks for improving the way one can plugin MLIR ! While trying to rebase our f18 project that uses MLIR after this patch, I had a cmake issue using installed LLVMs out-of-tree, I tracked it to one of the line in this patch (see in-lined comment).

This code is partially copied from clang., and it looks like clang has the same issue, if CLANG_HAS_EXPORTS is not set. The big difference is that there is currently no mechanism to set MLIR_HAS_EXPORTS, so this will always get triggered. I think your EXISTS check is probably the best fix. I'll put together a patch.

I've checked in your suggested fix as a short-term workaround I'll need to revisit this longer to determine whether everything is getting exported/installed correctly.

Hi, thanks for improving the way one can plugin MLIR ! While trying to rebase our f18 project that uses MLIR after this patch, I had a cmake issue using installed LLVMs out-of-tree, I tracked it to one of the line in this patch (see in-lined comment).

This code is partially copied from clang., and it looks like clang has the same issue, if CLANG_HAS_EXPORTS is not set. The big difference is that there is currently no mechanism to set MLIR_HAS_EXPORTS, so this will always get triggered. I think your EXISTS check is probably the best fix. I'll put together a patch.

I've checked in your suggested fix as a short-term workaround I'll need to revisit this longer to determine whether everything is getting exported/installed correctly.

Thanks, that fixed the issue for me.