[cmake] Respect LLVM_RUNTIMES_LIBDIR_SUFFIX

[mgorny]

Summary

Respect the newly-introduced LLVM_RUNTIMES_LIBDIR_SUFFIX variable that
can be used to override the libdir used to hold LLVM runtimes. Install
clang runtimes (headers & compiler-rt) there, and use it when looking
them up.

[beanz]

This looks reasonable to me, but I'm not super familiar with multi-lib conventions. I think @chandlerc is more familiar with how that stuff works.

[mgorny]

A gentle ping.

[beanz]

Looking more closely at this, there is a problem that I see.

The clang runtime directory only supports building compiler-rt, and is going to be replaced by the llvm runtimes directory in the (hopefully near) future.

Maybe a better way to go is to define RUNTIMES_LIBDIR_SUFFIX in LLVM and connect that into both the clang runtime and llvm runtimes directories.

[mgorny]

Do you think I should just focus on the suffix, or maybe try to make the whole path configurable?

[beanz]

I think doing just the suffix is the right starting point.

[mgorny]

Here's the initial patch for the suggested approach. I'm still testing it though.

[mgorny]

lib/Driver/Tools.cpp

2021

Note: I don't know if it's better to use LLVM_RUNTIMES_LIBDIR_SUFFIX here, or just #define LLVM_LIBDIR_SUFFIX in addition to that.

[beanz]

This looks like it is in the direction I would expect. Please let me know once the final patch is ready.

lib/Driver/Tools.cpp

2021

This should be LLVM_LIBDIR_SUFFIX as per Chandler's comments.

[chandlerc]

I don't get it.

There is nothing "RUNTIMES" about it. And this variable is actually used to produce real libraries: libclang, etc.

It's just the LIBDIR_SUFFIX. That's it. The reason it is called CLANG_LIBDIR_SUFFIX and set from LLVM_LIBDIR_SUFFIX is to support using the CMake build of Clang in a standalone mode where there *isn't* a LLVM_LIBDIR_SUFFIX and instead the CLANG_LIBDIR_SUFFIX needs to be set directly.

So I'm really not understanding what benefit this change is bringing.

And the other comment in the CMake file that the CLANG_LIBDIR_SUFFIX must exactly match whatever LLVM_LIBDIR_SUFFIX is remains, AFAICT, 100% correct.

[mgorny]

In D23752#581416, @chandlerc wrote:

I don't get it.

There is nothing "RUNTIMES" about it. And this variable is actually used to produce real libraries: libclang, etc.

It's just the LIBDIR_SUFFIX. That's it. The reason it is called CLANG_LIBDIR_SUFFIX and set from LLVM_LIBDIR_SUFFIX is to support using the CMake build of Clang in a standalone mode where there *isn't* a LLVM_LIBDIR_SUFFIX and instead the CLANG_LIBDIR_SUFFIX needs to be set directly.

So I'm really not understanding what benefit this change is bringing.

And the other comment in the CMake file that the CLANG_LIBDIR_SUFFIX must exactly match whatever LLVM_LIBDIR_SUFFIX is remains, AFAICT, 100% correct.

That is 100% incorrect. LLVM_LIBDIR_SUFFIX is used almost everywhere, mostly because it goes implicitly through LLVM's CMake macros. CLANG_LIBDIR_SUFFIX is truly only used to set the paths in the sources.

[chandlerc]

In D23752#581427, @mgorny wrote:

In D23752#581416, @chandlerc wrote:

I don't get it.

There is nothing "RUNTIMES" about it. And this variable is actually used to produce real libraries: libclang, etc.

It's just the LIBDIR_SUFFIX. That's it. The reason it is called CLANG_LIBDIR_SUFFIX and set from LLVM_LIBDIR_SUFFIX is to support using the CMake build of Clang in a standalone mode where there *isn't* a LLVM_LIBDIR_SUFFIX and instead the CLANG_LIBDIR_SUFFIX needs to be set directly.

So I'm really not understanding what benefit this change is bringing.

