[PATCH] D72554: [mlir] support building with BUILD_SHARED_LIBS=ON
Mehdi AMINI via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 12 13:20:05 PST 2020
mehdi_amini added a comment.
> Currently, MLIR itself cannot be compiled as a shared library because of the cyclic dependency among MLIR components.
Can we fix the cyclic dependency to begin with?
It seems like this patch is trying to dance around the underlying issue without addressing it.
================
Comment at: mlir/cmake/modules/AddMLIR.cmake:1
+macro(add_mlir_library name)
+ cmake_parse_arguments(ARG
----------------
Can you document this?
================
Comment at: mlir/cmake/modules/AddMLIR.cmake:8
+
+ # LIBTYPE=STATIC as the default within MLIR even with BUILD_SHARED_LIBS=ON
+ set(LIBTYPE STATIC)
----------------
Can you add the *why* in the doc please.
================
Comment at: mlir/cmake/modules/AddMLIR.cmake:64
+ set_target_properties(${name} PROPERTIES FOLDER "Libraries")
+ endif()
+endmacro(add_mlir_library)
----------------
According to the description, this code is identical to `add_llvm_library` but for the ` set(LIBTYPE STATIC)`, this is quite a non-negligible amount of code duplication. Can we refactor this? Can we call to `add_llvm_library` from here after setting the STATIC lib type?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72554/new/
https://reviews.llvm.org/D72554
More information about the llvm-commits
mailing list