[CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

[Ericson2314]

Summary

A simple sed doing these substitutions:

• ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}\> -> ${LLVM_LIBRARY_DIR}
• ${LLVM_BINARY_DIR}/bin\> -> ${LLVM_TOOLS_BINARY_DIR}

where \> means "word boundary".

The only manual modifications were reverting changes in

• runtimes/CMakeLists.txt

because these were "entry points" where we wanted to tread carefully not not introduce a "loop" which would end with an undefined variable being expanded to nothing.

There are some ${LLVM_BINARY_DIR}/lib without the ${LLVM_LIBDIR_SUFFIX}, but these refer to the lib subdirectory of the source (llvm/lib). That lib is automatically appended to make the local CMAKE_CURRENT_BINARY_DIR value by add_subdirectory; since the directory name in the source tree is fixed without any suffix, the corresponding CMAKE_CURRENT_BINARY_DIR will also be. We therefore do not replace it but leave it as-is.

This picks up where D133828 left off, getting the occurrences with*out* CMAKE_CFG_INTDIR. But this is difficult to do correctly and so not done in the (retroactively) previous diff.

This hopefully increases readability overall, and also decreases the usages of LLVM_LIBDIR_SUFFIX, preparing us for D130586.

[sebastian-ne]

llvm/tools/llvm-shlib/CMakeLists.txt

89 ↗

