[libcxx-commits] [PATCH] D104328: [libc++] Always build libc++ and libc++abi with -fPIC

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: phosek.
ldionne added a comment.

In D104328#2821342 <https://reviews.llvm.org/D104328#2821342>, @mstorsjo wrote:

>> Furthermore, there is apparently basically no downside to building the libraries with position independent code, since modern toolchains are sufficiently clever.
>
> FWIW it's not necessarily so much about modern toolchains, as it is about modern architectures. AFAIK position independent code is measurably less efficient on e.g. i386, while the overhead on e.g. x86_64 and arm64 is much much smaller (possibly also on 32 bit arm). Therefore I think the other patch (D104327 <https://reviews.llvm.org/D104327>) makes more sense out of these two, as the choice of PIC is more of a choice for how one configures and distributes it, than an intrinsic property of the project itself.

I also think the other patch makes more sense from a layering perspective.

Quoting @chandlerc from https://bugs.llvm.org/show_bug.cgi?id=43604:

> IMO, there is a relevant layering distinction here, but I'd put it in a slightly different place.
>
> The difference I would draw is between the build of libraries directly part of LLVM and Clang, and the *runtime* libraries that are used automatically by `clang` invocations of various forms.

Agreed, those are entirely different things and it makes sense to treat each group consistently (so apply the same treatment to all runtimes, whatever that is).

> For runtime libraries, I think always using PIC is reasonable.

I personally don't care. Does @mstorsjo's comment change your mind?

> But for non-runtime LLVM and Clang libraries, I don't think it's nearly as reasonable.
>
> Unfortunately, neither patch is exactly this runtimes vs. non-runtimes split...
> #1 (D104327 <https://reviews.llvm.org/D104327>) - covers all the runtimes but also non-runtimes...

If we were using the Runtimes build instead to produce LLVM releases, it might be possible to pass `-DCMAKE_POSITION_INDEPENDENT_CODE=ON` only to the build of the runtimes? @phosek

> #2 (D104328 <https://reviews.llvm.org/D104328>) - doesn't impose on non-runtimes, but misses compiler-rt and libunwind.

If that's the way we want to go, that should be easy to fix. I can make the same change in libunwind, and I don't know about compiler-rt but I assume it's not difficult to do it either.



================
Comment at: libcxx/src/CMakeLists.txt:276
       OUTPUT_NAME   "c++"
+      POSITION_INDEPENDENT_CODE ON
   )
----------------
compnerd wrote:
> Why build the static library as PIC?  It is going to be statically linked so it doesn't really need to be position independent.  Even if you were to statically link the library into a shared library, it would basically be fully internalized, and the linker should be able to lay it out so that you don't need the PLT/GOT indirection.
This is precisely what the bug report here is asking for: http://llvm.org/PR43604

In fact, this patch builds the shared library with `fPIC` mostly for consistency with the static library. I think it would be useful to have a discussion with the reporter of that bug, and I also know @chandlerc cares about this use case.

Basically, I'm willing to apply whatever fix is going to make people happy, but I'm not sure what's the correct fix here - I'm a bit out of my depth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104328