[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

133–134

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

133–134

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!