[LinkerWrapper] Accept some needed lld-link linker arguments for COFF targets

[jhuber6]

Summary:
The linker wrapper is a utility used to create offloading programs from
single-source offloading languages such as OpenMP or CUDA. This is done
by embedding device code into the host object, then feeding it into the
linker wrapper which extracts the accelerator object files, links them,
then wraps them in registration code for the target runtime. This
previously has only worked in Linux / ELF platforms.

This patch attempts to hand Windows / COFF inputs by also accepting COFF
forms of certain linker arguments we use internally. The important
arguments are library search paths, so we can identify libraries which
may contain device code, libraries themselves, and the output name used
for intermediate output.

I am not intimately familiar with the semantics here for the semantics
in how a lib file is earched. I am simply treating foo.lib as the
GNU equivalent -l:foo.lib in the search logic. Similarly, I am
assuming that static libraries will be llvm-ar style libraries. I will
need to investigate the actual deficiencies later, but this should be a
good starting point along with #72697

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Joseph Huber (jhuber6)

Changes

Summary:
The linker wrapper is a utility used to create offloading programs from
single-source offloading languages such as OpenMP or CUDA. This is done
by embedding device code into the host object, then feeding it into the
linker wrapper which extracts the accelerator object files, links them,
then wraps them in registration code for the target runtime. This
previously has only worked in Linux / ELF platforms.

This patch attempts to hand Windows / COFF inputs by also accepting COFF
forms of certain linker arguments we use internally. The important
arguments are library search paths, so we can identify libraries which
may contain device code, libraries themselves, and the output name used
for intermediate output.

I am not intimately familiar with the semantics here for the semantics
in how a lib file is earched. I am simply treating foo.lib as the
GNU equivalent -l:foo.lib in the search logic. Similarly, I am
assuming that static libraries will be llvm-ar style libraries. I will
need to investigate the actual deficiencies later, but this should be a
good starting point along with #72697

---

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

3 Files Affected:

• (modified) clang/test/Driver/linker-wrapper.c (+8)
• (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+13-5)
• (modified) clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td (+5)

diff --git a/clang/test/Driver/linker-wrapper.c b/clang/test/Driver/linker-wrapper.c
index da7bdc22153ceae..e82febd61823102 100644
--- a/clang/test/Driver/linker-wrapper.c
+++ b/clang/test/Driver/linker-wrapper.c
@@ -140,3 +140,11 @@
 // RUN:   --linker-path=/usr/bin/ld -- %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=CLANG-BACKEND
 
 // CLANG-BACKEND: clang{{.*}} -o {{.*}}.img --target=amdgcn-amd-amdhsa -mcpu=gfx908 -O2 -Wl,--no-undefined {{.*}}.bc
+
+// RUN: clang-offload-packager -o %t.out \
+// RUN:   --image=file=%t.elf.o,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
+// RUN: %clang -cc1 %s -triple x86_64-unknown-windows-msvc -emit-obj -o %t.o -fembed-offload-object=%t.out
+// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-windows-msvc --dry-run \
+// RUN:   --linker-path=/usr/bin/lld-link -- %t.o -libpath:./ -out:a.exe 2>&1 | FileCheck %s --check-prefix=COFF
+
+// COFF: "/usr/bin/lld-link" {{.*}}.o -libpath:./ -out:a.exe {{.*}}openmp.image.wrapper{{.*}}
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index bafe8ace60d1cea..db0ce3e2a190192 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -254,7 +254,7 @@ Error runLinker(ArrayRef<StringRef> Files, const ArgList &Args) {
       continue;
 
     Arg->render(Args, NewLinkerArgs);
-    if (Arg->getOption().matches(OPT_o))
+    if (Arg->getOption().matches(OPT_o) || Arg->getOption().matches(OPT_out))
       llvm::transform(Files, std::back_inserter(NewLinkerArgs),
                       [&](StringRef Arg) { return Args.MakeArgString(Arg); });
   }
