[Driver][OpenMP] Add support to create jobs for bundling actions.

[sfantao]

Summary

This patch adds the support to create a job for the OffloadBundlingAction which will invoke the clang-offload-bundler tool.

[ABataev]

include/clang/Driver/Action.h

161

'\brief'

lib/Driver/Action.cpp

134

1. Maybe it is enough to return just StringRef, rather than std::string
2. This can be made 'static'

lib/Driver/Tools.h

139

'final'?

[sfantao]

• Use StringRef instead of string and mark bundler tool as final.

[sfantao]

Hi Alexey,

Thanks for the review!

lib/Driver/Action.cpp

134

True, it can be a StringRef. This function is already marked static in its declaration.

[sfantao]

• Use StringRef instead of std::string, fix comments and mark class final.

[sfantao]

• Start static function name with caps.

[sfantao]

• Rebase.

[sfantao]

• Rebase.

[ABataev]

LG

[hfinkel]

LGTM

[thakis]

Sorry about the years-later question, but what's the motivation for shelling out to an external command here? In general, LLVM tries to use a library-based approach, and LLVM went e.g. through great lengths do use an integrated assembler so clang doesn't have to shell out to one. Concretely, why isn't there a clang/lib/OffloadBundle library that clang and clang-offload-bundler both use? Does the bundling have to happen out-of-process? And if so, why isn't the process something like clang -cc1bundle instead of a separate executable? It seems weird to make clang depend on another executable next to it.

I apologize for the clueless question.

[sfantao]

In D21856#1338687, @thakis wrote:

Sorry about the years-later question, but what's the motivation for shelling out to an external command here? In general, LLVM tries to use a library-based approach, and LLVM went e.g. through great lengths do use an integrated assembler so clang doesn't have to shell out to one. Concretely, why isn't there a clang/lib/OffloadBundle library that clang and clang-offload-bundler both use? Does the bundling have to happen out-of-process? And if so, why isn't the process something like clang -cc1bundle instead of a separate executable? It seems weird to make clang depend on another executable next to it.

I apologize for the clueless question.

@thakis, I don't think there is anything preventing the bundler from being driven by clang and implemented in a library. The discussions about the bundler were mostly around the bundling formats - so where it should be implemented was not as thoroughly discussed as the the formats we ended up adopting. Notwithstanding, there were a few reasons that motivated the current design:

• The bundler should have a driver: users/developers should be able to easily pack/unpack the pieces referring to different devices. It could be a clang -cc1bundle though.
• Offloading with many targets and different host/target combinations would require a myriad of formats that need to be used together, so they were some informal discussions where people seemed to prefer to separate this complexity from clang itself. The bundler would need some dependencies to handle object files that were not needed by clang (at least at the time).
• The main use of the bundler is the bundling/unbundling of object files which were produced by third party tools (e.g. NVIDIA tools), so there was not a clear advantage in integrating the bundler as a library, as one needs to spawn a separate process to create its dependences anyway.

include/clang/Driver/Action.h

bool isCollapsingWithDependingActionLegal() const {
     return CanBeCollapsedWithDependingAction;
   }
 
   /// Return a string containing the offload kind of the action.
   std::string getOffloadingKindPrefix() const;
   /// Return a string that can be used as prefix in order to generate unique
   /// files for each offloading kind.
   std::string getOffloadingFileNamePrefix(StringRef NormalizedTriple) const;
+  /// Return a string containing a offload kind name.
+  static StringRef GetOffloadKindName(OffloadKind Kind);
 
   /// Set the device offload info of this action and propagate it to its
   /// dependences.
   void propagateDeviceOffloadInfo(OffloadKind OKind, const char *OArch);
   /// Append the host offload info of this action and propagate it to its
   /// dependences.
   void propagateHostOffloadInfo(unsigned OKinds, const char *OArch);
   /// Set the offload info of this action to be the same as the provided action,

include/clang/Driver/ToolChain.h

