[Driver] Add CUDA support for --offload param

[dcastagna]

Summary

The --offload option was added in D110622 to "override the default
device target". When it landed it supported only HIP.
This CL extends that option to support SPIRV targets for CUDA.

[tra]

I think instead of setting the triple directly from the command line, we should start with adding another --cuda-gpu-arch (AKA --offload-arch) variant and derive the triple and other parameters from it.

[jlebar]

I defer to Art.

[dcastagna]

Using already existing --offload parameters instead of adding a new one

[dcastagna]

In D117137#3238275, @tra wrote:

I think instead of setting the triple directly from the command line, we should start with adding another --cuda-gpu-arch (AKA --offload-arch) variant and derive the triple and other parameters from it.

As discussed via IM, I uploaded a patch that extends the command line options --offload instead of introducing a new flag.
@tra, what do you think?

[tra]

LGTM in general, modulo few nits.
Nit: looks like the changes need some clang-formatting.

clang/lib/Driver/Driver.cpp

123–124

You either need to rephrase the diag message in clang/include/clang/Basic/DiagnosticDriverKinds.td and remove the argument, or provide the "CUDA" or "HIP" as the argument. Passing a "" as is will result in an incomplete sentence.

157–158

Just return llvm::Triple("amdgcn-amd-amdhsa") ?

[yaxunl]

The title says --offline option, which should be --offload.

clang/include/clang/Driver/Options.td

1146

There are other offloading toolchains e.g. OpenMP or SYCL. This option only supports CUDA and HIP, so it is better to add "(CUDA and HIP only)".

[dcastagna]

Address tra@ and yaxunl@ comments.
Error out if offload is used without --emit-llvm

[dcastagna]

Address yaxunl@ comment in Options.td

[dcastagna]

In D117137#3259276, @yaxunl wrote:

The title says --offline option, which should be --offload.

Fixed that and addressed the other comments.
The last patch also adds a check to error out if --offload is used in CUDA without --emit-llvm, since that will result in a assert failing later on.

[dcastagna]

Fix invalid-offload-options.cpp test

[dcastagna]

Rebase on ToT

[tra]

clang/lib/Driver/Driver.cpp

154–156

What's expected to happen if someone specifies spirv64-nvidia ?

If someone uses spirv64-foo-bar I think it would match this condition.

Accepting spirv64-foo-bar, but not spirv64-nvidia-unknown would be somewhat odd. I think we either should check a fully-specified triple, or only check the parts that matter -- getArch() in this case, or, maybe, arch and vendor.

[dcastagna]

clang/lib/Driver/Driver.cpp

154–156

This part is the check for the hip offload triple and this patch did not change the logic for HIP (at least not intentionally), it should be the same as the logic specified in the current getHIPOffloadTargetTriple on ToT.
Happy to change it if you think it shuold be different though.

[dcastagna]

Remove unknown-unknown check from HIP offload logic

[dcastagna]

clang/lib/Driver/Driver.cpp

154–156

Removed the vendor and os check to make it consistent with the cuda logic.

[tra]

clang/lib/Driver/Driver.cpp

154–156

@linjamaki, @yaxunl -- are you OK with ignoring the vendor/OS parts for spirv triples?

[linjamaki]

SPIR-V target requires that the OS and the environment type is unknown (see TargetInfo::AllocateTarget and BaseSPIRTargetInfo). The clang would fail to create a SPIR-V target if there is an OS or environment component in the target string known by the Triple. This could lead to a misleading error message.

[yaxunl]

In D117137#3268548, @linjamaki wrote:

SPIR-V target requires that the OS and the environment type is unknown (see TargetInfo::AllocateTarget and BaseSPIRTargetInfo). The clang would fail to create a SPIR-V target if there is an OS or environment component in the target string known by the Triple. This could lead to a misleading error message.

Does that mean only "spirv{64}-unknown-unknown" is acceptable, or "spirv{64}-amd-unknown-unknown" is also acceptable?

One usage of vendor component in spirv triple is that it may be used to choose toolchain if there are multiple toolchains supporting spirv.

@mkuper What are intended use of OS and environment components of spirv triple?

[tra]

