-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[mlir] Fix build after #167848 #167855
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
[mlir] Fix build after #167848 #167855
Conversation
|
@llvm/pr-subscribers-mlir Author: Matthias Springer (matthias-springer) ChangesFix build after #167848. Full diff: https://github.com/llvm/llvm-project/pull/167855.diff 1 Files Affected:
diff --git a/mlir/lib/ExecutionEngine/CMakeLists.txt b/mlir/lib/ExecutionEngine/CMakeLists.txt
index c813a431270d0..90024b1c8206e 100644
--- a/mlir/lib/ExecutionEngine/CMakeLists.txt
+++ b/mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -205,6 +205,11 @@ if(LLVM_ENABLE_PIC)
set_property(TARGET mlir_c_runner_utils PROPERTY CXX_STANDARD 17)
target_compile_definitions(mlir_c_runner_utils PRIVATE mlir_c_runner_utils_EXPORTS)
+ # Conditionally link apfloat wrappers only on Linux.
+ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ target_link_libraries(mlir_c_runner_utils PUBLIC mlir_apfloat_wrappers)
+ endif()
+
add_mlir_library(mlir_runner_utils
SHARED
RunnerUtils.cpp
@@ -216,6 +221,11 @@ if(LLVM_ENABLE_PIC)
)
target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)
+ # Conditionally link apfloat wrappers only on Linux.
+ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ target_link_libraries(mlir_runner_utils PUBLIC mlir_apfloat_wrappers)
+ endif()
+
add_mlir_library(mlir_async_runtime
SHARED
AsyncRuntime.cpp
|
|
@llvm/pr-subscribers-mlir-execution-engine Author: Matthias Springer (matthias-springer) ChangesFix build after #167848. Full diff: https://github.com/llvm/llvm-project/pull/167855.diff 1 Files Affected:
diff --git a/mlir/lib/ExecutionEngine/CMakeLists.txt b/mlir/lib/ExecutionEngine/CMakeLists.txt
index c813a431270d0..90024b1c8206e 100644
--- a/mlir/lib/ExecutionEngine/CMakeLists.txt
+++ b/mlir/lib/ExecutionEngine/CMakeLists.txt
@@ -205,6 +205,11 @@ if(LLVM_ENABLE_PIC)
set_property(TARGET mlir_c_runner_utils PROPERTY CXX_STANDARD 17)
target_compile_definitions(mlir_c_runner_utils PRIVATE mlir_c_runner_utils_EXPORTS)
+ # Conditionally link apfloat wrappers only on Linux.
+ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ target_link_libraries(mlir_c_runner_utils PUBLIC mlir_apfloat_wrappers)
+ endif()
+
add_mlir_library(mlir_runner_utils
SHARED
RunnerUtils.cpp
@@ -216,6 +221,11 @@ if(LLVM_ENABLE_PIC)
)
target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)
+ # Conditionally link apfloat wrappers only on Linux.
+ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
+ target_link_libraries(mlir_runner_utils PUBLIC mlir_apfloat_wrappers)
+ endif()
+
add_mlir_library(mlir_async_runtime
SHARED
AsyncRuntime.cpp
|
|
Why is this conditional? Can you add some more description to the comment before the guards? |
|
This should have been part of this PR which has the comment a few lines above: https://github.com/llvm/llvm-project/pull/167848/files#diff-59f728a05a7cfa4ed1fcb3e644268db28754a743adc28427119b2372e15327feR172 This is basically copied from the CMake setup of C bindings: In both cases, C bindings and ApFloatWrappers.cpp, we want to hide symbols. Unfortunately, the exact mechanism to do this is different on Linux, Mac, Windows. We have to find a better way to do this. |
|
I'm not sure I would put the bindings and a runtime library on the same footing though: the consequences don't seem to be the same: the bindings affect the compiler while the runtime library affect the executable! |
|
It's a bit of a mess tbh. Same pattern here:
What I haven't figured out yet: how come |
Fix build after #167848.