private:
 
   /// The list of toolchain specific path prefixes to search for
   /// programs.
   path_list ProgramPaths;
 
   mutable std::unique_ptr<Tool> Clang;
   mutable std::unique_ptr<Tool> Assemble;
   mutable std::unique_ptr<Tool> Link;
+  mutable std::unique_ptr<Tool> OffloadBundler;
   Tool *getClang() const;
   Tool *getAssemble() const;
   Tool *getLink() const;
   Tool *getClangAs() const;
+  Tool *getOffloadBundler() const;
 
   mutable std::unique_ptr<SanitizerArgs> SanitizerArguments;
 
 protected:
   MultilibSet Multilibs;
   const char *DefaultLinker = "ld";
 
   ToolChain(const Driver &D, const llvm::Triple &T,

lib/Driver/Action.cpp

 //===--- Action.cpp - Abstract compilation steps --------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Driver/Action.h"
 #include "clang/Driver/ToolChain.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Regex.h"
 #include <cassert>
 using namespace clang::driver;
 using namespace llvm::opt;
 
 Action::~Action() {}
Action::getOffloadingFileNamePrefix(StringRef NormalizedTriple) const {
 
   std::string Res("-");
   Res += getOffloadingKindPrefix();
   Res += "-";
   Res += NormalizedTriple;
   return Res;
 }
 
+/// Return a string with the offload kind name. If that is not defined, we
+/// assume 'host'.
+llvm::StringRef Action::GetOffloadKindName(OffloadKind Kind) {
+  switch (Kind) {
+  case OFK_None:
+  case OFK_Host:
+    return "host";
+  case OFK_Cuda:
+    return "cuda";
+  case OFK_OpenMP:
+    return "openmp";
+
+    // TODO: Add other programming models here.
+  }
+}
+
 void InputAction::anchor() {}
 
 InputAction::InputAction(const Arg &_Input, types::ID _Type)
   : Action(InputClass, _Type), Input(_Input) {
 }
 
 void BindArchAction::anchor() {}
 

lib/Driver/ToolChain.cpp

 }
 
 Tool *ToolChain::getLink() const {
   if (!Link)
     Link.reset(buildLinker());
   return Link.get();
 }
 
+Tool *ToolChain::getOffloadBundler() const {
+  if (!OffloadBundler)
+    OffloadBundler.reset(new tools::OffloadBundler(*this));
+  return OffloadBundler.get();
+}
+
 Tool *ToolChain::getTool(Action::ActionClass AC) const {
   switch (AC) {
   case Action::AssembleJobClass:
     return getAssemble();
 
   case Action::LinkJobClass:
     return getLink();
 
Tool *ToolChain::getTool(Action::ActionClass AC) const {
   case Action::AnalyzeJobClass:
   case Action::MigrateJobClass:
   case Action::VerifyPCHJobClass:
   case Action::BackendJobClass:
     return getClang();
 
   case Action::OffloadBundlingJobClass:
   case Action::OffloadUnbundlingJobClass:
-    // FIXME: Add a tool for the bundling actions.
-    return nullptr;
+    return getOffloadBundler();
   }
 
   llvm_unreachable("Invalid tool kind.");
 }
 
 static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
                                              const ArgList &Args) {
   const llvm::Triple &Triple = TC.getTriple();

lib/Driver/Tools.h

public:
   bool hasIntegratedCPP() const override { return false; }
 
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;
 };
 
