Skip to content

Conversation

@bgergely0
Copy link
Contributor

@bgergely0 bgergely0 commented Jul 30, 2024

This PR aims to fix a problem where the user is not able to instrument an aarch64 binary from az x86_64 host machine, because the runtime libraries are only built for the host architecture.

The fix checks if the BOLT_RT_TARGETs are supported by the compiler (both clang/GNU compilers), and compiles libbolt_rt_inst.a and libbolt_rt_hugify.a for supported architectures. The new libraries are placed under lib/Target/<target>/.
The change in the default path for the libraries is also reflected in the BOLT source code. If the user did not specify a custom location for the libs using either --runtime-instrumentation-lib or --runtime-hugify-lib, the default path is modified for the new version, taking into account the architecture of BinaryContext.

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@bgergely0
Copy link
Contributor Author

Hi @paschalis-mpeis @peterwaller-arm @yota9 ,
I suspect the automatic checks are not passing because this change requires cross-compilers to be installed, but they are not.
Any feedback on the code would be appreciated!
Thanks,
Gergely

Copy link
Contributor

@peterwaller-arm peterwaller-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes seem somewhat reasonable to me.

However:

  • I think this should gracefully degrade to the current behaviour if cross compilers are unavailable. So perhaps it is necessary to test if the cross compiler can be executed, and if not, fall back to not building the runtime for that target. That would also get the CI passing.
    • I'm assuming here that it's still useful to run bolt for targets where the runtimes are not available. It just means certain functionality would be unavailable, but this could still be useful in various testing scenarios.
  • I think the cross compilers should be configurable with BOLT_RT_C_COMPILER_${tgt} or similar. It seems reasonable for them to default to the values you have specified. I wondered if there was a general way to specify a cross compiler for a given with cmake but I didn't find one, I only so far found reference to specifying CMAKE_C_COMPILER and friends assuming you are only building for a single target within a given cmake invocation.
  • ( if clang is available, and built with the requested targets, it should be possible to use clang -target <triple> instead, and therefore not require the user to install a cross compiler so long as they have clang available. That said I see the runtimes use #include <cstddef> and friends so this is unlikely to work unless using a standard cross compiler install or a cross compiler configured to find the headers for the target, so I guess this approach is not going to be simple to make work)

Then coming to the point about CI and regular testing of this, I don't know who maintains that CI and whether they could be persuaded to install cross compilers there.

@llvmbot
Copy link
Member

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-bolt

Author: Gergely Bálint (bgergely0)

Changes

This PR aims to fix a problem where the user is not able to instrument an aarch64 binary from az x86_64 host machine, because the runtime libraries are only built for the host architecture.

The fix creates the AArch64 directory under lib, and generates the libbolt_rt_inst.a and libbolt_rt_hugify.a libraries under it. The user can specify which runtime library to use with the appropriate command line flag.


Full diff: https://github.com/llvm/llvm-project/pull/101180.diff

8 Files Affected:

  • (modified) bolt/CMakeLists.txt (+160-29)
  • (added) bolt/cmake/test/test.cpp (+3)
  • (modified) bolt/include/bolt/RuntimeLibs/RuntimeLibrary.h (+6-1)
  • (modified) bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp (+8-1)
  • (modified) bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp (+9-1)
  • (modified) bolt/lib/RuntimeLibs/RuntimeLibrary.cpp (+19-1)
  • (modified) bolt/runtime/CMakeLists.txt (+5-4)
  • (modified) bolt/tools/driver/CMakeLists.txt (+3-1)
diff --git a/bolt/CMakeLists.txt b/bolt/CMakeLists.txt
index 9f5875dd212847..c1873ece511aa0 100644
--- a/bolt/CMakeLists.txt
+++ b/bolt/CMakeLists.txt
@@ -84,8 +84,7 @@ set(BOLT_ENABLE_RUNTIME_default OFF)
 if ((CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64"
     OR CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64")
     AND (CMAKE_SYSTEM_NAME STREQUAL "Linux"
-      OR CMAKE_SYSTEM_NAME STREQUAL "Darwin")
-    AND (NOT CMAKE_CROSSCOMPILING))
+      OR CMAKE_SYSTEM_NAME STREQUAL "Darwin"))
   set(BOLT_ENABLE_RUNTIME_default ON)
 endif()
 option(BOLT_ENABLE_RUNTIME "Enable BOLT runtime" ${BOLT_ENABLE_RUNTIME_default})
