Add CLANG_DEFAULT_OBJCOPY to allow Clang to use llvm-objcopy for dwarf fission

[jakehehrlich]

Summary

llvm-objcopy is getting to where it can be used in non-trivial ways (such as for dwarf fission in clang). It now supports dwarf fission but this feature hasn't been thoroughly tested yet. This change allows people to optionally build clang to use llvm-objcopy rather than GNU objcopy. By default GNU objcopy is still used so nothing should change.

[Hahnfeld]

CMakeLists.txt

236

Nit: runtime -> executable

[jakehehrlich]

fixed typo

[phosek]

LGTM

[Hahnfeld]

I think the current tests will only pass iff the executable name ends with objcopy - can we make this more reliable?

[jakehehrlich]

In D39821#921703, @Hahnfeld wrote:

I think the current tests will only pass iff the executable name ends with objcopy - can we make this more reliable?

If I read the tests (split-debug.h, split-debug.c, and split-debug.s) correctly they check for the existence of the word "objcopy" somewhere in the executables name. I'm not sure how to make it anymore flexible than that. The right thing to do would be to somehow get the string from CMake and use that in this test. I'm not sure of llvm-lit supports any kind of find and replace like that however.

Ideally it would look like

// CHECK: ${DEFAULT_OBJCOPY}{{.*}}--extract-dwo{{.*}}"split-debug.dwo"

And llvm-lit would replace every occurence of ${DEFAULT_OBJCOPY} with the the value it pulled from CMAKE_DEFAULT_OBJCOPY (the way that it gets host and target information)

[Hahnfeld]

In D39821#922026, @jakehehrlich wrote:

In D39821#921703, @Hahnfeld wrote:

I think the current tests will only pass iff the executable name ends with objcopy - can we make this more reliable?

If I read the tests (split-debug.h, split-debug.c, and split-debug.s) correctly they check for the existence of the word "objcopy" somewhere in the executables name. I'm not sure how to make it anymore flexible than that. The right thing to do would be to somehow get the string from CMake and use that in this test. I'm not sure of llvm-lit supports any kind of find and replace like that however.

Right you are, I must have missed {{.*}} at the end. Another option would be to restrict CLANG_DEFAULT_OBJCOPY to contain objcopy or even to only allow objcopy or llvm-objcopy. I don't know if that's sensible though...

[jakehehrlich]

Another option would be to restrict CLANG_DEFAULT_OBJCOPY to contain objcopy or even to only allow objcopy or llvm-objcopy. I don't know if that's sensible though...

I think there are some other strip/objcopy like implementations out there but I think objcopy and llvm-objcopy are the only ones that support --extract-dwo and --strip-dwo right now but more could come. I think objcopy and llvm-objcopy are the only ones that have "objcopy" in the name so the current test kind of enforces that one of them must be used. Also the user might want to pull objcopy or llvm-objcopy from a specific place on their system (not just from PATH) so however we restrict this, it will have to be much more general than just allowing "objcopy" or "llvm-objcopy". That said restricting the set of program names seems reasonable to me since you should in theory be able to add your program to the CMakeList.txt check on CLANG_DEFAULT_OBJCOPY if you want clang to support it. I don't think any other implementation of objcopy is likely to exist anytime soon though so it seems kind of moot to me.

[jakehehrlich]

This has already landed, I'm not sure why it wasn't automatically closed.

[smeenai]

It wasn't automatically closed because the commit (https://reviews.llvm.org/rL317960) referenced the wrong differential in its Differential Revision field. Not sure how that happened.

CMakeLists.txt

 if (NOT(CLANG_DEFAULT_RTLIB STREQUAL "" OR
         CLANG_DEFAULT_RTLIB STREQUAL "libgcc" OR
         CLANG_DEFAULT_RTLIB STREQUAL "compiler-rt"))
   message(WARNING "Resetting default rtlib to use platform default")
   set(CLANG_DEFAULT_RTLIB "" CACHE STRING
     "Default runtime library to use (\"libgcc\" or \"compiler-rt\", empty for platform default)" FORCE)
 endif()
 
+set(CLANG_DEFAULT_OBJCOPY "objcopy" CACHE STRING
+  "Default objcopy executable to use.")
+
 set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
   "Default OpenMP runtime used by -fopenmp.")
 
 # OpenMP offloading requires at least sm_30 because we use shuffle instructions
 # to generate efficient code for reductions.
 set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_30" CACHE STRING
   "Default architecture for OpenMP offloading to Nvidia GPUs.")
 string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}")

include/clang/Config/config.h.cmake

 #define CLANG_DEFAULT_LINKER "${CLANG_DEFAULT_LINKER}"
 
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"
 
 /* Default runtime library to use. */
 #define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"
 
+/* Default objcopy to use */
+#define CLANG_DEFAULT_OBJCOPY "${CLANG_DEFAULT_OBJCOPY}"
+
 /* Default OpenMP runtime used by -fopenmp. */
 #define CLANG_DEFAULT_OPENMP_RUNTIME "${CLANG_DEFAULT_OPENMP_RUNTIME}"
 
 /* Default architecture for OpenMP offloading to Nvidia GPUs. */
 #define CLANG_OPENMP_NVPTX_DEFAULT_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}"
 
 /* Multilib suffix for libdir. */
 #define CLANG_LIBDIR_SUFFIX "${CLANG_LIBDIR_SUFFIX}"

lib/Driver/ToolChains/CommonArgs.cpp

void tools::SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
   ArgStringList StripArgs;
   StripArgs.push_back("--strip-dwo");
 
   // Grabbing the output of the earlier compile step.
   StripArgs.push_back(Output.getFilename());
   ExtractArgs.push_back(Output.getFilename());
   ExtractArgs.push_back(OutFile);
 
-  const char *Exec = Args.MakeArgString(TC.GetProgramPath("objcopy"));
+  const char *Exec =
+      Args.MakeArgString(TC.GetProgramPath(CLANG_DEFAULT_OBJCOPY));
   InputInfo II(types::TY_Object, Output.getFilename(), Output.getFilename());
 
   // First extract the dwo sections.
   C.addCommand(llvm::make_unique<Command>(JA, T, Exec, ExtractArgs, II));
 
   // Then remove them from the original .o file.
   C.addCommand(llvm::make_unique<Command>(JA, T, Exec, StripArgs, II));
 }