[libcxx-commits] [PATCH] D107799: [CMake] Enable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR by default on Linux

[Tom Stellard via Phabricator via libcxx-commits]

tstellar added inline comments.


================
Comment at: llvm/CMakeLists.txt:674-679
+if(CMAKE_SYSTEM_NAME MATCHES "Linux")
+  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)
+else()
+  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default OFF)
+endif()
+set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default} CACHE BOOL
----------------
manojgupta wrote:
> phosek wrote:
> > manojgupta wrote:
> > > MaskRay wrote:
> > > > manojgupta wrote:
> > > > > Hmm, trying to understand the reason to make it default since users can already opt in. On Chrome OS, we'll have to disable it explictly (LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF ?)
> > > > For most Linux distributions, they need to do nothing.
> > > > 
> > > > `ninja asan` uses the new path. `clang -fsanitize=address` picks the new path. `ninja install` installs the new file. Every step is transparent.
> > > > If a distributor copies the whole `lib/clang/14.0.0`, no care is needed.
> > > > 
> > > > If a build system does something special, e.g. a runtime file may be used in an ad-hoc way bypassing Clang, some adjustment may be needed.
> > > > If the distributor doesn't want to adjustment, `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=off` is the simplest approach.
> > > > 
> > > > Does Chrome OS do some stuff with the runtime paths so that this needs to be disabled?
> > > We have some complications because of various build systems, mix of packages using clang/gcc etc. Not all packages are consistent in passing "-target <abi>" (often inside packages that do mixed host/target compiles). 
> > > In addition, we have been using the old layout since 2016 so various internal tools etc. are relying on the existing locations. Since (so far), we do not support multiple ABIs per ISA in Chrome OS, this has not been important and the cleanup will be long time consuming. We do customization per device using the device's sysroot e.g. install device specfic c++ etc. at /build/<device>/usr/lib.
> > @manojgupta Would having to use additional option be too much of an overhead? There are always going to be some exceptions that need additional customization like Chrome OS, but we trying to choose the right default that works for most projects.
> This change is not a problem since we can disable it. I was mostly letting the author know that that on Chrome OS we'll be disabling this.
Doesn't clang need to be updated to find the compiler-rt libs in the per-target dirs?  It currently only looks in CLANG_RESOURCE_DIR from what I can tell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107799