In D117137#3268548, @linjamaki wrote:

SPIR-V target requires that the OS and the environment type is unknown (see TargetInfo::AllocateTarget and BaseSPIRTargetInfo).

The problem is that LLVM's triple parser will set UnknownVendor for *any* vendor it does not know about. As I've pointed in the previous comment positively checking for an unknown vendor leads to a somewhat odd situation, when a triple "vpirv64-whoknowswhat" will be accepted, but "spirv64-suse" will not, even though both are equally nonsensical as far as spirv is concerned.

If SPIRV needs a vendor-specific treatment, then it probably needs a specific vendor enum for that. UnknownVendor is ill-suited for that purpose.

[linjamaki]

In D117137#3269365, @yaxunl wrote:

Does that mean only "spirv{64}-unknown-unknown" is acceptable, or "spirv{64}-amd-unknown-unknown" is also acceptable?

Having a vendor component in the triple seems to be acceptable for the SPIR-V target.

[tra]

In D117137#3269365, @yaxunl wrote:

Does that mean only "spirv{64}-unknown-unknown" is acceptable, or "spirv{64}-amd-unknown-unknown" is also acceptable?

My point is that unknown part of the triple is a catch-all for anything, including something invalid and should not be used for positive checks.
If we do not care about those parts of the triple ( accepting invalid triple would imply it), then we should not check those parts at all.
Otherwise it leads to a weird inconsistency -- invalid triple like spirv64-foo-baris accepted, but an equally nonsensical triple like spir64-suse-whateverwhich happens to use a known vendor or OS parts is not.

The bottom line is that if there's currently no known use of the vendor/OS/env parts of the triple, then we should not be checking them.
If we do want to accept specific triple, then appropriate enums should be used/added.

[yaxunl]

In D117137#3273330, @tra wrote:

In D117137#3269365, @yaxunl wrote:

Does that mean only "spirv{64}-unknown-unknown" is acceptable, or "spirv{64}-amd-unknown-unknown" is also acceptable?

My point is that unknown part of the triple is a catch-all for anything, including something invalid and should not be used for positive checks.
If we do not care about those parts of the triple ( accepting invalid triple would imply it), then we should not check those parts at all.
Otherwise it leads to a weird inconsistency -- invalid triple like spirv64-foo-baris accepted, but an equally nonsensical triple like spir64-suse-whateverwhich happens to use a known vendor or OS parts is not.

The bottom line is that if there's currently no known use of the vendor/OS/env parts of the triple, then we should not be checking them.
If we do want to accept specific triple, then appropriate enums should be used/added.

I get your point. TT.getVendor() == llvm::Triple::UnknownVendor and TT.getOS() == llvm::Triple::UnknownOS checks the processed vendor/OS string instead of the original string, which could be misleading.

Since SPIRV backend requires OS and environment to be unknown. It seems we'd better check the original OS and environment string in the Triple by splitting the triple by - and taking the 3rd and 4th element (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Triple.cpp#L795).

[dcastagna]

@yaxunl Are you OK landing this change as it is, without the check for OS and environment in getHIPOffloadTargetTriple?
We can follow up with patch that adds checks for in OS and environment in Triple.cpp. Is that what you meant?

[yaxunl]

In D117137#3274035, @dcastagna wrote:

@yaxunl Are you OK landing this change as it is, without the check for OS and environment in getHIPOffloadTargetTriple?
We can follow up with patch that adds checks for in OS and environment in Triple.cpp. Is that what you meant?

LGTM. @tra @linjamaki What do you think?

[jlebar]

Pushed for Daniele:

To github.com:llvm/llvm-project.git
   99d2582164c4..6eb826567af0  main -> main

clang/include/clang/Basic/DiagnosticDriverKinds.td

 def err_cc1_round_trip_mismatch : Error<
   "generated arguments do not match in round-trip">;
 def err_cc1_unbounded_vscale_min : Error<
   "minimum vscale must be an unsigned integer greater than 0">;
 
 def err_drv_ssp_missing_offset_argument : Error<
   "'%0' is used without '-mstack-protector-guard-offset', and there is no default">;
 