And the other comment in the CMake file that the CLANG_LIBDIR_SUFFIX must exactly match whatever LLVM_LIBDIR_SUFFIX is remains, AFAICT, 100% correct.

That is 100% incorrect. LLVM_LIBDIR_SUFFIX is used almost everywhere, mostly because it goes implicitly through LLVM's CMake macros. CLANG_LIBDIR_SUFFIX is truly only used to set the paths in the sources.

OK, it used to be used in more exciting places.

If you want to go that route, the fix is just to s/CLANG_LIBDIR_SUFFIX/LLVM_LIBDIR_SUFFIX/.

[mgorny]

This really doesn't solve the problems we have. We don't want to build and install two almost-identical copies of compiler-rt and clang headers, when we can just build one and make clang use it.

[chandlerc]

In D23752#581436, @mgorny wrote:

This really doesn't solve the problems we have. We don't want to build and install two almost-identical copies of compiler-rt and clang headers, when we can just build one and make clang use it.

However, as I said before, I don't really think that this approach makes sense. I don't think we should be building Clang to re-use parts of some other Clang installation. I understand that is exactly what you want to do, but I disagree that it is a good idea. I think it is brittle and failure prone. I suggested ways you could make it work for your use cases however, but I don't think changing the CMake in this way is valid.

[mgorny]

Very well. I'll look into (ab)using the existing options to achieve the same result. Having stand-alone build systems, this should become easier these days, at least until some major change in runtime handling occurs.

CMakeLists.txt

 
 if(CLANG_REPOSITORY_STRING)
   add_definitions(-DCLANG_REPOSITORY_STRING="${CLANG_REPOSITORY_STRING}")
 endif()
 
 set(CLANG_VENDOR_UTI "org.llvm.clang" CACHE STRING
   "Vendor-specific uti.")
 
