Ericson2314 marked an inline comment as not done.
Ericson2314 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()
----------------
phosek wrote:
> 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.
With D99697 now all approved (thanks again!) it looks like this is the next
step? Would be very nice if this could be removed!
================
Comment at: polly/cmake/CMakeLists.txt:82-96
+set(POLLY_INSTALL_PREFIX "")
set(POLLY_CONFIG_LLVM_CMAKE_DIR
"${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR
"${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS
"${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+set(POLLY_CONFIG_CMAKE_DIR
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
+set(POLLY_CONFIG_LIBRARY_DIRS
"${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}${LLVM_LIBDIR_SUFFIX}")
if (POLLY_BUNDLED_ISL)
set(POLLY_CONFIG_INCLUDE_DIRS
+ "${POLLY_INSTALL_PREFIX}${CMAKE_INSTALL_FULL_LIBDIR}"
----------------
`POLLY_INSTALL_PREFIX`, like `COMPILER_RT_INSTALL_PATH`, I changed to be empty
by default. If the `COMPILER_RT_INSTALL_PATH` can be removed, maybe
`POLLY_INSTALL_PREFIX` can also be removed?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
