[CMake] Fix BUILD_SHARED_LIBS build on Solaris

[ro]

Summary

LLVM currently doesn't build with -DBUILD_SHARED_LIBS=ON on Solaris:

• Quite a number of libs fail to link like this:

Undefined                       first referenced
 symbol                             in file
ceil                                lib/DebugInfo/Symbolize/CMakeFiles/LLVMSymbolize.dir/DIPrinter.cpp.o  (symbol belongs to implicit dependency /usr/lib/amd64/libm.so.2)

This is due to linking with -z defs from llvm/cmake/modules/HandleLLVMOptions.cmake. Solaris ld requires shared objects to be self-contained. Due to the large number of affected libraries (roughly half of all), it's only practical to do this centrally.

• libLLVMTargetParser.so uses libkstat functions without linking it.
• libbenchmark.so likewise has a libm dependency which must be made explicit. This doesn't happen in upstream benchmark, but is also due to linking with z defs, so the patch must stay local to the LLVM tree.

Tested on amd64-pc-solaris2.11 and sparcv9-sun-solaris2.11.

There are a couple of regressions:

• 34 instances on Solaris/amd64 of

LVM ERROR: Must use fast (default) register allocator for unoptimized regalloc.

which are probably due to differences in constructor ordering.

• Also a few instances of

Polly-Unit :: DeLICM/./DeLICMTests/failed_to_discover_tests_from_gtest

which I mean to address later. This patch is only about the build failure.

[mgorny]

llvm/cmake/modules/AddLLVM.cmake

652 ↗

(On Diff #553423)

But is this dependency really specific to Solaris? I think that if we use libm stuff on all platforms, we should link to it whether the linker requires that or not.

That said, I don't know if the current policy is explicitly not do that.

[ro]

llvm/cmake/modules/AddLLVM.cmake

652 ↗

(On Diff #553423)

But is this dependency really specific to Solaris? I think that if we use libm stuff on all platforms, we should link to it whether the linker requires that or not.

No, the use of math functions is everywhere. Unless there are supported targets that don't have an explicit libm (like: folded the math functions into libc), this should be safe.

That said, I don't know if the current policy is explicitly not do that.

I think the Solaris ld's requirement of having self-contained shared objects makes a lot of sense: anything else is bound to cause trouble. And for static libraries, the dependency has no effect anyway.

[ro]

Always link LLVM libs and libbenchmark with libm if it exists.

[phosek]

C++ runtime depends on libm which is why the Clang driver links it on most platforms in the C++ mode, see for example https://github.com/llvm/llvm-project/blob/c998106a7fea2b11bee250dd1f92ed4418a08c5a/clang/lib/Driver/ToolChains/Gnu.cpp#L569, that's not the case on Solaris though where the driver always links libm unless building a shared library, see https://github.com/llvm/llvm-project/blob/c998106a7fea2b11bee250dd1f92ed4418a08c5a/clang/lib/Driver/ToolChains/Solaris.cpp#L253. That explains why this issue only manifests in the BUILD_SHARED_LIBS mode when targeting Solaris. Is this behavior intentional? Could we change the Solaris driver to behave like other platforms and always link libm in the C++ mode?

[ro]

I don't think omitting -lm when building shared objects was intentional. Unfortunately, the original authors of the Solaris driver code apparently didn't believe in comments, so it's hard to say why the deviate from other targets. Add to that the multiyear neglect of that code and you get the current situation. I've recently (while working on [[ https://github.com/llvm/llvm-project/pull/65487| [Driver] Wrap -lgcc_s in -z ignore/-z record on Solaris ]]) noticed that unconditional linking executables with -lm, but didn't make the connection.

As it turns out, when one follows the lead of Gnu.cpp and links with -lm for C++ code, the libm parts of the current patch become unnecessary, only leaving the (hopefully uncontroversial) libkstat part in TargetParser.

I'll post the Solaris driver patch shortly.

[ro]

Remove unnecessary libm handling.

[ro]

Ping? It's been a week and the remainder of the patch is pretty obvious.

[phosek]

LGTM

llvm/lib/TargetParser/CMakeLists.txt

 # Ensure that libLLVMTargetParser does not carry any static global initializer.
 # ManagedStatic can be used to enable lazy-initialization of globals.
 #
 # HAS_WERROR_GLOBAL_CTORS and LLVM_HAS_NOGLOBAL_CTOR_MUTEX should have been set
 # by llvm/lib/Support/CMakeLists.txt (which provides the required Support
 # dependency).
 if (HAS_WERROR_GLOBAL_CTORS AND NOT LLVM_HAS_NOGLOBAL_CTOR_MUTEX)
   SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=global-constructors")
 endif()
 
+# Solaris code uses kstat, so specify dependency explicitly for shared builds.
+if (${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
+  set(system_libs kstat)
+endif()
+
 add_llvm_component_library(LLVMTargetParser
   AArch64TargetParser.cpp
   ARMTargetParserCommon.cpp
   ARMTargetParser.cpp
   CSKYTargetParser.cpp
   Host.cpp
   LoongArchTargetParser.cpp
   RISCVTargetParser.cpp
   SubtargetFeature.cpp
   TargetParser.cpp
   Triple.cpp
   X86TargetParser.cpp
 
   ADDITIONAL_HEADER_DIRS
   Unix
   Windows
 
+  LINK_LIBS
+  ${system_libs}
+
   LINK_COMPONENTS
   Support
 
   DEPENDS
   RISCVTargetParserTableGen
   )