[PATCH] D139045: [HIP] support --offload-arch=native

[Yaxun Liu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
yaxunl marked 2 inline comments as done.
Closed by commit rGe8fd998e6194: [HIP] support --offload-arch=native (authored 
by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D139045?vs=482240=482463#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3067,6 +3067,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == "native" &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3067,6 +3067,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == "native" &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139045: [HIP] support --offload-arch=native

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:3058
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;

tra wrote:
> Nit: It's used only once. May as well compare with "native" directly.
will do



Comment at: clang/lib/Driver/Driver.cpp:3076
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {

tra wrote:
> Does it guarantee that a returned list of architectures contains only 
> canonical names?
Yes. amdgpu-arch calls ROCm runtime to get GPU arch, which uses canonical gfx 
names 
https://github.com/RadeonOpenCompute/ROCR-Runtime/blob/adae6c61e10d371f7cbc3d0e94ae2c070cab18a4/src/core/runtime/isa.cpp#L245


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

https://reviews.llvm.org/D139045

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

[PATCH] D139045: [HIP] support --offload-arch=native

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/Driver.cpp:3058
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;

Nit: It's used only once. May as well compare with "native" directly.



Comment at: clang/lib/Driver/Driver.cpp:3076
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {

Does it guarantee that a returned list of architectures contains only canonical 
names?


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

https://reviews.llvm.org/D139045

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

[PATCH] D139045: [HIP] support --offload-arch=native

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 482240.
yaxunl added a comment.

fix error handling


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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139045: [HIP] support --offload-arch=native

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 482234.
yaxunl retitled this revision from "[HIP] use detected GPU in --offload-arch" 
to "[HIP] support --offload-arch=native".
yaxunl edited the summary of this revision.
yaxunl added a comment.

use detected GPU when --offload-arch=native is specified based on RFC discussion


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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,15 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+if (!TC->detectSystemGPUs(Args, GPUs)) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+}
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList ,
std::string ) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
+   SmallVector ) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList ) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList ) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList ,
-   SmallVector ) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,15 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+if (!TC->detectSystemGPUs(Args, GPUs)) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+}
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits