-
Notifications
You must be signed in to change notification settings - Fork 17
Enable dynamic linking MLIR and LLVM #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ijie/add-cpu-runner
depends on #65 |
CMakeLists.txt
Outdated
@@ -10,6 +10,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake") | |||
|
|||
option(GC_LEGACY_ENABLE ON) | |||
option(GC_TEST_ENABLE "Build the tests" ON) | |||
option(GC_DEV_LINK_DYN_LLVM "Link dynamic libraries of LLVM and MLIR. For developers only. Do not use it in packing the library." OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to change the option name to GC_DEV_LINK_LLVM_DYLIB
which is more align with LLVM's LLVM_LINK_LLVM_DYLIB
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you. I will change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just on minor comment
lib/gc/CMakeLists.txt
Outdated
@@ -2,5 +2,7 @@ if(GC_MLIR_CXX_FLAGS) | |||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${GC_MLIR_CXX_FLAGS}") | |||
endif() | |||
|
|||
include(../../cmake/functions.cmake) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably unnecessary, the functions should be available globally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After removing it, cmake complains missing command. Let's keep it for now.
Added a cmake option
GC_DEV_LINK_DYN_LLVM
for developer. It reduces binary sizes ofgc-cpu-runner
andgc-opt
from36MB
and85MB
to0.2MB
and2MB
. This helps to reduce the link time and the loading time of debuggers likegdb
. This option is by default off.