Page MenuHomePhabricator

[MLIR] Move from using target_link_libraries to LINK_LIBS for llvm libraries.
ClosedPublic

Authored by stephenneuendorffer on Feb 19 2020, 1:28 PM.

Details

Summary

When compiling libLLVM.so, add_llvm_library() manipulates the link libraries
being used. This means that when using add_llvm_library(), we need to pass
the list of libraries to be linked (using the LINK_LIBS keyword) instead of
using the standard target_link_libraries call. This is preparation for
properly dealing with creating libMLIR.so as well.

Diff Detail

Event Timeline

vchuravy added inline comments.Feb 19 2020, 6:32 PM
mlir/examples/toy/Ch6/CMakeLists.txt
40

Indentation seems off here and in other files

This is preparation for properly dealing with creating libMLIR.so as well.

I asked in another revision, but I don't think I got an answer: what is the story for the circular dependency if we were to use MLIR in LLVM in the future? I'd be worried about adding barriers into this and I have some memory of a previous discussion that keeping a single .so would be somehow required?

I asked in another revision, but I don't think I got an answer: what is the story for the circular dependency if we were to use MLIR in LLVM in the future? I'd be worried about adding barriers into this and I have some memory of a previous discussion that keeping a single .so would be somehow required?

I think way of doing that would be to remove libMLIR (all users of that library would need to use libLLVM instead), Right now the MLIR libraries are included in LLVM_AVAILABLE_LIBS, but they are not part of LLVM_COMPONENT_LIBS which is the set of things that are considered the inclusion into the shared library. So instead of add_llvm_library we would use add_llvm_component_library.

All the work that Stephen is doing is still required otherwise MLIR wouldn't work as part of the libLLVM. The same with your recent work on reducing the use of static initializers (I think).

Can we get directly to libLLVM using add_llvm_component_library instead of introducing libMLIR? Otherwise folks will start to depend on libMLIR and it'll cause more pain to remove it later

Can we get directly to libLLVM using add_llvm_component_library instead of introducing libMLIR? Otherwise folks will start to depend on libMLIR and it'll cause more pain to remove it later

We could: see https://reviews.llvm.org/D72586
However, based on the original reviews, it seemed that having libMLIR.so was preferred.

@mehdi_amini and I had a conversation on discord about libMLIR.so and I think we allayed all fears that this would lock us into being stuck with a libMLIR.so if we want to eventually go down the path of including it as part of libMLIR.so
So I think we are good to go ahead with this.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 28 2020, 11:51 AM
This revision was automatically updated to reflect the committed changes.

@mehdi_amini and I had a conversation on discord about libMLIR.so and I think we allayed all fears that this would lock us into being stuck with a libMLIR.so if we want to eventually go down the path of including it as part of libMLIR.so
So I think we are good to go ahead with this.

Absolutely! Thanks both :)