[polly][cmake] Don't build LLVMPolly.so without PIC

[ro]

Summary

A build on sparcv9-sun-solaris2.11 with -DLLVM_ENABLE_PIC=Off failed linking LLVMPolly.so:

[2277/2297] Linking CXX shared module lib/LLVMPolly.so
FAILED: lib/LLVMPolly.so
[...]
ld: fatal: relocation error: R_SPARC_H44: file tools/polly/lib/CMakeFiles/obj.Polly.dir/Analysis/DependenceInfo.cpp.o: symbol .data._ZL16__gthread_active (section): invalid shared object relocation type: ABS44 code model unsupported
[...]

As on many other targets, one cannot link non-PIC objects into a shared object on Solaris/sparcv9.

The following patch avoids this by not building the library without PIC. It allowed the build to finish.

Testing showed one regression however:

FAIL: Polly :: Isl/isl-args.ll (66183 of 67884)
[...]
Command Output (stderr):
--
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.      Program arguments: opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/vol/llvm/src/llvm-project/dist/polly/test/Isl -polly-codegen-verify -polly-scops -disable-output -polly-isl-arg=-h
1.      Running pass 'Function Pass Manager' on module '<stdin>'.
2.      Running pass 'Region Pass Manager' on function '@foo_1d'
3.      Running pass 'Polly - Create polyhedral description of Scops' on basic block '%bb1'
0  opt       0x000000010460fb70 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 20
1  opt       0x000000010460e650 SignalHandler(int) + 420
2  libc.so.1 0xffffffff7eedc800 __sighndlr + 12
3  libc.so.1 0xffffffff7eecf23c call_user_handler + 852
4  libc.so.1 0xffffffff7eecf594 sigacthandler + 84
5  libc.so.1 0xffffffff7edecd50 strlen + 80
6  libc.so.1 0xffffffff7ee4d28c _ndoprnt + 20
7  libc.so.1 0xffffffff7ee4c110 printf + 272
8  opt       0x0000000105208814 print_bool_help + 308
9  opt       0x0000000105209e70 isl_args_parse + 1764
10 opt       0x0000000104f36310 polly::Scop::Scop(llvm::Region&, llvm::ScalarEvolution&, llvm::LoopInfo&, llvm::DominatorTree&, polly::ScopDetection::DetectionContext&, llvm::OptimizationRemarkEmitter&, int) + 692
11 opt       0x0000000104f69b50 polly::ScopBuilder::buildScop(llvm::Region&, llvm::AssumptionCache&) + 80
12 opt       0x0000000104f6ada0 polly::ScopBuilder::ScopBuilder(llvm::Region*, llvm::AssumptionCache&, llvm::AAResults&, llvm::DataLayout const&, llvm::DominatorTree&, llvm::LoopInfo&, polly::ScopDetection&, llvm::ScalarEvolution&, llvm::OptimizationRemarkEmitter&) + 588
13 opt       0x0000000104f37400 polly::ScopInfoRegionPass::runOnRegion(llvm::Region*, llvm::RGPassManager&) + 744
14 opt       0x00000001036beda8 llvm::RGPassManager::runOnFunction(llvm::Function&) + 3172
15 opt       0x0000000103dd1c18 llvm::FPPassManager::runOnFunction(llvm::Function&) + 1084
16 opt       0x0000000103dd32e0 llvm::FPPassManager::runOnModule(llvm::Module&) + 32
17 opt       0x0000000103dd0ee0 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 1440
18 opt       0x00000001021070ac main + 9856
19 opt       0x00000001020e6864 _start + 100

gdb shows that strlen incurred a SEGV which is usually a sign of calling it with NULL. Debugging that requires a Debug build which I don't currently have the time and disk space to try.

[Meinersbur]

Could you outline your intended solution? If I read correctly, with LLVM_ENABLE_PIC=OFF, no Polly module will be built, making Polly useless unless Polly is built in-tree and LLVM_POLLY_LINK_INTO_TOOLS is used.

polly/lib/CMakeLists.txt

140–142

Please update the comment

[ro]

Improve comments.

[ro]

In D85627#2206550, @Meinersbur wrote:

Could you outline your intended solution? If I read correctly, with LLVM_ENABLE_PIC=OFF, no Polly module will be built, making Polly useless unless Polly is built in-tree and LLVM_POLLY_LINK_INTO_TOOLS is used.

What do you mean by intended solution? My intent was to avoid breaking the build with PIC code turned off. Just as Windows doesn't support loadable modules and thus creation of LLVMPolly.so is disabled, it's just not possible to build a loadable module/shared object without PIC code, so there's no choice but to disable it.

polly/lib/CMakeLists.txt

140–142

Done. I've also tried to clarify the other comments.

[Meinersbur]

In D85627#2226214, @ro wrote:

In D85627#2206550, @Meinersbur wrote:

What do you mean by intended solution? My intent was to avoid breaking the build with PIC code turned off. Just as Windows doesn't support loadable modules and thus creation of LLVMPolly.so is disabled, it's just not possible to build a loadable module/shared object without PIC code, so there's no choice but to disable it.

