[Driver] Add arch-specific rpath for libc++

[Hahnfeld]

Summary

libc++ may be installed there as a runtime library.

[pirama]

lib/Driver/ToolChain.cpp

652

addArchSpecificRPath is a static function in Tools.cpp and isn't visible here.

[jroelofs]

As I said on D30214, it is inappropriate to be installing libc++ in the resource directory... please do not do that.

[Hahnfeld]

In D30733#697108, @jroelofs wrote:

As I said on D30214, it is inappropriate to be installing libc++ in the resource directory... please do not do that.

Can you give reason for that? I can understand that header files are independent of the target architecture but how do you handle multiple binary libraries for let's say 32 and 64 bit?
This was the main motivation for the OpenMP runtime in http://lists.llvm.org/pipermail/openmp-dev/2016-December/001612.html, please also see https://bugs.llvm.org//show_bug.cgi?id=31300. I don't think libc++ would be any different here.

Additionally, my main motiviation was for libunwind as explained here: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html
On that thread multiple people suggested to use an extra directory for runtime libraries, @rnk and @mgorny listed as reviewers for this patch. If libunwind goes there and is added together with libc++, we need to add the rpath here. And what's the reason against installing libc++ in the same path then?

lib/Driver/ToolChain.cpp

652

No, it's not since recent refactoring. I do compile test my changes usually ;-)

[jroelofs]

In D30733#697313, @Hahnfeld wrote:

In D30733#697108, @jroelofs wrote:

As I said on D30214, it is inappropriate to be installing libc++ in the resource directory... please do not do that.

Can you give reason for that?

libc++ is not version-locked to the compiler, however that directory is.

I understand you want to solve your problem, but I think we (as a community) need to take a step back and look at the implications of decisions like this... and having thought through this, I think this is not a wise choice if we value long term support. Yes, this plan /could/ work for a given individual release, but I think it does not scale to use cases where the libs have been chosen by some external constraint. For example, user code built against an old version of libc++: sometimes it's ok to upgrade the compiler, but often you're stuck with a particular version of the libraries because some other software was built against them.

I can understand that header files are independent of the target architecture but how do you handle multiple binary libraries for let's say 32 and 64 bit?

Arch + multilib suffixed directories solves this problem.

This was the main motivation for the OpenMP runtime in http://lists.llvm.org/pipermail/openmp-dev/2016-December/001612.html, please also see

My objection isn't to putting the libs in a directory that contains the arch name / multilib name, but rather it is to having that directory be a child of lib/clang/$version/{lib, include}.

https://bugs.llvm.org//show_bug.cgi?id=31300. I don't think libc++ would be any different here.

Additionally, my main motiviation was for libunwind as explained here: http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html
On that thread multiple people suggested to use an extra directory for runtime libraries, @rnk and @mgorny listed as reviewers for this patch.

And I disagree. The only things that belong in clang's resource directory are the builtins, sanitizer libs, and maybe the OpenMP libs (assuming they're tied to a particular compiler version, and don't have a stable API/ABI).

If libunwind goes there and is added together with libc++, we need to add the rpath here. And what's the reason against installing libc++ in the same path then?

IMNSHO, libunwind does not belong in the resource directory either.

Concrete suggestion (and maybe we ought to discus this more widely on cfe-dev in RFC form): create a new directory not tied to the compiler version, and place non version locked libraries there. One way that could look is something like:

lib/clang/SDKs/$vendor/$platform/$abi/$arch/{lib/$multilib, include}

with, for example:

lib
└── clang
    └── SDKs
        ├── apple
        │   └── darwin15
        │       ├── fat
        │       ├── powerpc
        │       └── x86_64
        └── ubuntu10
            └── linux
                └── gnueabi
                    ├── arm
                    └── x86_64
                        ├── lib
                        └── lib64

(I don't want to get in the weeds bike-shedding the specific details of this layout just yet, so take this as a general idea with room for flexibility, and a small amount of hand-waving)

representing the layout for a cross compiler with SDKs for:

• ppc darwin
• x86_64 darwin
• fat library containing several darwin targets
• arm linux
• x86_64 linux, with both -m32 and -m64 multilibs

With this strategy, now you can have say clang-4.0 installed on your system, along with some libraries that came with it. Now if you've built things against those libs, and upgrade your clang version, you are not tied to the new libc++ that comes with it, as you would be with libc++ placed in the resource dir.

[jroelofs]

I'll also add that we had a BOF at EuroLLVM 2014, where this got support from the community and people generally thought it was a good plan... Just needed someone to follow through with it.

We (wearing my CodeSourcery hat) said we would do so, but have been making slow progress on it since then. Now that we've got more time to work on our arm bare-metal toolchain, I expect that to pick up... sorry it has taken so long :/

[joerg]

Installing libc++ along clang makes a lot of sense for non-system compilers. It doesn't imply any version locking like in GCC, i.e. you are still free to install whatever version of libunwind and libc++ you want. I'm somewhat conflicted about this patch. I think it is an actual improvement for some deployment cases, but I also consider it a bit of an abuse of the ressource directory. I'm not sure we have a better mechanism at hand right now though.

[mgorny]

I'm sorry, I've completely forgot that the path contains version number. In this case, it indeed probably doesn't make much sense to add rpath for that.

[pirama]

lib/Driver/ToolChain.cpp

652

My bad. I'd have found about it if I actually sync to ToT more often than once a week.

lib/Driver/ToolChain.cpp

 
 void ToolChain::AddCXXStdlibLibArgs(const ArgList &Args,
                                     ArgStringList &CmdArgs) const {
   CXXStdlibType Type = GetCXXStdlibType(Args);
 
   switch (Type) {
   case ToolChain::CST_Libcxx:
     CmdArgs.push_back("-lc++");
+    // libc++ may be installed per arch.
+    addArchSpecificRPath(*this, Args, CmdArgs);
     break;
 
   case ToolChain::CST_Libstdcxx:
     CmdArgs.push_back("-lstdc++");
     break;
   }
 }