-# The libdir suffix must exactly match whatever LLVM's configuration used.
-set(CLANG_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}")
-
 set(CLANG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 set(CLANG_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+set(LLVM_RUNTIMES_LIBRARY_OUTPUT_INTDIR
+    ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_RUNTIMES_LIBDIR_SUFFIX})
 
 if( CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE )
   message(FATAL_ERROR "In-source builds are not allowed. CMake would overwrite "
 "the makefiles distributed with LLVM. Please create a directory and run cmake "
 "from there, passing the path to this source directory as the last argument. "
 "This process created the file `CMakeCache.txt' and the directory "
 "`CMakeFiles'. Please delete them.")
 endif()

include/clang/Config/config.h.cmake

 
 /* Default runtime library to use. */
 #define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"
 
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
 /* Multilib suffix for libdir. */
-#define CLANG_LIBDIR_SUFFIX "${CLANG_LIBDIR_SUFFIX}"
+#define LLVM_RUNTIMES_LIBDIR_SUFFIX "${LLVM_RUNTIMES_LIBDIR_SUFFIX}"
 
 /* Relative directory for resource files */
 #define CLANG_RESOURCE_DIR "${CLANG_RESOURCE_DIR}"
 
 /* Directories clang will search for headers */
 #define C_INCLUDE_DIRS "${C_INCLUDE_DIRS}"
 
 /* Default <path> to all compiler invocations for --sysroot=<path>. */

lib/Driver/Driver.cpp

Driver::Driver(StringRef ClangExecutable, StringRef DefaultTargetTriple,
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
   // Compute the path to the resource directory.
   StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
   SmallString<128> P(Dir);
   if (ClangResourceDir != "") {
     llvm::sys::path::append(P, ClangResourceDir);
   } else {
-    StringRef ClangLibdirSuffix(CLANG_LIBDIR_SUFFIX);
-    llvm::sys::path::append(P, "..", Twine("lib") + ClangLibdirSuffix, "clang",
+    StringRef RuntimeLibdirSuffix(LLVM_RUNTIMES_LIBDIR_SUFFIX);
+    llvm::sys::path::append(P, "..", Twine("lib") + RuntimeLibdirSuffix, "clang",
                             CLANG_VERSION_STRING);
   }
   ResourceDir = P.str();
 }
 
 Driver::~Driver() {
   delete Opts;
 

lib/Driver/Tools.cpp

 static void AddGoldPlugin(const ToolChain &ToolChain, const ArgList &Args,
                           ArgStringList &CmdArgs, bool IsThinLTO,
                           const Driver &D) {
   // Tell the linker to load the plugin. This has to come before AddLinkerInputs
   // as gold requires -plugin to come before any -plugin-opt that -Wl might
   // forward.
   CmdArgs.push_back("-plugin");
   std::string Plugin =
-      ToolChain.getDriver().Dir + "/../lib" CLANG_LIBDIR_SUFFIX "/LLVMgold.so";
+      ToolChain.getDriver().Dir + "/../lib" LLVM_RUNTIMES_LIBDIR_SUFFIX "/LLVMgold.so";
   CmdArgs.push_back(Args.MakeArgString(Plugin));
 
   // Try to pass driver level flags relevant to LTO code generation down to
   // the plugin.
 
   // Handle flags for selecting CPU variants.
   std::string CPU = getCPUName(Args, ToolChain.getTriple());
   if (!CPU.empty())

lib/Frontend/CompilerInvocation.cpp

std::string CompilerInvocation::GetResourcesPath(const char *Argv0,
   StringRef Dir = llvm::sys::path::parent_path(ClangExecutable);
 
   // Compute the path to the resource directory.
   StringRef ClangResourceDir(CLANG_RESOURCE_DIR);
   SmallString<128> P(Dir);
   if (ClangResourceDir != "")
     llvm::sys::path::append(P, ClangResourceDir);
   else
-    llvm::sys::path::append(P, "..", Twine("lib") + CLANG_LIBDIR_SUFFIX,
+    llvm::sys::path::append(P, "..", Twine("lib") + LLVM_RUNTIMES_LIBDIR_SUFFIX,
                             "clang", CLANG_VERSION_STRING);
 
   return P.str();
 }
 
 static void ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args) {
   using namespace options;
   Opts.Sysroot = Args.getLastArgValue(OPT_isysroot, "/");

lib/Headers/CMakeLists.txt

set(files
   )
 
 set(cuda_wrapper_files
   cuda_wrappers/algorithm
   cuda_wrappers/complex
   cuda_wrappers/new
 )
 
-set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
+set(output_dir ${LLVM_RUNTIMES_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
 
 # Generate arm_neon.h
 clang_tablegen(arm_neon.h -gen-arm-neon
   SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_neon.td)
 
 set(out_files)
 foreach( f ${files} ${cuda_wrapper_files} )
   set( src ${CMAKE_CURRENT_SOURCE_DIR}/${f} )

runtime/CMakeLists.txt

ExternalProject_Add(compiler-rt
     BINARY_DIR ${BINARY_DIR}
     CMAKE_ARGS ${CLANG_COMPILER_RT_CMAKE_ARGS}
                -DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang
                -DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang++
                -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
                -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
                -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
                -DLLVM_LIT_ARGS=${LLVM_LIT_ARGS}
-               -DCOMPILER_RT_OUTPUT_DIR=${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}
+               -DCOMPILER_RT_OUTPUT_DIR=${LLVM_RUNTIMES_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}
                -DCOMPILER_RT_EXEC_OUTPUT_DIR=${LLVM_RUNTIME_OUTPUT_INTDIR}
-               -DCOMPILER_RT_INSTALL_PATH:STRING=lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}
+               -DCOMPILER_RT_INSTALL_PATH:STRING=lib${LLVM_RUNTIMES_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}
                -DCOMPILER_RT_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
                -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
                -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}
                ${COMPILER_RT_PASSTHROUGH_VARIABLES}
     INSTALL_COMMAND ""
     STEP_TARGETS configure build
     USES_TERMINAL_CONFIGURE 1
     USES_TERMINAL_BUILD 1