Page MenuHomePhabricator

[CMake] don't build libLTO when LLVM_ENABLE_PIC is OFF

Authored by sugak on Feb 9 2016, 3:30 PM.



When cmake is run with -DLLVM_ENABLE_PIC=OFF, build fails while
linking shared library, because its dependencies are built
with -fno-PIC. More details here:
This diff reverts r252652 (git 9fd4377ddb83aee3c049dc8757e7771edbb8ee71),
which removed check NOT LLVM_ENABLE_PIC before disabling build for

I've verified that when cmake is run with -DLLVM_ENABLE_PIC=ON, is
still generated, as well as when this option is not specified and the default
value (ON) is used. Tested both on master and release_38.

Diff Detail


Event Timeline

sugak updated this revision to Diff 47383.Feb 9 2016, 3:30 PM
sugak retitled this revision from to [CMake] don't build libLTO when LLVM_ENABLE_PIC is OFF.
sugak updated this object.
sugak added a reviewer: beanz.
sugak added subscribers: hans, cfe-commits.
sugak edited subscribers, added: llvm-commits; removed: cfe-commits.Feb 10 2016, 9:04 AM

That makes sense to me, but I'm not sure what Chris meant when he wrote: "Autoconf builds libLTO with -fPIC, CMake should be able to as well", it seems his change in r252652 enabled building libLTO *without* PIC?

sugak updated this object.Feb 10 2016, 9:19 AM
sugak added a comment.Feb 10 2016, 9:43 AM

@joker.eph: yes, in environments that don't set CYGWIN, after r252652 libLTO is always built, regardless of LLVM_ENABLE_PIC.

hans added a comment.Feb 10 2016, 11:03 AM

This seems reasonable to me too, but it would be good to hear what Chris thinks.

beanz accepted this revision.Feb 11 2016, 1:29 PM
beanz edited edge metadata.

The LLVM_ENABLE_PIC option is oddly implemented on Darwin, and is really just broken. I have on my todo list to make better sense of it on Darwin.

This patch looks good, and should land.


This revision is now accepted and ready to land.Feb 11 2016, 1:29 PM
sugak added a comment.Feb 11 2016, 7:38 PM

I need someone to commit this for me.

Thank you!

This revision was automatically updated to reflect the committed changes.
hans added a comment.Feb 12 2016, 11:07 AM

And merged to 3.8 in r260704.