I had a warning in mind, but you are right, there is no warning for Windows either. I didn't see the similarity. I could still be improved.

The other issue is that LLVM_ENABLE_PIC is defined by LLVM's CMakeLists.txt, but that is not processed when Polly is built out-of-tree (cmake -S llvm-project/polly), i.e. it this patch would never build LLVMPolly.so when configuring this way.
Anyway, I invested some time to see that LLVM_ENABLE_PIC uf actually is defined when building out-of-tree, defined by the imported LLVMConfig.cmake.

Hence, no fundamental problem and patch LGTM.

[ro]

In D85627#2239832, @Meinersbur wrote:

In D85627#2226214, @ro wrote:

In D85627#2206550, @Meinersbur wrote:

What do you mean by intended solution? My intent was to avoid breaking the build with PIC code turned off. Just as Windows doesn't support loadable modules and thus creation of LLVMPolly.so is disabled, it's just not possible to build a loadable module/shared object without PIC code, so there's no choice but to disable it.

I had a warning in mind, but you are right, there is no warning for Windows either. I didn't see the similarity. I could still be improved.

Probably, yes. However, warnings from cmake are easily overlooked in the large amount of output during the run.

The other issue is that LLVM_ENABLE_PIC is defined by LLVM's CMakeLists.txt, but that is not processed when Polly is built out-of-tree (cmake -S llvm-project/polly), i.e. it this patch would never build LLVMPolly.so when configuring this way.
Anyway, I invested some time to see that LLVM_ENABLE_PIC uf actually is defined when building out-of-tree, defined by the imported LLVMConfig.cmake.

Ah, I'd completely forgotten about standalone builds: I very rarely use them myself (only for libcxx IIRC).

Hence, no fundamental problem and patch LGTM.

Thanks. Btw., I've now seen the Polly :: Isl/isl-args.ll failure again in a RelWithDebInfo build: it's completely unrelated to this patch and caused by a strlen(NULL), which SEGVs on Solaris. I'll post a patch separately and have thus omitted the relevant section from the commit message.

polly/CMakeLists.txt

if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
     # LLVM Doesn't export gtest's include directorys, so do that here
     set_target_properties(gtest
       PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
       "${UNITTEST_DIR}/googletest/include;${UNITTEST_DIR}/googlemock/include"
       )
     set(POLLY_GTEST_AVAIL 1)
   endif()
 
-  # Make sure the isl c files are built as fPIC
+  if (LLVM_ENABLE_PIC)
+    # Make sure the isl c files are built as fPIC if possible
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC")
+  endif ()
 
   # Set directory for polly-isl-test.
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/bin")
 else ()
   set(LLVM_SOURCE_ROOT "${LLVM_MAIN_SRC_DIR}")
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 

polly/cmake/CMakeLists.txt

 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 
 set(LLVM_INSTALL_PACKAGE_DIR "lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
 set(POLLY_INSTALL_PACKAGE_DIR "lib${LLVM_LIBDIR_SUFFIX}/cmake/polly")
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$<LOWER_CASE:$<CONFIG>>.cmake")
 else()
   # avoid conflicts in the build-tree when changing configuration
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-all.cmake")
 endif()
 
 set(POLLY_CONFIG_EXPORTED_TARGETS Polly ${ISL_TARGET})
-if (NOT MSVC)
-  # LLVMPolly is a dummy target on Win
+if (NOT MSVC AND LLVM_ENABLE_PIC)
+  # LLVMPolly is a dummy target on Win or if PIC code is disabled.
   list(APPEND POLLY_CONFIG_EXPORTED_TARGETS LLVMPolly)
 endif()
 if (POLLY_ENABLE_GPGPU_CODEGEN)
   list(APPEND POLLY_CONFIG_EXPORTED_TARGETS PollyPPCG)
 endif()
 
 # Get the target type for every exported target
 foreach(tgt IN LISTS POLLY_CONFIG_EXPORTED_TARGETS)

polly/lib/CMakeLists.txt

 
 if (NOT LLVM_LINK_LLVM_DYLIB AND NOT LLVM_POLLY_LINK_INTO_TOOLS)
     # Polly-ACC requires the NVPTX target to be present in the executable it is linked to
     set_property(TARGET bugpoint APPEND PROPERTY LINK_LIBRARIES LLVMTarget)
 endif ()
 
 # Create a loadable module Polly.so that can be loaded using
 # LLVM's/clang's "-load" option.
-if (MSVC)
-  # Add dummy target, because loadable modules are not supported on Windows
+if (MSVC OR NOT LLVM_ENABLE_PIC)
+  # Add dummy target, either because loadable modules are not supported
+  # as on Windows or because PIC code has been disabled
   add_custom_target(LLVMPolly)
   set_target_properties(LLVMPolly PROPERTIES FOLDER "Polly")
 else ()
   add_polly_loadable_module(LLVMPolly
     Plugin/Polly.cpp
     $<TARGET_OBJECTS:obj.Polly>
   )