[PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

Tue Aug 23 15:10:41 PDT 2022

Ericson2314 marked 3 inline comments as done.
Ericson2314 added inline comments.

================
Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:91

   set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)
----------------
sebastian-ne wrote:
> Ericson2314 wrote:
> > sebastian-ne wrote:
> > > Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?
> > I am not sure what is up with `CMAKE_CFG_INTDIR`, so I rather leave that for a later revision.
> `LLVM_LIBRARY_DIR` includes `CMAKE_CFG_INTDIR` as it’s set through
> ```
> set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
> set(LLVM_LIBRARY_DIR      ${LLVM_LIBRARY_OUTPUT_INTDIR})
> ```
> 
> As far as I understand, `CMAKE_CFG_INTDIR` is set to `.`, unless one does a multi-config build, i.e. one build directory for debug and release builds. multi-config builds are the default when creating Visual Studio solutions and ninja supports multi-config builds since some time (rather recently).
Oh I see. Silly me. That is a fine reason to convert then.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316