[OpenMP] Add better testing for the linker wrapper

[jhuber6]

Summary

The linker wrapper is used to perform linking and wrapping of embedded
device object files. Currently its internals are not able to be tested
easily. This patch adds the --dry-run and --print-wrapped-module
options to investigate the link jobs that will be run along with the
wrapped code that will be created to register the binaries.

[JonChesterfield]

Nice! I like the strategy.

[jhuber6]

Fix test on Windows failing because of ".exe" at the end.

[thakis]

Looks like this breaks tests on Mac: http://45.33.8.238/macm1/33656/step_7.txt

Please take a look and revert for now if it takes a while to fix. (Maybe just needs an explicit triple?)

[jhuber6]

In D124039#3461482, @thakis wrote:

Looks like this breaks tests on Mac: http://45.33.8.238/macm1/33656/step_7.txt

Please take a look and revert for now if it takes a while to fix. (Maybe just needs an explicit triple?)

Yeah I probably just need to specify the triple, I forgot that Mach-O doesn't like these section names. I pushed rG1dfe0273fda3 to address this, let me know if it's still broken.

[thakis]

Thanks, that did the trick :)

[zibi]

clang/test/Driver/linker-wrapper-image.c

8

On PowerPC I'm seeing the following error: pointing at @ char:
clang/test/Driver/linker-wrapper-image.c:10:12: error: OPENMP: expected string not found in input
// OPENMP: @__start_omp_offloading_entries = external hidden constant %__tgt_offload_entry

Are you aware of this?

clang/test/Driver/linker-wrapper.c

9

Similar error to above, the coordinates are off by 2 because of subsequent changes:

clang/test/Driver/linker-wrapper.c:11:16: error: NVPTX_LINK: expected string not found in input
// NVPTX_LINK: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o

[jhuber6]

clang/test/Driver/linker-wrapper-image.c

8

Do you have a link to a buildbot with the full error log? I'll try to test it locally by manually setting the triple.

[jhuber6]

clang/test/Driver/linker-wrapper-image.c

8

I tested this on a PPC64LE machine and couldn't reproduce, do you have any more information?

[zibi]

clang/test/Driver/linker-wrapper-image.c

8

Yes, those test cases do pass on external bots. At the time of reporting I did not know this affects only the internal builds. We will handle this internally.

clang/test/Driver/linker-wrapper-image.c

+// REQUIRES: x86-registered-target
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,nvptx64-nvida-cuda,sm_70
+// RUN: clang-linker-wrapper --print-wrapped-module --dry-run -linker-path /usr/bin/ld \
+// RUN:   -- %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=OPENMP
+
+// OPENMP: @__start_omp_offloading_entries = external hidden constant %__tgt_offload_entry
+// OPENMP-NEXT: @__stop_omp_offloading_entries = external hidden constant %__tgt_offload_entry
+// OPENMP-NEXT: @__dummy.omp_offloading.entry = hidden constant [0 x %__tgt_offload_entry] zeroinitializer, section "omp_offloading_entries"
+// OPENMP-NEXT: @.omp_offloading.device_image = internal unnamed_addr constant [0 x i8] zeroinitializer
+// OPENMP-NEXT: @.omp_offloading.device_images = internal unnamed_addr constant [1 x %__tgt_device_image] [%__tgt_device_image { i8* getelementptr inbounds ([0 x i8], [0 x i8]* @.omp_offloading.device_image, i64 0, i64 0), i8* getelementptr inbounds ([0 x i8], [0 x i8]* @.omp_offloading.device_image, i64 0, i64 0), %__tgt_offload_entry* @__start_omp_offloading_entries, %__tgt_offload_entry* @__stop_omp_offloading_entries }]
+// OPENMP-NEXT: @.omp_offloading.descriptor = internal constant %__tgt_bin_desc { i32 1, %__tgt_device_image* getelementptr inbounds ([1 x %__tgt_device_image], [1 x %__tgt_device_image]* @.omp_offloading.device_images, i64 0, i64 0), %__tgt_offload_entry* @__start_omp_offloading_entries, %__tgt_offload_entry* @__stop_omp_offloading_entries }
+// OPENMP-NEXT: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 1, void ()* @.omp_offloading.descriptor_reg, i8* null }]
+// OPENMP-NEXT: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 1, void ()* @.omp_offloading.descriptor_unreg, i8* null }]
+
+// OPENMP: define internal void @.omp_offloading.descriptor_reg() section ".text.startup" {
+// OPENMP-NEXT: entry:
+// OPENMP-NEXT:   call void @__tgt_register_lib(%__tgt_bin_desc* @.omp_offloading.descriptor)
+// OPENMP-NEXT:   ret void
+// OPENMP-NEXT: }
+
+// OPENMP: define internal void @.omp_offloading.descriptor_unreg() section ".text.startup" {
+// OPENMP-NEXT: entry:
+// OPENMP-NEXT:   call void @__tgt_unregister_lib(%__tgt_bin_desc* @.omp_offloading.descriptor)
+// OPENMP-NEXT:   ret void
+// OPENMP-NEXT: }