-def err_drv_only_one_offload_target_supported_in : Error<
-  "Only one offload target is supported in %0.">;
+def err_drv_only_one_offload_target_supported : Error<
+  "only one offload target is supported">;
 def err_drv_invalid_or_unsupported_offload_target : Error<
-  "Invalid or unsupported offload target: '%0'.">;
+  "invalid or unsupported offload target: '%0'">;
+def err_drv_cuda_offload_only_emit_bc : Error<
+  "CUDA offload target is supported only along with --emit-llvm">;
 }

clang/include/clang/Driver/Options.td

defm autolink : BoolFOption<"autolink",
   CodeGenOpts<"Autolink">, DefaultTrue,
   NegFlag<SetFalse, [CC1Option], "Disable generation of linker directives for automatic library linking">,
   PosFlag<SetTrue>>;
 
 // In the future this option will be supported by other offloading
 // languages and accept other values such as CPU/GPU architectures,
 // offload kinds and target aliases.
 def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
-  HelpText<"Specify comma-separated list of offloading target triples"
-           " (HIP only)">;
+  HelpText<"Specify comma-separated list of offloading target triples (CUDA and HIP only)">;
 
 // C++ Coroutines TS
 defm coroutines_ts : BoolFOption<"coroutines-ts",
   LangOpts<"Coroutines">, Default<cpp20.KeyPath>,
   PosFlag<SetTrue, [CC1Option], "Enable support for the C++ Coroutines TS">,
   NegFlag<SetFalse>>;
 
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,

clang/lib/Driver/Driver.cpp

 #include <unistd.h> // getpid
 #endif
 
 using namespace clang::driver;
 using namespace clang;
 using namespace llvm::opt;
 
 static llvm::Optional<llvm::Triple>
