[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

153

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

153

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(MLIRJitRunner
   MLIRIR
   MLIRParser
   MLIRLLVMToLLVMIRTranslation
   MLIRTargetLLVMIRExport
   MLIRTransforms
   MLIRSupport
 )
 
+if(NOT LLVM_ENABLE_PIC)
+  return()
+endif()
+
 add_mlir_library(mlir_float16_utils
   SHARED
   Float16bits.cpp
 
   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_mlir_library(mlir_c_runner_utils
   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)
 
 add_mlir_library(mlir_runner_utils
   SHARED
   RunnerUtils.cpp
 
   EXCLUDE_FROM_LIBMLIR
 )
 target_compile_definitions(mlir_runner_utils PRIVATE mlir_runner_utils_EXPORTS)
 
 add_mlir_library(mlir_async_runtime
   SHARED

mlir/test/CMakeLists.txt

set(MLIR_TEST_DEPENDS
   mlir-tblgen
   mlir-translate
   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})
+if(LLVM_ENABLE_PIC AND TARGET ${LLVM_NATIVE_ARCH})
   list(APPEND MLIR_TEST_DEPENDS
     mlir-cpu-runner
     llc
     mlir_async_runtime
     mlir-capi-execution-engine-test
     mlir_c_runner_utils
     mlir_runner_utils
   )