clang/test/Driver/linker-wrapper.c

+// REQUIRES: x86-registered-target
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,nvptx64-nvida-cuda,sm_70 \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,nvptx64-nvida-cuda,sm_70
+// RUN: clang-linker-wrapper --dry-run -linker-path /usr/bin/ld -- %t.o -o a.out \
+// RUN:   2>&1 | FileCheck %s --check-prefix=NVPTX_LINK
+
+// NVPTX_LINK: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.o {{.*}}.o
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,amdgcn-amd-amdhsam,gfx908 \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,amdgcn-amd-amdhsam,gfx908
+// RUN: clang-linker-wrapper --dry-run -linker-path /usr/bin/ld -- %t.o -o a.out \
+// RUN:   2>&1 | FileCheck %s --check-prefix=AMDGPU_LINK
+
+// AMDGPU_LINK: lld{{.*}}-flavor gnu --no-undefined -shared -o {{.*}}.out {{.*}}.o {{.*}}.o
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,x86_64-unknown-linux-gnu, \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-elf.o,openmp,x86_64-unknown-linux-gnu,
+// RUN: clang-linker-wrapper --dry-run -linker-path /usr/bin/ld.lld -- %t.o -o a.out \
+// RUN:   2>&1 | FileCheck %s --check-prefix=CPU_LINK
+
+// CPU_LINK: ld.lld{{.*}}-m elf_x86_64 -shared -Bsymbolic -o {{.*}}.out {{.*}}.o {{.*}}.o
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o
+// RUN: clang-linker-wrapper --dry-run -linker-path /usr/bin/ld.lld -- -a -b -c %t.o -o a.out \
+// RUN:   2>&1 | FileCheck %s --check-prefix=HOST_LINK
+
+// HOST_LINK: ld.lld{{.*}}-a -b -c {{.*}}.o -o a.out
+
+// RUN: %clang -cc1 %s -emit-obj -o %t.o \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-bc.bc,openmp,nvptx64-nvida-cuda,sm_70 \
+// RUN:   -fembed-offload-object=%S/Inputs/dummy-bc.bc,openmp,nvptx64-nvida-cuda,sm_70
+// RUN: clang-linker-wrapper --dry-run -linker-path /usr/bin/ld -- %t.o -o a.out \
+// RUN:   2>&1 | FileCheck %s --check-prefix=LTO
+
+// LTO: ptxas{{.*}}-m64 -o {{.*}}.cubin -O2 --gpu-name sm_70 -c {{.*}}.s
+// LTO: nvlink{{.*}}-m64 -o {{.*}}.out -arch sm_70 {{.*}}.cubin

clang/test/lit.cfg.py

 # For each occurrence of a clang tool name, replace it with the full path to
 # the build directory holding that tool.  We explicitly specify the directories
 # to search to ensure that we get the tools just built and not some random
 # tools that might happen to be in the user's PATH.
 tool_dirs = [config.clang_tools_dir, config.llvm_tools_dir]
 
 tools = [
     'apinotes-test', 'c-index-test', 'clang-diff', 'clang-format', 'clang-repl',
-    'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj',
+    'clang-tblgen', 'clang-scan-deps', 'opt', 'llvm-ifs', 'yaml2obj', 'clang-linker-wrapper',
     ToolSubst('%clang_extdef_map', command=FindTool(
         'clang-extdef-mapping'), unresolved='ignore'),
     ToolSubst('%clang_dxc', command=config.clang,
         extra_args=['--driver-mode=dxc']),
 ]
 
 if config.clang_examples:
     config.available_features.add('examples')

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