-getHIPOffloadTargetTriple(const Driver &D, const ArgList &Args) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-    auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
-
-    // HIP compilation flow does not support multiple targets for now. We need
-    // the HIPActionBuilder (and possibly the CudaActionBuilder{,Base}too) to
-    // support multiple tool chains first.
-    switch (HIPOffloadTargets.size()) {
-    default:
-      D.Diag(diag::err_drv_only_one_offload_target_supported_in) << "HIP";
-      return llvm::None;
-    case 0:
-      D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
-      return llvm::None;
-    case 1:
-      break;
-    }
-    llvm::Triple TT(HIPOffloadTargets[0]);
-    if (TT.getArch() == llvm::Triple::amdgcn &&
-        TT.getVendor() == llvm::Triple::AMD &&
-        TT.getOS() == llvm::Triple::AMDHSA)
-      return TT;
-    if (TT.getArch() == llvm::Triple::spirv64 &&
-        TT.getVendor() == llvm::Triple::UnknownVendor &&
-        TT.getOS() == llvm::Triple::UnknownOS)
+getOffloadTargetTriple(const Driver &D, const ArgList &Args) {
+  auto OffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+  // Offload compilation flow does not support multiple targets for now. We
+  // need the HIPActionBuilder (and possibly the CudaActionBuilder{,Base}too)
+  // to support multiple tool chains first.
+  switch (OffloadTargets.size()) {
+    D.Diag(diag::err_drv_only_one_offload_target_supported);
+  return llvm::Triple(OffloadTargets[0]);
+}
+
+static llvm::Optional<llvm::Triple>
+getNVIDIAOffloadTargetTriple(const Driver &D, const ArgList &Args,
+                             const llvm::Triple &HostTriple) {
+  if (!Args.hasArg(options::OPT_offload_EQ)) {
+    return llvm::Triple(HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
+                                                 : "nvptx-nvidia-cuda");
+  }
+  auto TT = getOffloadTargetTriple(D, Args);
+  if (TT && (TT->getArch() == llvm::Triple::spirv32 ||
+             TT->getArch() == llvm::Triple::spirv64)) {
+    if (Args.hasArg(options::OPT_emit_llvm))
       return TT;
-    D.Diag(diag::err_drv_invalid_or_unsupported_offload_target)
-        << HIPOffloadTargets[0];
+    D.Diag(diag::err_drv_cuda_offload_only_emit_bc);
     return llvm::None;
   }
-
-  static const llvm::Triple T("amdgcn-amd-amdhsa"); // Default HIP triple.
-  return T;
+  D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << TT->str();
+  return llvm::None;
+}
+static llvm::Optional<llvm::Triple>
+getHIPOffloadTargetTriple(const Driver &D, const ArgList &Args) {
+  if (!Args.hasArg(options::OPT_offload_EQ)) {
+    return llvm::Triple("amdgcn-amd-amdhsa"); // Default HIP triple.
+  }
+  auto TT = getOffloadTargetTriple(D, Args);
+  if (!TT)
+    return llvm::None;
+  if (TT->getArch() == llvm::Triple::amdgcn &&
+      TT->getVendor() == llvm::Triple::AMD &&
+      TT->getOS() == llvm::Triple::AMDHSA)
+    return TT;
+  if (TT->getArch() == llvm::Triple::spirv64)
+    return TT;
+  D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << TT->str();
+  return llvm::None;
 }
 
 // static
 std::string Driver::GetResourcesPath(StringRef BinaryPath,
                                      StringRef CustomResourceDir) {
   // Since the resource directory is embedded in the module hash, it's important
   // that all places that need it call this function, so that they get the
   // exact same string ("a/../b/" and "b/" get different hashes, for example).
 
bool IsHIP =
       C.getInputArgs().hasArg(options::OPT_hip_link);
   if (IsCuda && IsHIP) {
     Diag(clang::diag::err_drv_mix_cuda_hip);
     return;
   }
   if (IsCuda) {
     const ToolChain *HostTC = C.getSingleOffloadToolChain<Action::OFK_Host>();
     const llvm::Triple &HostTriple = HostTC->getTriple();
-    StringRef DeviceTripleStr;
     auto OFK = Action::OFK_Cuda;
-    DeviceTripleStr =
-        HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda" : "nvptx-nvidia-cuda";
-    llvm::Triple CudaTriple(DeviceTripleStr);
+    auto CudaTriple =
+        getNVIDIAOffloadTargetTriple(*this, C.getInputArgs(), HostTriple);
+    if (!CudaTriple)
+      return;
     // Use the CUDA and host triples as the key into the ToolChains map,
     // because the device toolchain we create depends on both.
-    auto &CudaTC = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
+    auto &CudaTC = ToolChains[CudaTriple->str() + "/" + HostTriple.str()];
     if (!CudaTC) {
       CudaTC = std::make_unique<toolchains::CudaToolChain>(
-          *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
+          *this, *CudaTriple, *HostTC, C.getInputArgs(), OFK);
     }
     C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   } else if (IsHIP) {
     if (auto *OMPTargetArg =
             C.getInputArgs().getLastArg(options::OPT_fopenmp_targets_EQ)) {
       Diag(clang::diag::err_drv_unsupported_opt_for_language_mode)
           << OMPTargetArg->getSpelling() << "HIP";
       return;

clang/test/Driver/cuda-device-triple.cu

+// REQUIRES: clang-driver
+
+// RUN: %clang -### -emit-llvm --cuda-device-only \
+// RUN:   -nocudalib -nocudainc --offload=spirv32-unknown-unknown -c %s 2>&1 | FileCheck %s
+
+// CHECK: clang{{.*}}" "-cc1" "-triple" "spirv32-unknown-unknown" {{.*}} "-fcuda-is-device" {{.*}}

clang/test/Driver/invalid-offload-options.cpp

 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // UNSUPPORTED: system-windows
 
 // RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
 // RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
 // RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
 // RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
 // RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
 // RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
 
-// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+// INVALID-TARGET: error: invalid or unsupported offload target: '{{.*}}'
 
 // In the future we should be able to specify multiple targets for HIP
 // compilation but currently it is not supported.
 //
 // RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
 // RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
 // RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
 // RUN: %clang -### -x hip -target x86_64-linux-gnu \
 // RUN:   --offload=foo --offload=bar \
 // RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
 // RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
 
-// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+// TOO-MANY-TARGETS: error: only one offload target is supported
 
 // RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
 // RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
 // RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
 
 // OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'