[PATCH] D107799: [CMake] Enable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR by default on most platforms

Tue Aug 24 13:30:35 PDT 2021

phosek added a comment.

In D107799#2963311 <https://reviews.llvm.org/D107799#2963311>, @mstorsjo wrote:

> In D107799#2963198 <https://reviews.llvm.org/D107799#2963198>, @MaskRay wrote:
>
>> Ping:)
>>
>> I'll assume that @mstorsjo has fixed the Windows issues (or I can exclude Windows like Darwin).
>>
>> Apologies that we want to do this for Linux and can make it a bit inconvenient for Chrome OS (@manojgupta), but we will keep in mind the Linux support for the LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=off configuration should not retire.
>
> I've fixed some Windows issues (D107893 <https://reviews.llvm.org/D107893>) but others (D107892 <https://reviews.llvm.org/D107892> and D107895 <https://reviews.llvm.org/D107895>) are still stalled.
>
> The biggest blocker is not fixed in general though: Clang in general does not add the libc++ per-target include directory to its include path. In D89013 <https://reviews.llvm.org/D89013> this was added specifically only to the Gnu and Fuchsia drivers, and D107893 <https://reviews.llvm.org/D107893> added it for the mingw driver. Even for other ELF drivers, like e.g. FreeBSD, the per-target include directory isn't added. See e.g. https://github.com/llvm/llvm-project/blob/204038d52e03811cc39c18cc073c6020fbae4a4b/clang/lib/Driver/ToolChains/FreeBSD.cpp#L405-L409 vs https://github.com/llvm/llvm-project/blob/204038d52e03811cc39c18cc073c6020fbae4a4b/clang/lib/Driver/ToolChains/MinGW.cpp#L591-L602.

I'd prefer moving the header search logic for libc++ into `ToolChain`. There's a comment that says this logic should be handled in subclasses (https://github.com/llvm/llvm-project/blob/b97ca3aca12148bd6ddd0dd78d73003a92f8e553/clang/lib/Driver/ToolChain.cpp#L905) but that leads to a lot of duplication and I'm not sure whether that comment is still relevant. We already have the generic logic for finding the library in `ToolChain` so I don't see why we shouldn't have the header logic there as well. If some platforms need special handling, they can always override the method.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107799