@@ -1188,7 +1188,7 @@ searchLibraryBaseName(StringRef Name, StringRef Root,
 /// `-lfoo` or `-l:libfoo.a`.
 std::optional<std::string> searchLibrary(StringRef Input, StringRef Root,
                                          ArrayRef<StringRef> SearchPaths) {
-  if (Input.startswith(":"))
+  if (Input.startswith(":") || Input.ends_with(".lib"))
     return findFromSearchPaths(Input.drop_front(), Root, SearchPaths);
   return searchLibraryBaseName(Input, Root, SearchPaths);
 }
@@ -1339,7 +1339,7 @@ Expected<SmallVector<OffloadFile>> getDeviceInput(const ArgList &Args) {
 
   StringRef Root = Args.getLastArgValue(OPT_sysroot_EQ);
   SmallVector<StringRef> LibraryPaths;
-  for (const opt::Arg *Arg : Args.filtered(OPT_library_path))
+  for (const opt::Arg *Arg : Args.filtered(OPT_library_path, OPT_libpath))
     LibraryPaths.push_back(Arg->getValue());
 
   BumpPtrAllocator Alloc;
@@ -1348,7 +1348,7 @@ Expected<SmallVector<OffloadFile>> getDeviceInput(const ArgList &Args) {
   // Try to extract device code from the linker input files.
   SmallVector<OffloadFile> InputFiles;
   DenseMap<OffloadFile::TargetID, DenseMap<StringRef, Symbol>> Syms;
-  bool WholeArchive = false;
+  bool WholeArchive = Args.hasArg(OPT_wholearchive_flag) ? true : false;
   for (const opt::Arg *Arg : Args.filtered(
            OPT_INPUT, OPT_library, OPT_whole_archive, OPT_no_whole_archive)) {
     if (Arg->getOption().matches(OPT_whole_archive) ||
@@ -1474,9 +1474,17 @@ int main(int Argc, char **Argv) {
   Verbose = Args.hasArg(OPT_verbose);
   DryRun = Args.hasArg(OPT_dry_run);
   SaveTemps = Args.hasArg(OPT_save_temps);
-  ExecutableName = Args.getLastArgValue(OPT_o, "a.out");
   CudaBinaryPath = Args.getLastArgValue(OPT_cuda_path_EQ).str();
 
+  llvm::Triple Triple(
+      Args.getLastArgValue(OPT_host_triple_EQ, sys::getDefaultTargetTriple()));
+  if (Args.hasArg(OPT_o))
+    ExecutableName = Args.getLastArgValue(OPT_o, "a.out");
+  else if (Args.hasArg(OPT_out))
+    ExecutableName = Args.getLastArgValue(OPT_out, "a.exe");
+  else
+    ExecutableName = Triple.isOSWindows() ? "a.exe" : "a.out";
+
   parallel::strategy = hardware_concurrency(1);
   if (auto *Arg = Args.getLastArg(OPT_wrapper_jobs)) {
     unsigned Threads = 0;
diff --git a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
index 2ce6ee0d21d1d6e..21e479dd31c9721 100644
--- a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
+++ b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
@@ -126,3 +126,8 @@ def version : Flag<["--", "-"], "version">, Flags<[HelpHidden]>, Alias<v>;
 
 def whole_archive : Flag<["--", "-"], "whole-archive">, Flags<[HelpHidden]>;
 def no_whole_archive : Flag<["--", "-"], "no-whole-archive">, Flags<[HelpHidden]>;
+
+// COFF-style linker options.
+def out : Joined<["/", "-", "/?", "-?"], "out:">, Flags<[HelpHidden]>;
+def libpath : Joined<["/", "-", "/?", "-?"], "libpath:">, Flags<[HelpHidden]>;
+def wholearchive_flag : Joined<["/", "-", "/?", "-?"], "wholearchive">, Flags<[HelpHidden]>;

[Meinersbur]

The command-line argument handling is not related to PE/COFF, but to Microsoft's link.exe command line interface, for instance /libpath:. /usr/bin/lld-link is a link.exe-compatible interface for lld with an appropriate default triple, like clang-cl is for clang. IIRC, lld choses its command-line interface based on the argv[0] name, so should clang-linker-wrapper when passed --linker-path=/usr/bin/lld-link instead of --linker-path=/usr/bin/lld, but both should be able to generate PE files.

That is, this patch is not necessarily wrong, but the commit message and "// COFF-style linker options." should refer to the command line interface instead.

[Meinersbur]

Why /? and -? in the permitted option prefix list? Wouldn't this allow e.g. -?out:?

[jhuber6]

I copied this from the COFF implementation of lld-link and assumed that it knew the flags better than I did.

[Meinersbur]

Feels strange to me, but justification is in 4b81e9f

[jhuber6]

The command-line argument handling is not related to PE/COFF, but to Microsoft's link.exe command line interface, for instance /libpath:. /usr/bin/lld-link is a link.exe-compatible interface for lld with an appropriate default triple, like clang-cl is for clang. IIRC, lld choses its command-line interface based on the argv[0] name, so should clang-linker-wrapper when passed --linker-path=/usr/bin/lld-link instead of --linker-path=/usr/bin/lld, but both should be able to generate PE files.

That is, this patch is not necessarily wrong, but the commit message and "// COFF-style linker options." should refer to the command line interface instead.

I changed the title, realistically I could probably try to separate these more logically, but I think it's easier to just handle them both independently but identically in the logic.

[jhuber6]

Do COFF / Windows libraries have a special file magic / handling? I may need to update the extraction code to handle those in a later patch.

[Meinersbur]

PE file start with the magic bytes MZ (0x4D5A), already recognized by identify_magic. Extracting sections can usually done with the dumpbin.exe tool. I don't know whether LLVM has means to do it itself as well. The reference for the file format is here: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format

Edit: LLVM has parsing support here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Object/COFFObjectFile.cpp