[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Pushed for Daniele:

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Justin Lebar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6eb826567af0: [Driver] Add CUDA support for --offload param 
(authored by dcastagna, committed by jlebar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- clang/test/Driver/invalid-offload-options.cpp
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -9,7 +9,7 @@
 // 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.
@@ -22,7 +22,7 @@
 // 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 \
Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -103,39 +103,58 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna added a comment.

@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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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-bar`is accepted, but an equally nonsensical triple like 
> `spir64-suse-whatever`which 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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

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-bar`is accepted, but an equally nonsensical triple like 
`spir64-suse-whatever`which 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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}-unkown-unkown" is acceptable, or 
"spirv{64}-amd-unkown-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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: linjamaki.
tra added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:153-155
+  if (TT->getArch() == llvm::Triple::spirv64 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)

dcastagna wrote:
> dcastagna wrote:
> > tra wrote:
> > > 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.
> > 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.
> Removed the vendor and os check to make it consistent with the cuda logic.
@linjamaki, @yaxunl -- are you OK with ignoring the vendor/OS parts for spirv 
triples? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna marked an inline comment as done.
dcastagna added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:153-155
+  if (TT->getArch() == llvm::Triple::spirv64 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)

dcastagna wrote:
> tra wrote:
> > 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.
> 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.
Removed the vendor and os check to make it consistent with the cuda logic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna updated this revision to Diff 402657.
dcastagna added a comment.

Remove unknown-unknown check from HIP offload logic


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- clang/test/Driver/invalid-offload-options.cpp
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -9,7 +9,7 @@
 // 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.
@@ -22,7 +22,7 @@
 // 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 \
Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -102,39 +102,58 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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
+getHIPOffloadTargetTriple(const Driver , const ArgList ) {
+  if 

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:153-155
+  if (TT->getArch() == llvm::Triple::spirv64 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)

tra wrote:
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:153-155
+  if (TT->getArch() == llvm::Triple::spirv64 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna updated this revision to Diff 402031.
dcastagna added a comment.

Rebase on ToT


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- clang/test/Driver/invalid-offload-options.cpp
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -9,7 +9,7 @@
 // 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.
@@ -22,7 +22,7 @@
 // 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 \
Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -102,39 +102,60 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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
+getHIPOffloadTargetTriple(const Driver , const ArgList ) {
+  if (!Args.hasArg(options::OPT_offload_EQ)) {
+

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna updated this revision to Diff 401813.
dcastagna added a comment.

Fix invalid-offload-options.cpp test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- clang/test/Driver/invalid-offload-options.cpp
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -9,7 +9,7 @@
 // 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.
@@ -22,7 +22,7 @@
 // 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 \
Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -102,39 +102,60 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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
+getHIPOffloadTargetTriple(const Driver , const ArgList ) {
+  if 

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna updated this revision to Diff 401760.
dcastagna marked 2 inline comments as done.
dcastagna added a comment.

Address yaxunl@ comment in Options.td


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu

Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -102,39 +102,60 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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
+getHIPOffloadTargetTriple(const Driver , const ArgList ) {
+  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 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)
+return TT;
+  D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << TT->str();
+  return llvm::None;
 }
 
 // static
@@ -704,17 +725,17 @@
   if (IsCuda) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = 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(), 

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna updated this revision to Diff 401759.
dcastagna marked an inline comment as done.
dcastagna retitled this revision from "[Driver] Add CUDA support for --offline 
param" to "[Driver] Add CUDA support for --offload param".
dcastagna added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117137/new/

https://reviews.llvm.org/D117137

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-device-triple.cu

Index: clang/test/Driver/cuda-device-triple.cu
===
--- /dev/null
+++ clang/test/Driver/cuda-device-triple.cu
@@ -0,0 +1,6 @@
+// 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" {{.*}}
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -102,39 +102,60 @@
 using namespace llvm::opt;
 
 static llvm::Optional
-getHIPOffloadTargetTriple(const Driver , const ArgList ) {
-  if (Args.hasArg(options::OPT_offload_EQ)) {
-auto HIPOffloadTargets = Args.getAllArgValues(options::OPT_offload_EQ);
+getOffloadTargetTriple(const Driver , const ArgList ) {
+  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()) {
+  default:
+D.Diag(diag::err_drv_only_one_offload_target_supported);
+return llvm::None;
+  case 0:
+D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << "";
+return llvm::None;
+  case 1:
+break;
+  }
+  return llvm::Triple(OffloadTargets[0]);
+}
 
-// 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)
+static llvm::Optional
+getNVIDIAOffloadTargetTriple(const Driver , const ArgList ,
+ const llvm::Triple ) {
+  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
+getHIPOffloadTargetTriple(const Driver , const ArgList ) {
+  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 &&
+  TT->getVendor() == llvm::Triple::UnknownVendor &&
+  TT->getOS() == llvm::Triple::UnknownOS)
+return TT;
+  D.Diag(diag::err_drv_invalid_or_unsupported_offload_target) << TT->str();
+  return llvm::None;
 }
 
 // static
@@ -704,17 +725,17 @@
   if (IsCuda) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
-StringRef DeviceTripleStr;
 auto OFK = Action::OFK_Cuda;
-DeviceTripleStr =
-HostTriple.isArch64Bit()