[mlir] Avoid building some shared libraries when PIC is off

[zhuhan0]

Summary

When LLVM_ENABLE_PIC = OFF, shared libraries cannot be built against code that's compiled without -fPIC. Example error message:

ld.lld: error: relocation R_X86_64_32 cannot be used against local symbol; recompile with -fPIC
>>> defined in lib/libLLVMSupport.a(StringMap.cpp.o)
>>> referenced by StringMap.cpp
>>>               StringMap.cpp.o:(llvm::StringMapImpl::StringMapImpl(unsigned int, unsigned int)) in archive lib/libLLVMSupport.a

Similar to how libclang handles this, skip building these shared libraries when LLVM_ENABLE_PIC = OFF.

[zhuhan0]

mlir_runner_utils can still be built

[zhuhan0]

Can I get a review on this?

[MatzeB]

mlir/lib/ExecutionEngine/CMakeLists.txt

149–150

There's other libraries like this one marked with SHARED too. What's different about this one? Not linking against LLVM?

Is the MLIR Exectutiong engine still useful without the two libs excluded here? Otherwise there is already a if(NOT MLIR_ENABLE_EXECUTION_ENGINE) return() endif() above which could be used to just disable it all if LLVM_ENABLE_PIC is false...

[zhuhan0]

mlir/lib/ExecutionEngine/CMakeLists.txt

149–150

Yes the difference about this one is that it doesn't link against other code, which is built without -fPIC, so this one builds fine.

Let me see if we can disable the execution engine altogether with PIC=OFF.

[zhuhan0]

Make it cleaner. I didn't disable MLIRExecutionEngine altogether, but just added an early return as soon as anything related to shared library starts. MLIRExecutionEngine and MLIRJitRunner do not link against any shared library, so they should still be useful to stay.

[krzysz00]

So, as a note, having that early return feels like a great way for someone to add a new utility library to the wrong part of the file. Would it make more sense to make that an if and add an explanatory comment?

[zhuhan0]

Good point. Make it an if and add comment.

[krzysz00]

Seems like a reasonable un-breaking of the build, approved

[zhuhan0]

Thanks @MatzeB and @krzysz00 for the review!

mlir/lib/ExecutionEngine/CMakeLists.txt

add_mlir_library(mlir_float16_utils
 
   EXCLUDE_FROM_LIBMLIR
   )
 set_property(TARGET mlir_float16_utils PROPERTY CXX_STANDARD 17)
 target_compile_definitions(mlir_float16_utils PRIVATE mlir_float16_utils_EXPORTS)
 
 add_subdirectory(SparseTensor)
 
+if(LLVM_ENABLE_PIC)
   add_mlir_library(mlir_c_runner_utils
     SHARED
     CRunnerUtils.cpp
     SparseTensorRuntime.cpp
 
     EXCLUDE_FROM_LIBMLIR
 
     LINK_LIBS PUBLIC
     mlir_float16_utils
     MLIRSparseTensorEnums
     MLIRSparseTensorRuntime
     )
   set_property(TARGET mlir_c_runner_utils PROPERTY CXX_STANDARD 17)
   target_compile_definitions(mlir_c_runner_utils PRIVATE mlir_c_runner_utils_EXPORTS)
+endif()
 
+if(LLVM_ENABLE_PIC)
   add_mlir_library(mlir_runner_utils
     SHARED
     RunnerUtils.cpp
 
     EXCLUDE_FROM_LIBMLIR
   )
   target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)
+endif()
 
+if(LLVM_ENABLE_PIC)
   add_mlir_library(mlir_async_runtime
     SHARED
     AsyncRuntime.cpp
 
     EXCLUDE_FROM_LIBMLIR
 
     LINK_LIBS PUBLIC
     ${LLVM_PTHREAD_LIB}
   )
   set_property(TARGET mlir_async_runtime PROPERTY CXX_VISIBILITY_PRESET hidden)
   target_compile_definitions(mlir_async_runtime PRIVATE mlir_async_runtime_EXPORTS)
+endif()
 
 if(MLIR_ENABLE_CUDA_RUNNER)
   # Configure CUDA support. Using check_language first allows us to give a
   # custom error message.
   include(CheckLanguage)
   check_language(CUDA)
   if (CMAKE_CUDA_COMPILER)
     enable_language(CUDA)

mlir/test/CMakeLists.txt

set(MLIR_TEST_DEPENDS
   tblgen-lsp-server
   )
 
 # The native target may not be enabled, in this case we won't
 # run tests that involves executing on the host: do not build
 # useless binaries.
 if(TARGET ${LLVM_NATIVE_ARCH})
   list(APPEND MLIR_TEST_DEPENDS
-    mlir-cpu-runner
     llc
-    mlir_async_runtime
     mlir-capi-execution-engine-test
+  )
+  # These tests require libmlir_async_runtime.so, libmlir_c_runner_utils.so
+  # and libmlir_runner_utils.so, which are only built with -fPIC.
+  if(LLVM_ENABLE_PIC)
+    list(APPEND MLIR_TEST_DEPENDS
+      mlir-cpu-runner
+      mlir_async_runtime
       mlir_c_runner_utils
       mlir_runner_utils
     )
   endif()
+endif()
 
 if (MLIR_INCLUDE_INTEGRATION_TESTS)
   list(APPEND MLIR_TEST_DEPENDS lli)
 endif()
 
 if(MLIR_ENABLE_CUDA_RUNNER)
   list(APPEND MLIR_TEST_DEPENDS mlir_cuda_runtime)
 endif()