phosek added inline comments.
================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:389
get_compiler_rt_target(${arch} target)
- set(${install_dir} ${COMPILER_RT_INSTALL_PATH}/lib/${target} PARENT_SCOPE)
+ set(${install_dir}
${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target} PARENT_SCOPE)
else()
----------------
Ericson2314 wrote:
> ldionne wrote:
> > lebedev.ri wrote:
> > > This looks suspect
> > Yeah, I don't understand why this isn't just `CMAKE_INSTALL_LIBDIR` like
> > elsewhere.
> See the non-line comment I responded to @lebidev.ri with. In sort if
>
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
>
> is a relative path, then we end up with
>
> ```
> ${CMAKE_INSTALL_PREFIX}/${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
>
> instead of
>
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/${target}
> ```
>
> as we do with the other per-package prefixes. Also if `CMAKE_INSTALL_LIBDIR`
> is already an absolute path, then
> ```
> ${COMPILER_RT_INSTALL_PATH}/${CMAKE_INSTALL_FULL_LIBDIR}/${target}
> ```
> is the same thing, and closer to the second than the first.
I'm not sure if that's desirable. I'm not sure if we still need
`COMPILER_RT_INSTALL_PATH`. That variable is only used by
`clang/runtime/CMakeLists.txt` which predates `runtimes/CMakeLists.txt` and was
AFAIK only ever used by Apple. I think we should consider removing
`COMPILER_RT_INSTALL_PATH`. We'll need to check if
`clang/runtime/CMakeLists.txt` is still being used or not.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