+/// Offload bundler tool.
+class LLVM_LIBRARY_VISIBILITY OffloadBundler final : public Tool {
+public:
+  OffloadBundler(const ToolChain &TC)
+      : Tool("offload bundler", "clang-offload-bundler", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  void ConstructJob(Compilation &C, const JobAction &JA,
+                    const InputInfo &Output, const InputInfoList &Inputs,
+                    const llvm::opt::ArgList &TCArgs,
+                    const char *LinkingOutput) const override;
+};
+
 /// \brief Base class for all GNU tools that provide the same behavior when
 /// it comes to response files support
 class LLVM_LIBRARY_VISIBILITY GnuTool : public Tool {
   virtual void anchor();
 
 public:
   GnuTool(const char *Name, const char *ShortName, const ToolChain &TC)
       : Tool(Name, ShortName, TC, RF_Full, llvm::sys::WEM_CurrentCodePage) {}

lib/Driver/Tools.cpp

void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
   // creating an object.
   // TODO: Currently only works on linux with newer objcopy.
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
       getToolChain().getTriple().isOSLinux())
     SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
                    SplitDebugName(Args, Input));
 }
 
+void OffloadBundler::ConstructJob(Compilation &C, const JobAction &JA,
+                                  const InputInfo &Output,
+                                  const InputInfoList &Inputs,
+                                  const llvm::opt::ArgList &TCArgs,
+                                  const char *LinkingOutput) const {
+  assert(isa<OffloadBundlingJobAction>(JA) && "Expecting bundling job!");
+
+  // The bundling command looks like this:
+  // clang-offload-bundler -type=bc
+  //   -targets=host-triple,openmp-triple1,openmp-triple2
+  //   -outputs=input_file
+  //   -inputs=unbundle_file_host,unbundle_file_tgt1,unbundle_file_tgt2"
+
+  ArgStringList CmdArgs;
+
+  // Get the type.
+  CmdArgs.push_back(TCArgs.MakeArgString(
+      Twine("-type=") + types::getTypeTempSuffix(Output.getType())));
+
+  assert(JA.getInputs().size() == Inputs.size() &&
+         "Not have inputs for all dependence actions??");
+
+  // Get the targets.
+  SmallString<128> Triples;
+  Triples += "-targets=";
+  for (unsigned I = 0; I < Inputs.size(); ++I) {
+    if (I)
+      Triples += ',';
+
+    Action::OffloadKind CurKind = Action::OFK_Host;
+    const ToolChain *CurTC = &getToolChain();
+    const Action *CurDep = JA.getInputs()[I];
+
+    if (const auto *OA = dyn_cast<OffloadAction>(CurDep)) {
+      OA->doOnEachDependence([&](Action *A, const ToolChain *TC, const char *) {
+        CurKind = A->getOffloadingDeviceKind();
+        CurTC = TC;
+      });
+    }
+    Triples += Action::GetOffloadKindName(CurKind);
+    Triples += '-';
+    Triples += CurTC->getTriple().normalize();
+  }
+  CmdArgs.push_back(TCArgs.MakeArgString(Triples));
+
+  // Get bundled file command.
+  CmdArgs.push_back(
+      TCArgs.MakeArgString(Twine("-outputs=") + Output.getFilename()));
+
+  // Get unbundled files command.
+  SmallString<128> UB;
+  UB += "-inputs=";
+  for (unsigned I = 0; I < Inputs.size(); ++I) {
+    if (I)
+      UB += ',';
+    UB += Inputs[I].getFilename();
+  }
+  CmdArgs.push_back(TCArgs.MakeArgString(UB));
+
+  // All the inputs are encoded as commands.
+  C.addCommand(llvm::make_unique<Command>(
+      JA, *this,
+      TCArgs.MakeArgString(getToolChain().GetProgramPath(getShortName())),
+      CmdArgs, None));
+}
+
 void GnuTool::anchor() {}
 
 void gcc::Common::ConstructJob(Compilation &C, const JobAction &JA,
                                const InputInfo &Output,
                                const InputInfoList &Inputs, const ArgList &Args,
                                const char *LinkingOutput) const {
   const Driver &D = getToolChain().getDriver();
   ArgStringList CmdArgs;

test/Driver/openmp-offload.c

 // CHK-UBUACTIONS: 9: offload, "host-openmp (powerpc64le--linux)" {2}, "device-openmp (x86_64-pc-linux-gnu)" {8}, ir
 // CHK-UBUACTIONS: 10: backend, {9}, assembler, (device-openmp)
 // CHK-UBUACTIONS: 11: assembler, {10}, object, (device-openmp)
 // CHK-UBUACTIONS: 12: offload, "device-openmp (x86_64-pc-linux-gnu)" {11}, object
 // CHK-UBUACTIONS: 13: backend, {2}, assembler, (host-openmp)
 // CHK-UBUACTIONS: 14: assembler, {13}, object, (host-openmp)
 // CHK-UBUACTIONS: 15: clang-offload-bundler, {7, 12, 14}, object, (host-openmp)
 
+/// ###########################################################################
+
+/// Check separate compilation with offloading - bundling jobs construct
+// RUN:   %clang -### -fopenmp -c -o %t.o -lsomelib -target powerpc64le-linux -fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-BUJOBS %s
+// RUN:   %clang -### -fopenmp -c -o %t.o -lsomelib -target powerpc64le-linux -fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu %s -save-temps 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHK-BUJOBS-ST %s
+
+// Create host BC.
+// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-emit-llvm-bc"  {{.*}}"-fopenmp" {{.*}}"-o" "[[HOSTBC:.+\.bc]]" "-x" "c" "[[INPUT:.+\.c]]" "-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu"
+
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-E"  {{.*}}"-fopenmp" {{.*}}"-o" "[[HOSTPP:.+\.i]]" "-x" "c" "[[INPUT:.+\.c]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-emit-llvm-bc"  {{.*}}"-fopenmp" {{.*}}"-o" "[[HOSTBC:.+\.bc]]" "-x" "cpp-output" "[[HOSTPP]]" "-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu"
+
+// Create target 1 object.
+// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "[[T1OBJ:.+\.o]]" "-x" "c" "[[INPUT]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "[[HOSTBC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "[[T1PP:.+\.i]]" "-x" "c" "[[INPUT]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-emit-llvm-bc" {{.*}}"-fopenmp" {{.*}}"-o" "[[T1BC:.+\.bc]]" "-x" "cpp-output" "[[T1PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "[[HOSTBC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "[[T1ASM:.+\.s]]" "-x" "ir" "[[T1BC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1as" "-triple" "powerpc64le-ibm-linux-gnu" "-filetype" "obj" {{.*}}"-o" "[[T1OBJ:.+\.o]]" "[[T1ASM]]"
+
+// Create target 2 object.
+// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "[[T2OBJ:.+\.o]]" "-x" "c" "[[INPUT]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "[[HOSTBC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "[[T2PP:.+\.i]]" "-x" "c" "[[INPUT]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-emit-llvm-bc" {{.*}}"-fopenmp" {{.*}}"-o" "[[T2BC:.+\.bc]]" "-x" "cpp-output" "[[T2PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" "[[HOSTBC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "[[T2ASM:.+\.s]]" "-x" "ir" "[[T2BC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1as" "-triple" "x86_64-pc-linux-gnu" "-filetype" "obj" {{.*}}"-o" "[[T2OBJ:.+\.o]]" "[[T2ASM]]"
+
+// Create host object and bundle.
+// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "[[HOSTOBJ:.+\.o]]" "-x" "ir" "[[HOSTBC]]"
+// CHK-BUJOBS: clang-offload-bundler" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=[[RES:.+\.o]]" "-inputs=[[T1OBJ]],[[T2OBJ]],[[HOSTOBJ]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "[[HOSTASM:.+\.s]]" "-x" "ir" "[[HOSTBC]]"
+// CHK-BUJOBS-ST: clang{{.*}}" "-cc1as" "-triple" "powerpc64le--linux" "-filetype" "obj" {{.*}}"-o" "[[HOSTOBJ:.+\.o]]" "[[HOSTASM]]"
+// CHK-BUJOBS-ST: clang-offload-bundler" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=[[RES:.+\.o]]" "-inputs=[[T1OBJ]],[[T2OBJ]],[[HOSTOBJ]]"