(On Diff #454256)

The lib here is not used as a folder, so this should probably not be replaced?
It should probably include CMAKE_CFG_INTDIR though (i.e. ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/libllvm-c.exports), to be consistent with other places, though this only affects the ninja multi-config build. I’m not sure if that works at all and it’s outside of the scope of this patch anyway.

91 ↗

(On Diff #454256)

Should this be set(LIB_DIR ${LLVM_LIBRARY_DIR})?

139 ↗

(On Diff #454256)

This should probably use LLVM_LIBRARY_DIR as well.

[Ericson2314]

Redo sed mandating ending on word boundary

[Ericson2314]

llvm/tools/llvm-shlib/CMakeLists.txt

91 ↗

(On Diff #454256)

I am not sure what is up with CMAKE_CFG_INTDIR, so I rather leave that for a later revision.

139 ↗

(On Diff #454256)

Ditto, rather tackle things with CMAKE_CFG_INTDIR separately.

[sebastian-ne]

llvm/tools/llvm-shlib/CMakeLists.txt

91 ↗

(On Diff #454256)

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).

[Ericson2314]

Convert CMAKE_CFG_INTDIR as @sebastian-ne suggests, also .../bin

[Ericson2314]

llvm/tools/llvm-shlib/CMakeLists.txt

91 ↗

(On Diff #454256)

Oh I see. Silly me. That is a fine reason to convert then.

[Ericson2314]

https://lab.llvm.org/buildbot#builders/13/builds/24958 I am worried this failure is not spurious?

[sebastian-ne]

The build afterwards succeeded again. Seems like every ~20th build on that windows machine fails.

[Ericson2314]

Whew, thanks!

[CarlosAlbertoEnciso]

@Ericson2314 It seems that these changes break building on Windows using Visual Studio 2019.
There are 2 issues:

• CMake now creates a directory $(Configuration) in the root build directory.

Location before the changes: lib\cmake\llvm
Location after the changes:  $(Configuration)\lib\cmake\llvm

• The build fails with the errors:

Error C1083 Cannot open include file: 'PPCGenRegisterInfo.inc': No such file or directory	LLVMExegesisPowerPC	llvm-project\llvm\lib\Target\PowerPC\MCTargetDesc\PPCMCTargetDesc.h
Error C1083 Cannot open include file: 'MipsGenRegisterInfo.inc': No such file or directory	LLVMExegesisMips	llvm-project\llvm\lib\Target\Mips\MCTargetDesc\MipsMCTargetDesc.h
Error C1083 Cannot open include file: 'X86GenRegisterInfo.inc': No such file or directory	LLVMExegesisX86	        llvm-project\llvm\lib\Target\X86\MCTargetDesc\X86MCTargetDesc.h
Error C1083 Cannot open include file: 'AArch64GenRegisterInfo.inc': No such file or directory	LLVMExegesisAArch64	llvm-project\llvm\lib\Target\AArch64\MCTargetDesc\AArch64MCTargetDesc.h

[sebastian-ne]

This is probably caused by using CMAKE_CFG_INTDIR (indirectly) in more places. Seems like it expands to $(Configuration) for Visual Studio.
Searching more about it, it’s only set at build time, but not at install time, which is a problem as well.
On top of all, CMAKE_CFG_INTDIR is deprecated and superseded by generator expression: https://cmake.org/cmake/help/latest/variable/CMAKE_CFG_INTDIR.html

[gchatelet]

It also broke llvm-exegesis for 32 bits targets, see https://github.com/llvm/llvm-project/issues/57348
Reverting changes for the files in tools/llvm-exegesis/ solves the compilation issue.

[Ericson2314]

Let's just revert this. I'll do it later today if no one beats me to it.

[CarlosAlbertoEnciso]

In D132316#3748845, @Ericson2314 wrote:

Let's just revert this. I'll do it later today if no one beats me to it.

Thanks

[Ericson2314]

Rebase on top of D133828

[Ericson2314]

Require LLVM_LIBDIR_SUFFIX in the sed

[Ericson2314]

I think I fixed the issue: I was confusing /lib stemming from lib subdirs in the source and /lib stemming from building things in a lib directory that will later be installed also to a lib directory. It is just the latter case that uses the suffix, and so just the latter case will be so seded.

[sebastian-ne]

LGTM

clang/cmake/modules/CMakeLists.txt

set(CLANG_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/clang" CACHE STRING
   "Path for CMake subdirectory for Clang (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/clang')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
 set(clang_cmake_builddir "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/clang")
 
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(CLANG_EXPORTS GLOBAL PROPERTY CLANG_EXPORTS)
 export(TARGETS ${CLANG_EXPORTS} FILE ${clang_cmake_builddir}/ClangTargets.cmake)
 
 # Generate ClangConfig.cmake for the build tree.
 set(CLANG_CONFIG_CMAKE_DIR "${clang_cmake_builddir}")
 set(CLANG_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(CLANG_CONFIG_INCLUDE_EXPORTS "include(\"${clang_cmake_builddir}/ClangTargets.cmake\")")

flang/cmake/modules/CMakeLists.txt

 include(GNUInstallPackageDir)
 include(ExtendPath)
 include(FindPrefixFromConfig)
 
 # Generate a list of CMake library targets so that other CMake projects can
 # link against them. LLVM calls its version of this file LLVMExports.cmake, but
 # the usual CMake convention seems to be ${Project}Targets.cmake.
 set(FLANG_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/flang" CACHE STRING
   "Path for CMake subdirectory for Flang (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/flang')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
 set(flang_cmake_builddir "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/flang")
 
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(FLANG_EXPORTS GLOBAL PROPERTY FLANG_EXPORTS)
 export(TARGETS ${FLANG_EXPORTS} FILE ${flang_cmake_builddir}/FlangTargets.cmake)
 
 # Generate FlangConfig.cmake for the build tree.
 set(FLANG_CONFIG_CMAKE_DIR "${flang_cmake_builddir}")
 set(FLANG_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(FLANG_CONFIG_INCLUDE_EXPORTS "include(\"${flang_cmake_builddir}/FlangTargets.cmake\")")

lld/cmake/modules/CMakeLists.txt

 include(GNUInstallPackageDir)
 include(ExtendPath)
 include(FindPrefixFromConfig)
 
 # Generate a list of CMake library targets so that other CMake projects can
 # link against them. LLVM calls its version of this file LLVMExports.cmake, but
 # the usual CMake convention seems to be ${Project}Targets.cmake.
 set(LLD_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/lld" CACHE STRING
   "Path for CMake subdirectory for LLD (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/lld')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
 set(lld_cmake_builddir "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/lld")
 
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(LLD_EXPORTS GLOBAL PROPERTY LLD_EXPORTS)
 export(TARGETS ${LLD_EXPORTS} FILE ${lld_cmake_builddir}/LLDTargets.cmake)
 
 # Generate LLDConfig.cmake for the build tree.
 set(LLD_CONFIG_CMAKE_DIR "${lld_cmake_builddir}")
 set(LLD_CONFIG_LLVM_CMAKE_DIR "${llvm_cmake_builddir}")
 set(LLD_CONFIG_INCLUDE_EXPORTS "include(\"${lld_cmake_builddir}/LLDTargets.cmake\")")

lldb/cmake/modules/LLDBConfig.cmake

 # directory that LLDB is using for its embedded Clang instance needs to point
 # to the resource directory of the used Clang installation.
 if (NOT TARGET clang-resource-headers)
   set(LLDB_CLANG_RESOURCE_DIR_NAME "${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")
   # Iterate over the possible places where the external resource directory
   # could be and pick the first that exists.
   foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
                     "${LLVM_BUILD_LIBRARY_DIR}"
-                    "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+                    "${LLVM_LIBRARY_DIR}")
     # Build the resource directory path by appending 'clang/<version number>'.
     set(CANDIDATE_RESOURCE_DIR "${CANDIDATE}/clang/${LLDB_CLANG_RESOURCE_DIR_NAME}")
     if (IS_DIRECTORY "${CANDIDATE_RESOURCE_DIR}")
       set(LLDB_EXTERNAL_CLANG_RESOURCE_DIR "${CANDIDATE_RESOURCE_DIR}")
       break()
     endif()
   endforeach()
 

llvm/CMakeLists.txt

configure_file(
   ${LLVM_INCLUDE_DIR}/llvm/Config/Targets.def
   )
 configure_file(
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Config/TargetMCAs.def.in
   ${LLVM_INCLUDE_DIR}/llvm/Config/TargetMCAs.def
   )
 
 # They are not referenced. See set_output_directory().
-set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
-set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
-set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_TOOLS_BINARY_DIR} )
+set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
+set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
 
 if(LLVM_INCLUDE_TESTS)
   include(GetErrcMessages)
   get_errc_messages(LLVM_LIT_ERRC_MESSAGES)
 endif()
 
 # For up-to-date instructions for installing the TFLite dependency, refer to
 # the bot setup script: https://github.com/google/ml-compiler-opt/blob/main/buildbot/buildbot_init.sh

llvm/cmake/modules/AddLLVM.cmake

function(process_llvm_pass_plugins)
   if(ARG_GEN_CONFIG)
 
       ## Part 1: Extension header to be included whenever we need extension
       #  processing.
       if(NOT DEFINED LLVM_INSTALL_PACKAGE_DIR)
           message(FATAL_ERROR "LLVM_INSTALL_PACKAGE_DIR must be defined and writable. GEN_CONFIG should only be passe when building LLVM proper.")
       endif()
       # LLVM_INSTALL_PACKAGE_DIR might be absolute, so don't reuse below.
-      set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+      set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
       file(WRITE
           "${llvm_cmake_builddir}/LLVMConfigExtensions.cmake"
           "set(LLVM_STATIC_EXTENSIONS ${LLVM_STATIC_EXTENSIONS})")
       install(FILES
           ${llvm_cmake_builddir}/LLVMConfigExtensions.cmake
           DESTINATION ${LLVM_INSTALL_PACKAGE_DIR}
           COMPONENT cmake-exports)
 

llvm/cmake/modules/CMakeLists.txt

 include(ExtendPath)
 include(LLVMDistributionSupport)
 include(FindPrefixFromConfig)
 
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 # First for users who use an installed LLVM, create the LLVMExports.cmake file.
 set(LLVM_EXPORTS_FILE ${llvm_cmake_builddir}/LLVMExports.cmake)
 get_property(LLVM_EXPORTS GLOBAL PROPERTY LLVM_EXPORTS)
 export(TARGETS ${LLVM_EXPORTS} FILE ${LLVM_EXPORTS_FILE})
 
 # Then for users who want to link against the LLVM build tree, provide the
 # normal targets and the build tree only targets.

mlir/cmake/modules/CMakeLists.txt

set(MLIR_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/mlir" CACHE STRING
   "Path for CMake subdirectory for Polly (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/polly')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
 set(mlir_cmake_builddir "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/mlir")
 
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(MLIR_EXPORTS GLOBAL PROPERTY MLIR_EXPORTS)
 export(TARGETS ${MLIR_EXPORTS} FILE ${mlir_cmake_builddir}/MLIRTargets.cmake)
 
 get_property(MLIR_ALL_LIBS GLOBAL PROPERTY MLIR_ALL_LIBS)
 get_property(MLIR_DIALECT_LIBS GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 get_property(MLIR_CONVERSION_LIBS GLOBAL PROPERTY MLIR_CONVERSION_LIBS)
 get_property(MLIR_TRANSLATION_LIBS GLOBAL PROPERTY MLIR_TRANSLATION_LIBS)

polly/cmake/CMakeLists.txt

 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 
 include(GNUInstallPackageDir)
 include(ExtendPath)
 include(FindPrefixFromConfig)
 
 set(POLLY_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/polly" CACHE STRING
   "Path for CMake subdirectory for Polly (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/polly')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
 set(polly_cmake_builddir "${POLLY_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/polly")
 
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$<LOWER_CASE:$<CONFIG>>.cmake")
 else()
   # avoid conflicts in the build-tree when changing configuration
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-all.cmake")
 endif()
 

polly/test/CMakeLists.txt

if (POLLY_BUNDLED_ISL)
   list(APPEND POLLY_TEST_DEPS polly-isl-test)
 endif()
 if (POLLY_GTEST_AVAIL)
   list(APPEND POLLY_TEST_DEPS PollyUnitTests)
 endif ()
 
 set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
-set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+set(LLVM_LIBS_DIR "${LLVM_LIBRARY_DIR}")
 if (CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set(POLLY_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
 else()
   set(POLLY_LIB_DIR "${POLLY_BINARY_DIR}/lib")
 endif()
 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in