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

Thu Aug 27 02:06:09 PDT 2020

ro added a comment.

In D85627#2239832 <https://reviews.llvm.org/D85627#2239832>, @Meinersbur wrote:

> In D85627#2226214 <https://reviews.llvm.org/D85627#2226214>, @ro wrote:
>
>> In D85627#2206550 <https://reviews.llvm.org/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.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85627/new/

https://reviews.llvm.org/D85627