@@ -135,36 +134,168 @@ if (LLVM_INCLUDE_TESTS)
   endif()
 endif()
 
-if (BOLT_ENABLE_RUNTIME)
-  message(STATUS "Building BOLT runtime libraries for X86")
-  set(extra_args "")
-  if(CMAKE_SYSROOT)
-    list(APPEND extra_args -DCMAKE_SYSROOT=${CMAKE_SYSROOT})
+set(AARCH64_GNU_C_COMPILER aarch64-linux-gnu-gcc)
+set(AARCH64_GNU_CXX_COMPILER aarch64-linux-gnu-g++)
+set(X86_64_GNU_C_COMPILER x86_64-linux-gnu-gcc)
+set(X86_64_GNU_CXX_COMPILER x86_64-linux-gnu-g++)
+set(RISCV_GNU_C_COMPILER riscv64-linux-gnu-gcc)
+set(RISCV_GNU_CXX_COMPILER riscv64-linux-gnu-g++)
+
+function(bolt_rt_target_supported_clang target supported)
+
+    if(${target} STREQUAL ${HOST_NAME})
+      set(${supported} TRUE PARENT_SCOPE)
+      return()
+    elseif(${target} STREQUAL "X86")
+      set(CMAKE_CXX_FLAGS "--target=x86_64-linux-gnu")
+    elseif(${target} STREQUAL "AArch64")
+      set(CMAKE_CXX_FLAGS "--target=aarch64-linux-gnu")
+    elseif(${target} STREQUAL "RISCV")
+      set(CMAKE_CXX_FLAGS "--target=riscv64-linux-gnu")
+    endif()
+
+    try_compile(CROSS_COMP
+      ${CMAKE_BINARY_DIR}/CMakeFiles/CMakeScratch
+      SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/test/test.cpp
+      CMAKE_FLAGS ${CMAKE_CXX_FLAGS}
+      TRY_COMP_OUTPUT)
+
+  if(CROSS_COMP)
+    message(STATUS "cross compilation test for ${target} was successful")
+    set(${supported} TRUE PARENT_SCOPE)
+  else()
+    message(STATUS "cross compilation test for ${target} was NOT successful")
+    set(${supported} FALSE PARENT_SCOPE)
   endif()
 
