[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