BitcodeLibraries("target-library", cl::ZeroOrMore,
                      cl::desc("Path for the target bitcode library"),
                      cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt<bool> EmbedBitcode(
     "target-embed-bc", cl::ZeroOrMore,
     cl::desc("Embed linked bitcode instead of an executable device image"),
     cl::init(false), cl::cat(ClangLinkerWrapperCategory));
 
+static cl::opt<bool> DryRun(
+    "dry-run", cl::ZeroOrMore,
+    cl::desc("List the linker commands to be run without executing them"),
+    cl::init(false), cl::cat(ClangLinkerWrapperCategory));
+
+static cl::opt<bool>
+    PrintWrappedModule("print-wrapped-module", cl::ZeroOrMore,
+                       cl::desc("Print the wrapped module's IR for testing"),
+                       cl::init(false), cl::cat(ClangLinkerWrapperCategory));
+
 static cl::opt<std::string>
     HostTriple("host-triple", cl::ZeroOrMore,
                cl::desc("Triple to use for the host compilation"),
                cl::init(sys::getDefaultTargetTriple()),
                cl::cat(ClangLinkerWrapperCategory));
 
 static cl::list<std::string>
     PtxasArgs("ptxas-args", cl::ZeroOrMore,
Error createOutputFile(const Twine &Prefix, StringRef Extension,
   } else {
     const Twine &Filename = Prefix + "." + Extension;
     Filename.toNullTerminatedStringRef(NewFilename);
   }
 
   return Error::success();
 }
 
+/// Execute the command \p ExecutablePath with the arguments \p Args.
+Error executeCommands(StringRef ExecutablePath, ArrayRef<StringRef> Args) {
+  if (Verbose || DryRun)
+    printCommands(Args);
+
+  if (!DryRun)
+    if (sys::ExecuteAndWait(ExecutablePath, Args))
+      return createStringError(inconvertibleErrorCode(),
+                               "'" + sys::path::filename(ExecutablePath) + "'" +
+                                   " failed");
+  return Error::success();
+}
+
+Expected<std::string> findProgram(StringRef Name, ArrayRef<StringRef> Paths) {
+
+  ErrorOr<std::string> Path = sys::findProgramByName(Name, Paths);
+  if (!Path)
+    Path = sys::findProgramByName(Name);
+  if (!Path && DryRun)
+    return Name.str();
+  if (!Path)
+    return createStringError(Path.getError(),
+                             "Unable to find '" + Name + "' in path");
+  return *Path;
+}
+
 Error runLinker(std::string &LinkerPath, SmallVectorImpl<std::string> &Args) {
   std::vector<StringRef> LinkerArgs;
   LinkerArgs.push_back(LinkerPath);
   for (auto &Arg : Args)
     LinkerArgs.push_back(Arg);
 
-  if (Verbose)
-    printCommands(LinkerArgs);
-
-  if (sys::ExecuteAndWait(LinkerPath, LinkerArgs))
-    return createStringError(inconvertibleErrorCode(), "'linker' failed");
+  if (Error Err = executeCommands(LinkerPath, LinkerArgs))
+    return Err;
   return Error::success();
 }
 
 void PrintVersion(raw_ostream &OS) {
   OS << clang::getClangToolFullVersion("clang-linker-wrapper") << '\n';
 }
 
 void removeFromCompilerUsed(Module &M, GlobalValue &Value) {
if (!sys::fs::exists(StripFile)) {
     std::copy(Contents.begin(), Contents.end(), Output->getBufferStart());
     if (Error E = Output->commit())
       return std::move(E);
     StripFile = TempFile;
   }
 
   // We will use llvm-strip to remove the now unneeded section containing the
   // offloading code.
-  ErrorOr<std::string> StripPath =
-      sys::findProgramByName("llvm-strip", {getMainExecutable("llvm-strip")});
-  if (!StripPath)
-    StripPath = sys::findProgramByName("llvm-strip");
+  Expected<std::string> StripPath =
+      findProgram("llvm-strip", {getMainExecutable("llvm-strip")});
   if (!StripPath)
-    return None;
+    return StripPath.takeError();
 
   SmallString<128> TempFile;
   if (Error Err = createOutputFile(Prefix + "-host", Extension, TempFile))
     return std::move(Err);
 
   SmallVector<StringRef, 8> StripArgs;
   StripArgs.push_back(*StripPath);
   StripArgs.push_back("--no-strip-all");
   StripArgs.push_back(StripFile);
   for (auto &Section : ToBeStripped) {
     StripArgs.push_back("--remove-section");
     StripArgs.push_back(Section);
   }
   StripArgs.push_back("-o");
   StripArgs.push_back(TempFile);
 
-  if (Verbose)
-    printCommands(StripArgs);
-
-  if (sys::ExecuteAndWait(*StripPath, StripArgs))
-    return createStringError(inconvertibleErrorCode(), "'llvm-strip' failed");
+  if (Error Err = executeCommands(*StripPath, StripArgs))
+    return Err;
 
   return static_cast<std::string>(TempFile);
 }
 
 Expected<Optional<std::string>>
 extractFromBitcode(std::unique_ptr<MemoryBuffer> Buffer,
                    SmallVectorImpl<DeviceFile> &DeviceFiles) {
   LLVMContext Context;
extractFromBuffer(std::unique_ptr<MemoryBuffer> Buffer,
   }
 }
 
 // TODO: Move these to a separate file.
 namespace nvptx {
 Expected<std::string> assemble(StringRef InputFile, Triple TheTriple,
                                StringRef Arch) {
   // NVPTX uses the ptxas binary to create device object files.
-  ErrorOr<std::string> PtxasPath =
-      sys::findProgramByName("ptxas", {CudaBinaryPath});
-  if (!PtxasPath)
-    PtxasPath = sys::findProgramByName("ptxas");
+  Expected<std::string> PtxasPath = findProgram("ptxas", {CudaBinaryPath});
   if (!PtxasPath)
-    return createStringError(PtxasPath.getError(),
-                             "Unable to find 'ptxas' in path");
+    return PtxasPath.takeError();
 
   // Create a new file to write the linked device image to.
   SmallString<128> TempFile;
   if (Error Err =
           createOutputFile(sys::path::filename(ExecutableName) + "-device-" +
                                TheTriple.getArchName() + "-" + Arch,
                            "cubin", TempFile))
     return std::move(Err);
Expected<std::string> assemble(StringRef InputFile, Triple TheTriple,
   CmdArgs.push_back(Arch);
   CmdArgs.push_back("-c");
 
   CmdArgs.push_back(InputFile);
 
   if (Verbose)
     printCommands(CmdArgs);
 
-  if (sys::ExecuteAndWait(*PtxasPath, CmdArgs))
-    return createStringError(inconvertibleErrorCode(), "'ptxas' failed");
+  if (Error Err = executeCommands(*PtxasPath, CmdArgs))
+    return Err;
 
   return static_cast<std::string>(TempFile);
 }
 
 Expected<std::string> link(ArrayRef<std::string> InputFiles, Triple TheTriple,
                            StringRef Arch) {
   // NVPTX uses the nvlink binary to link device object files.
-  ErrorOr<std::string> NvlinkPath =
-      sys::findProgramByName("nvlink", {CudaBinaryPath});
-  if (!NvlinkPath)
-    NvlinkPath = sys::findProgramByName("nvlink");
+  Expected<std::string> NvlinkPath = findProgram("nvlink", {CudaBinaryPath});
   if (!NvlinkPath)
-    return createStringError(NvlinkPath.getError(),
-                             "Unable to find 'nvlink' in path");
+    return NvlinkPath.takeError();
 
   // Create a new file to write the linked device image to.
   SmallString<128> TempFile;
   if (Error Err =
           createOutputFile(sys::path::filename(ExecutableName) + "-device-" +
                                TheTriple.getArchName() + "-" + Arch,
                            "out", TempFile))
     return std::move(Err);
Expected<std::string> link(ArrayRef<std::string> InputFiles, Triple TheTriple,
 
   // Add extracted input files.
   for (StringRef Input : InputFiles)
     CmdArgs.push_back(Input);
 
   if (Verbose)
     printCommands(CmdArgs);
 
-  if (sys::ExecuteAndWait(*NvlinkPath, CmdArgs))
-    return createStringError(inconvertibleErrorCode(), "'nvlink' failed");
+  if (Error Err = executeCommands(*NvlinkPath, CmdArgs))
+    return Err;
 
   return static_cast<std::string>(TempFile);
 }
 } // namespace nvptx
 namespace amdgcn {
 Expected<std::string> link(ArrayRef<std::string> InputFiles, Triple TheTriple,
                            StringRef Arch) {
   // AMDGPU uses lld to link device object files.
-  ErrorOr<std::string> LLDPath =
-      sys::findProgramByName("lld", {getMainExecutable("lld")});
-  if (!LLDPath)
-    LLDPath = sys::findProgramByName("lld");
+  Expected<std::string> LLDPath = findProgram("lld", {CudaBinaryPath});
   if (!LLDPath)
-    return createStringError(LLDPath.getError(),
-                             "Unable to find 'lld' in path");
+    return LLDPath.takeError();
 
   // Create a new file to write the linked device image to.
   SmallString<128> TempFile;
   if (Error Err = createOutputFile(sys::path::filename(ExecutableName) + "-" +
                                        TheTriple.getArchName() + "-" + Arch,
                                    "out", TempFile))
     return std::move(Err);
 
   SmallVector<StringRef, 16> CmdArgs;
   CmdArgs.push_back(*LLDPath);
   CmdArgs.push_back("-flavor");
   CmdArgs.push_back("gnu");
   CmdArgs.push_back("--no-undefined");
   CmdArgs.push_back("-shared");
   CmdArgs.push_back("-o");
   CmdArgs.push_back(TempFile);
 
   // Add extracted input files.
   for (StringRef Input : InputFiles)
     CmdArgs.push_back(Input);
 
   if (Verbose)
     printCommands(CmdArgs);
 
-  if (sys::ExecuteAndWait(*LLDPath, CmdArgs))
-    return createStringError(inconvertibleErrorCode(), "'lld' failed");
+  if (Error Err = executeCommands(*LLDPath, CmdArgs))
+    return Err;
 
   return static_cast<std::string>(TempFile);
 }
 } // namespace amdgcn
 
 namespace generic {
 
 const char *getLDMOption(const llvm::Triple &T) {
Expected<std::string> link(ArrayRef<std::string> InputFiles, Triple TheTriple,
 
   // Add extracted input files.
   for (StringRef Input : InputFiles)
     CmdArgs.push_back(Input);
 
   if (Verbose)
     printCommands(CmdArgs);
 
-  if (sys::ExecuteAndWait(LinkerUserPath, CmdArgs))
-    return createStringError(inconvertibleErrorCode(), "'linker' failed");
+  if (Error Err = executeCommands(LinkerUserPath, CmdArgs))
+    return Err;
 
   return static_cast<std::string>(TempFile);
 }
 } // namespace generic
 
 Expected<std::string> linkDevice(ArrayRef<std::string> InputFiles,
                                  Triple TheTriple, StringRef Arch) {
   switch (TheTriple.getArch()) {
Expected<std::string> wrapDeviceImages(ArrayRef<std::string> Images) {
   }
 
   LLVMContext Context;
   Module M("offload.wrapper.module", Context);
   M.setTargetTriple(HostTriple);
   if (Error Err = wrapBinaries(M, ImagesToWrap))
     return std::move(Err);
 
+  if (PrintWrappedModule)
+    llvm::errs() << M;
+
   return compileModule(M);
 }
 
 Optional<std::string> findFile(StringRef Dir, const Twine &Name) {
   SmallString<128> Path;
   if (Dir.startswith("="))
     sys::path::append(Path, Sysroot, Dir.substr(1), Name);
   else