-  include(ExternalProject)
-  ExternalProject_Add(bolt_rt
-    SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/runtime"
-    STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-stamps
-    BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins
-    CMAKE_ARGS -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
-               -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
-               -DCMAKE_BUILD_TYPE=Release
-               -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-               -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}
-               -DLLVM_LIBRARY_DIR=${LLVM_LIBRARY_DIR}
-               -DBOLT_BUILT_STANDALONE=${BOLT_BUILT_STANDALONE}
-               ${extra_args}
-    INSTALL_COMMAND ""
-    BUILD_ALWAYS True
+endfunction()
+
+
+function(bolt_rt_target_supported_gnu target supported)
+
+    if(${target} STREQUAL ${HOST_NAME})
+      set(${supported} TRUE PARENT_SCOPE)
+      return()
+    elseif(${target} STREQUAL "X86")
+      find_program(CC_EXECUTABLE  NAMES ${X86_64_GNU_C_COMPILER} NO_CACHE)
+      find_program(CXX_EXECUTABLE NAMES ${X86_64_GNU_CXX_COMPILER} NO_CACHE)
+    elseif(${target} STREQUAL "AArch64")
+      find_program(CC_EXECUTABLE  NAMES ${AARCH64_GNU_C_COMPILER} NO_CACHE)
+      find_program(CXX_EXECUTABLE NAMES ${AARCH64_GNU_CXX_COMPILER} NO_CACHE)
+    elseif(${target} STREQUAL "RISCV")
+      find_program(CC_EXECUTABLE  NAMES ${RISCV_GNU_C_COMPILER} NO_CACHE)
+      find_program(CXX_EXECUTABLE NAMES ${RISCV_GNU_CXX_COMPILER} NO_CACHE)
+    endif()
+
+    if(CC_EXECUTABLE AND CXX_EXECUTABLE)
+      set(${supported} TRUE PARENT_SCOPE)
+    else()
+      set(${supported} FALSE PARENT_SCOPE)
+    endif()
+
+endfunction()
+
+
+if(BOLT_ENABLE_RUNTIME)
+  if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64")
+    set(HOST_NAME "X86")
+  elseif(CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64")
+    set(HOST_NAME "AArch64")
+  elseif(CMAKE_SYSTEM_PROCESSOR STREQUAL "riscv64")
+    set(HOST_NAME "RISCV")
+  endif()
+
+  # Further filter BOLT runtime targets: check if the runtime can be compiled
+  set(BOLT_RT_TARGETS_TO_BUILD)
+  foreach(tgt ${BOLT_TARGETS_TO_BUILD})
+
+    if(CMAKE_C_COMPILER_ID MATCHES ".*Clang.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*Clang.*")
+      bolt_rt_target_supported_clang(${tgt} supported)
+
+    elseif(CMAKE_C_COMPILER_ID MATCHES ".*GNU.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*GNU.*")
+      bolt_rt_target_supported_gnu(${tgt} supported)
+    endif()
+
+    if(${supported})
+      list(APPEND BOLT_RT_TARGETS_TO_BUILD ${tgt})
+    endif()
+
+  endforeach()
+endif()
+
+if(BOLT_ENABLE_RUNTIME)
+
+  foreach(tgt ${BOLT_RT_TARGETS_TO_BUILD})
+    message(STATUS "Building BOLT runtime libraries for ${tgt}")
+    set(extra_args "")
+    if(CMAKE_SYSROOT)
+      list(APPEND extra_args -DCMAKE_SYSROOT=${CMAKE_SYSROOT})
+    endif()
+
+    # set up paths: target-specific libs will be generated under lib/Target/${tgt}/
+    set(BOLT_RT_LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
+    set(SUBDIR "${tgt}")
+    cmake_path(APPEND BOLT_RT_LIBRARY_DIR ${BOLT_RT_LIBRARY_DIR} "Target" ${SUBDIR})
+
+    # set up compilers and flags
+    if(CMAKE_C_COMPILER_ID MATCHES ".*Clang.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*Clang.*")
+      set(BOLT_RT_C_COMPILER ${CMAKE_C_COMPILER})
+      set(BOLT_RT_CXX_COMPILER ${CMAKE_CXX_COMPILER})
+
+      if(${tgt} STREQUAL ${HOST_NAME})
+          set(BOLT_RT_FLAGS)
+      elseif(${tgt} STREQUAL "AArch64")
+          set(BOLT_RT_FLAGS "--target=aarch64-linux-gnu")
+      elseif(${tgt} STREQUAL "X86")
+          set(BOLT_RT_FLAGS "--target=x86_64-linux-gnu")
+      elseif(${tgt} STREQUAL "RISCV")
+          set(BOLT_RT_FLAGS "--target=riscv64-linux-gnu")
+      endif()
+
+    elseif(CMAKE_C_COMPILER_ID MATCHES ".*GNU.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*GNU.*")
+        set(BOLT_RT_FLAGS)
+      if(${tgt} STREQUAL ${HOST_NAME})
+        set(BOLT_RT_C_COMPILER ${CMAKE_C_COMPILER})
+        set(BOLT_RT_CXX_COMPILER ${CMAKE_CXX_COMPILER})
+      elseif(${tgt} STREQUAL "AArch64")
+        set(BOLT_RT_C_COMPILER ${AARCH64_GNU_C_COMPILER})
+        set(BOLT_RT_CXX_COMPILER ${AARCH64_GNU_CXX_COMPILER})
+      elseif(${tgt} STREQUAL "X86")
+        set(BOLT_RT_C_COMPILER ${X86_64_GNU_C_COMPILER})
+        set(BOLT_RT_CXX_COMPILER ${X86_64_GNU_CXX_COMPILER})
+      elseif(${tgt} STREQUAL "RISCV")
+        set(BOLT_RT_C_COMPILER ${RISCV_GNU_C_COMPILER})
+        set(BOLT_RT_CXX_COMPILER ${RISCV_GNU_CXX_COMPILER})
+      endif()
+    endif()
+
+
+    include(ExternalProject)
+    ExternalProject_Add(bolt_rt_${tgt}
+      SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/runtime"
+      STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-${tgt}-stamps
+      BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-${tgt}-bins
+      CMAKE_ARGS
+      -DCMAKE_C_COMPILER=${BOLT_RT_C_COMPILER}
+      -DCMAKE_CXX_COMPILER=${BOLT_RT_CXX_COMPILER}
+      -DCMAKE_BUILD_TYPE=Release
+      -DBOLT_RT_FLAGS=${BOLT_RT_FLAGS}
+      -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
+      -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}
+      -DLLVM_LIBRARY_DIR=${BOLT_RT_LIBRARY_DIR}
+      -DBOLT_BUILT_STANDALONE=${BOLT_BUILT_STANDALONE}
+      ${extra_args}
+      INSTALL_COMMAND ""
+      BUILD_ALWAYS True
     )
-  install(CODE "execute_process\(COMMAND \${CMAKE_COMMAND} -DCMAKE_INSTALL_PREFIX=\${CMAKE_INSTALL_PREFIX} -P ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins/cmake_install.cmake \)"
-    COMPONENT bolt)
-  add_llvm_install_targets(install-bolt_rt
-    DEPENDS bolt_rt bolt
-    COMPONENT bolt)
-  set(LIBBOLT_RT_INSTR "${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins/lib/libbolt_rt_instr.a")
-  set(LIBBOLT_RT_HUGIFY "${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins/lib/libbolt_rt_hugify.a")
+    install(CODE "execute_process\(COMMAND \${CMAKE_COMMAND} -DCMAKE_INSTALL_PREFIX=\${CMAKE_INSTALL_PREFIX} -P ${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-${tgt}-bins/cmake_install.cmake \)"
+      COMPONENT bolt)
+    add_llvm_install_targets(install-bolt_rt_${tgt}
+      DEPENDS bolt_rt_${tgt} bolt
+      COMPONENT bolt)
+    set(LIBBOLT_RT_INSTR "${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins/lib/libbolt_rt_instr.a")
+    set(LIBBOLT_RT_HUGIFY "${CMAKE_CURRENT_BINARY_DIR}/bolt_rt-bins/lib/libbolt_rt_hugify.a")
+  endforeach()
 endif()
 
 find_program(GNU_LD_EXECUTABLE NAMES ${LLVM_DEFAULT_TARGET_TRIPLE}-ld.bfd ld.bfd DOC "GNU ld")
diff --git a/bolt/cmake/test/test.cpp b/bolt/cmake/test/test.cpp
new file mode 100644
index 00000000000000..4e43f4be139592
--- /dev/null
+++ b/bolt/cmake/test/test.cpp
@@ -0,0 +1,3 @@
+#include <stdio.h>
+
+int main() { return 0; }
diff --git a/bolt/include/bolt/RuntimeLibs/RuntimeLibrary.h b/bolt/include/bolt/RuntimeLibs/RuntimeLibrary.h
index fc1db7369eb4a3..cc7e3f78264dad 100644
--- a/bolt/include/bolt/RuntimeLibs/RuntimeLibrary.h
+++ b/bolt/include/bolt/RuntimeLibs/RuntimeLibrary.h
@@ -61,14 +61,19 @@ class RuntimeLibrary {
   /// Get the full path to a runtime library specified by \p LibFileName and \p
   /// ToolPath.
   static std::string getLibPathByToolPath(StringRef ToolPath,
+                                          StringRef ToolSubPath,
                                           StringRef LibFileName);
 
+  /// Create architecture-specific ToolSubPath to be used in the full path
+  static std::string createToolSubPath(StringRef ArchName);
+
   /// Get the full path to a runtime library by the install directory.
   static std::string getLibPathByInstalled(StringRef LibFileName);
 
   /// Gets the full path to a runtime library based on whether it exists
   /// in the install libdir or runtime libdir.
-  static std::string getLibPath(StringRef ToolPath, StringRef LibFileName);
+  static std::string getLibPath(StringRef ToolPath, StringRef ToolSubPath,
+                                StringRef LibFileName);
 
   /// Load a static runtime library specified by \p LibPath.
   static void loadLibrary(StringRef LibPath, BOLTLinker &Linker,
diff --git a/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
index 026f8d35c55c63..fd8172fb2932fc 100644
--- a/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/HugifyRuntimeLibrary.cpp
@@ -63,7 +63,14 @@ void HugifyRuntimeLibrary::link(BinaryContext &BC, StringRef ToolPath,
                                 BOLTLinker &Linker,
                                 BOLTLinker::SectionsMapper MapSections) {
 
-  std::string LibPath = getLibPath(ToolPath, opts::RuntimeHugifyLib);
+  // If the default filename is selected, add architecture-specific Target
+  // subdirectory to it.
+  std::string ToolSubPath = "";
+  if (opts::RuntimeHugifyLib == "libbolt_rt_hugify.a") {
+    ToolSubPath = createToolSubPath(BC.TheTriple->getArchName().str().c_str());
+  }
+  std::string LibPath =
+      getLibPath(ToolPath, ToolSubPath, opts::RuntimeHugifyLib);
   loadLibrary(LibPath, Linker, MapSections);
 
   assert(!RuntimeStartAddress &&
diff --git a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
index 53a0c811b41d58..a16529e68376de 100644
--- a/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/InstrumentationRuntimeLibrary.cpp
@@ -197,7 +197,15 @@ void InstrumentationRuntimeLibrary::emitBinary(BinaryContext &BC,
 void InstrumentationRuntimeLibrary::link(
     BinaryContext &BC, StringRef ToolPath, BOLTLinker &Linker,
     BOLTLinker::SectionsMapper MapSections) {
-  std::string LibPath = getLibPath(ToolPath, opts::RuntimeInstrumentationLib);
+
+  // If the default filename is selected, add architecture-specific Target
+  // subdirectory to it.
+  std::string ToolSubPath = "";
+  if (opts::RuntimeInstrumentationLib == "libbolt_rt_instr.a") {
+    ToolSubPath = createToolSubPath(BC.TheTriple->getArchName().str().c_str());
+  }
+  std::string LibPath =
+      getLibPath(ToolPath, ToolSubPath, opts::RuntimeInstrumentationLib);
   loadLibrary(LibPath, Linker, MapSections);
 
   if (BC.isMachO())
diff --git a/bolt/lib/RuntimeLibs/RuntimeLibrary.cpp b/bolt/lib/RuntimeLibs/RuntimeLibrary.cpp
index 336c6768a7f712..878753eea6b554 100644
--- a/bolt/lib/RuntimeLibs/RuntimeLibrary.cpp
+++ b/bolt/lib/RuntimeLibs/RuntimeLibrary.cpp
@@ -27,6 +27,7 @@ using namespace bolt;
 void RuntimeLibrary::anchor() {}
 
 std::string RuntimeLibrary::getLibPathByToolPath(StringRef ToolPath,
+                                                 StringRef ToolSubPath,
                                                  StringRef LibFileName) {
   StringRef Dir = llvm::sys::path::parent_path(ToolPath);
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
@@ -37,10 +38,26 @@ std::string RuntimeLibrary::getLibPathByToolPath(StringRef ToolPath,
     LibPath = llvm::sys::path::parent_path(llvm::sys::path::parent_path(Dir));
     llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
   }
+  llvm::sys::path::append(LibPath, ToolSubPath);
+
   llvm::sys::path::append(LibPath, LibFileName);
   return std::string(LibPath);
 }
 
+std::string RuntimeLibrary::createToolSubPath(StringRef ArchName) {
+  std::string ToolSubPath = "";
+  if (ArchName == "x86_64")
+    ToolSubPath = "Target/X86";
+  else if (ArchName == "aarch64")
+    ToolSubPath = "Target/AArch64";
+  else if (ArchName == "riscv64")
+    ToolSubPath = "Target/RISCV";
+  else
+    llvm_unreachable("Unsupported architecture");
+
+  return ToolSubPath;
+}
+
 std::string RuntimeLibrary::getLibPathByInstalled(StringRef LibFileName) {
   SmallString<128> LibPath(CMAKE_INSTALL_FULL_LIBDIR);
   llvm::sys::path::append(LibPath, LibFileName);
@@ -48,12 +65,13 @@ std::string RuntimeLibrary::getLibPathByInstalled(StringRef LibFileName) {
 }
 
 std::string RuntimeLibrary::getLibPath(StringRef ToolPath,
+                                       StringRef ToolSubPath,
                                        StringRef LibFileName) {
   if (llvm::sys::fs::exists(LibFileName)) {
     return std::string(LibFileName);
   }
 
-  std::string ByTool = getLibPathByToolPath(ToolPath, LibFileName);
+  std::string ByTool = getLibPathByToolPath(ToolPath, ToolSubPath, LibFileName);
   if (llvm::sys::fs::exists(ByTool)) {
     return ByTool;
   }
diff --git a/bolt/runtime/CMakeLists.txt b/bolt/runtime/CMakeLists.txt
index 40f4fbc9f30d54..2bf46e9f397fdb 100644
--- a/bolt/runtime/CMakeLists.txt
+++ b/bolt/runtime/CMakeLists.txt
@@ -29,24 +29,25 @@ if(NOT BOLT_BUILT_STANDALONE)
   add_custom_command(TARGET bolt_rt_hugify POST_BUILD
     COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_CURRENT_BINARY_DIR}/lib/libbolt_rt_hugify.a" "${LLVM_LIBRARY_DIR}")
 endif()
-
-set(BOLT_RT_FLAGS
+# In case of compiling with clang, the '--target' option is passed in BOLT_RT_FLAGS.
+set(BOLT_RT_FLAGS ${BOLT_RT_FLAGS}
   -ffreestanding
   -fno-exceptions
   -fno-rtti
   -fno-stack-protector
   -fPIC
   -mgeneral-regs-only)
-if (CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64")
+if(CMAKE_TARGET_SYSTEM_PROCESSOR STREQUAL "x86_64")
   set(BOLT_RT_FLAGS ${BOLT_RT_FLAGS} "-mno-sse")
 endif()
-if (CMAKE_SYSTEM_PROCESSOR MATCHES "aarch64")
+if(CMAKE_TARGET_SYSTEM_PROCESSOR STREQUAL "aarch64")
   check_cxx_compiler_flag("-mno-outline-atomics" CXX_SUPPORTS_OUTLINE_ATOMICS)
   if (CXX_SUPPORTS_OUTLINE_ATOMICS)
     set(BOLT_RT_FLAGS ${BOLT_RT_FLAGS} "-mno-outline-atomics")
   endif()
 endif()
 
+
 # Don't let the compiler think it can create calls to standard libs
 target_compile_options(bolt_rt_instr PRIVATE ${BOLT_RT_FLAGS})
 target_include_directories(bolt_rt_instr PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
diff --git a/bolt/tools/driver/CMakeLists.txt b/bolt/tools/driver/CMakeLists.txt
index 9bf9ff85edc7b4..a06fa90b268fa0 100644
--- a/bolt/tools/driver/CMakeLists.txt
+++ b/bolt/tools/driver/CMakeLists.txt
@@ -6,7 +6,9 @@ set(LLVM_LINK_COMPONENTS
   )
 
 if (BOLT_ENABLE_RUNTIME)
-  set(BOLT_DRIVER_DEPS "bolt_rt")
+  foreach(tgt ${BOLT_RT_TARGETS_TO_BUILD})
+    set(BOLT_DRIVER_DEPS ${BOLT_DRIVER_DEPS} "bolt_rt_${tgt}")
+  endforeach()
 else()
   set(BOLT_DRIVER_DEPS "")
 endif()

Copy link
Contributor

@peterwaller-arm peterwaller-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update; here are some more comments.

Please could you update the pull request body/commit to illustrate how you're moving the runtime libraries so that it's clear to anyone who sees this? i.e. Just put lib/libbolt_rt_instr.a -> lib/Target/<target>/libbolt_rt_instr.a there.

Speaking of which, I wonder if lib/<target>/<lib> would be sufficient (perhaps having /Target/ is redundant?

@bgergely0
Copy link
Contributor Author

bgergely0 commented Oct 3, 2024

Speaking of which, I wonder if lib/<target>/<lib> would be sufficient (perhaps having /Target/ is redundant?

In the original version I just had lib/<target> as the new path, but I realized there are already target-specific files under lib/Target/<target> so it felt like the right place to store the new files.

@peterwaller-arm
Copy link
Contributor

I realized there are already target-specific files under lib/Target/<target> so it felt like the right place to store the new files.

My read of this (correct me if I'm wrong) is that lib/Target/<target> are directories which exist in the sources. I think traditionally the install directories of library archives like this would live at lib/<target> rather than lib/Target/<target> -- sources and builds are different.

@bgergely0
Copy link
Contributor Author

bgergely0 commented Oct 3, 2024

I realized there are already target-specific files under lib/Target/<target> so it felt like the right place to store the new files.

My read of this (correct me if I'm wrong) is that lib/Target/<target> are directories which exist in the sources. I think traditionally the install directories of library archives like this would live at lib/<target> rather than lib/Target/<target> -- sources and builds are different.

No, these exist in the build directory as well. I am following the build steps from the BOLT README, so my build dir is different from the sources. The build/lib/Target/<target> directory contains a lot of TableGenerated files, and other target-specific object files.

@peterwaller-arm
Copy link
Contributor

No, these exist in the build directory as well.

Build directory layout mirrors source directory layout. But what about the install directory layout? (set -DCMAKE_INSTALL_PREFIX and do ninja install). I'm talking about installation, and I think Target is probably not wanted there.

@bgergely0
Copy link
Contributor Author

Build directory layout mirrors source directory layout. But what about the install directory layout? (set -DCMAKE_INSTALL_PREFIX and do ninja install). I'm talking about installation, and I think Target is probably not wanted there.

Yes, you are correct, I missed the install step.

    This patch builds libbolt_rt_inst.a and libbolt_rt_hugify.a
    for each target architecture. This enables using the --instrument
    and --hugify options on binaries with different architecture from
    the host.
    The patch also changes the default locations searched for the libs.
Copy link
Contributor

@aaupov aaupov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this can't be merged in the current state.

LLVM project does runtime libraries build differently, please check runtimes folder for more details. We don't want to introduce dependencies on host cross-compilers all the while Clang can be used for that purpose. We can add the dependency on Clang for cross-target runtimes build if necessary, as we already do for BOLT tests.

CC @petrhosek

@bgergely0
Copy link
Contributor Author

bgergely0 commented Oct 16, 2024

We don't want to introduce dependencies on host cross-compilers all the while Clang can be used for that purpose. We can add the dependency on Clang for cross-target runtimes build if necessary, as we already do for BOLT tests.

@aaupov @petrhosek

Hi!
I'll check runtimes more thorougly, but I am not sure what you mean by introducing dependencies on host cross-compilers. My code only allows using other cross compilers if they are available (and clang isn't), it is not a dependency. By default we expect the user to use clang for cross compilation.
Any dependence on host cross compilers is after an elseif:

    # set up compilers and flags
    if(CMAKE_C_COMPILER_ID MATCHES ".*Clang.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*Clang.*")
      set(BOLT_RT_C_COMPILER ${CMAKE_C_COMPILER})
      set(BOLT_RT_CXX_COMPILER ${CMAKE_CXX_COMPILER})
     # [...] setting up --target flags for clang

    elseif(CMAKE_C_COMPILER_ID MATCHES ".*GNU.*" AND CMAKE_CXX_COMPILER_ID MATCHES ".*GNU.*")
      set(BOLT_RT_FLAGS)
      # [...] setting up cross compilers
      endif()
    endif()

Could you clarify what do you mean by dependency?
Thanks!

@bgergely0
Copy link
Contributor Author

Hi @aaupov,

I have removed gcc cross-compilers all together. Let me know if this is what you meant, or other changes you think are needed.
Thanks!

@bgergely0
Copy link
Contributor Author

@aaupov ping

@tdusnoki
Copy link
Contributor

This is just my informal feedback: I've tested it and it works as intended. Thanks for this PR, useful feature to have.

@bgergely0
Copy link
Contributor Author

Closing, as it does not apply without conflicts, and it's not clear what is needed to be accepted.

@bgergely0 bgergely0 closed this Sep 12, 2025
@paschalis-mpeis
Copy link
Member

Hey folks,

Apart the referenced issues (incl. the recent #157849), I know people working with Android who still apply this patch locally. It'll need a rebase, but it would be great to have this functionality.

@aaupov, you requested some changes here:

We don't want to introduce dependencies on host cross-compilers all the while Clang can be used for that purpose. We can add the dependency on Clang for cross-target runtimes build if necessary

with @bgergely0 following up here:

I have removed gcc cross-compilers all together. Let me know if this is what you meant, or other changes you think are needed.

Just checking if those have been addressed or if any concerns remain, so we can unblock this.
Thanks a lot!

@kaadam
Copy link
Contributor

kaadam commented Sep 12, 2025

Hi Everyone,

Sorry for jumping this conversation.
I agree with Paschlis' argument. This functionality would be beneficial to the Bolt project.
It is especially useful when the target device is not a developer device (like Android). It saves lot of time, if it shouldn't compile these runtime libs "manually". Probably others - who are in the same situation - will be able to join the Bolt development easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants