[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG359e4a824731: [Clang] Parse toolchain-specific offloading 
arguments directly (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129435

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4355,7 +4355,17 @@
 return KnownArchs.lookup(TC);
 
   llvm::DenseSet Archs;
-  for (auto &Arg : Args) {
+  for (auto *Arg : Args) {
+// Extract any '--[no-]offload-arch' arguments intended for this toolchain.
+std::unique_ptr ExtractedArg = nullptr;
+if (Arg->getOption().matches(options::OPT_Xopenmp_target_EQ) &&
+ToolChain::getOpenMPTriple(Arg->getValue(0)) == TC->getTriple()) {
+  Arg->claim();
+  unsigned Index = Args.getBaseArgs().MakeIndex(Arg->getValue(1));
+  ExtractedArg = getOpts().ParseOneArg(Args, Index);
+  Arg = ExtractedArg.get();
+}
+
 if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
   for (StringRef Arch : llvm::split(Arg->getValue(), ","))
 Archs.insert(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
@@ -4425,8 +4435,7 @@
 // Get the product of all bound architectures and toolchains.
 SmallVector> TCAndArchs;
 for (const ToolChain *TC : ToolChains)
-  for (StringRef Arch : getOffloadArchs(
-   C, C.getArgsForToolChain(TC, "generic", Kind), Kind, TC))
+  for (StringRef Arch : getOffloadArchs(C, Args, Kind, TC))
 TCAndArchs.push_back(std::make_pair(TC, Arch));
 
 for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4355,7 +4355,17 @@
 return KnownArchs.lookup(TC);
 
   llvm::DenseSet Archs;
-  for (auto &Arg : Args) {
+  for (auto *Arg : Args) {
+// Extract any '--[no-]offload-arch' arguments intended for this toolchain.
+std::unique_ptr ExtractedArg = nullptr;
+if (Arg->getOption().matches(options::OPT_Xopenmp_target_EQ) &&
+ToolChain::getOpenMPTriple(Arg->getValue(0)) == TC->getTriple()) {
+  Arg->claim();
+  unsigned Index = Args.getBaseArgs().MakeIndex(Arg->getValue(1));
+  ExtractedArg = getOpts().ParseOneArg(Args, Index);
+  Arg = ExtractedArg.get();
+}
+
 if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
   for (StringRef Arch : llvm::split(Arg->getValue(), ","))
 Archs.insert(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
@@ -4425,8 +4435,7 @@
 // Get the product of all bound architectures and toolchains.
 SmallVector> TCAndArchs;
 for (const ToolChain *TC : ToolChains)
-  for (StringRef Arch : getOffloadArchs(
-   C, C.getArgsForToolChain(TC, "generic", Kind), Kind, TC))
+  for (StringRef Arch : getOffloadArchs(C, Args, Kind, TC))
 TCAndArchs.push_back(std::make_pair(TC, Arch));
 
 for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129435

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

[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D129435#3642450 , @yaxunl wrote:

> need a test

There's a test `clang/test/Driver/openmp-offload-gpu-new.c` already making sure 
that the following works, so this patch should maintain the same functionality 
we had before.

  clang input.c -fopenmp -fopenmp-targets=nvptx64,amdgcn 
-Xopenmp-target=nvptx64 --offload-arch=sm_70 -Xopenmp-target=amdgcn 
--offload-arch=gfx908




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129435

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

[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

need a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129435

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

[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

LG, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129435

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

[PATCH] D129435: [Clang] Parse toolchain-specific offloading arguments directly

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, tra, yaxunl, MaskRay.
Herald added subscribers: kosarev, StephenFan, tpr.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

OpenMP supports multiple offloading toolchains and architectures. In
order to support this we originally used `getArgsForToolchain` to get
the arguments only intended for each toolchain. This allowed users to
manually specify if an `--offload-arch=` argument was intended for which
toolchain using `-Xopenmp-target=` or other methods. For example,

  clang input.c -fopenmp -fopenmp-targets=nvptx64,amdgcn 
-Xopenmp-target=nvptx64 --offload-arch=sm_70 -Xopenmp-target=amdgcn 
--offload-arch=gfx908

However, this was causing problems with the AMDGPU toolchain. This is
because the AMDGPU toolchain for OpenMP uses an `amdgpu` arch to determine the
architecture. If this tool is not availible the compiler will exit with an error
even when manually specifying the architecture. This patch pulls out the logic 
in
`getArgsForToolchain` and specializes it for extracting `--offload-arch`
arguments to avoid this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129435

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4355,7 +4355,17 @@
 return KnownArchs.lookup(TC);
 
   llvm::DenseSet Archs;
-  for (auto &Arg : Args) {
+  for (auto *Arg : Args) {
+// Extract any '--[no-]offload-arch' arguments intended for this toolchain.
+std::unique_ptr ExtractedArg = nullptr;
+if (Arg->getOption().matches(options::OPT_Xopenmp_target_EQ) &&
+ToolChain::getOpenMPTriple(Arg->getValue(0)) == TC->getTriple()) {
+  Arg->claim();
+  unsigned Index = Args.getBaseArgs().MakeIndex(Arg->getValue(1));
+  ExtractedArg = getOpts().ParseOneArg(Args, Index);
+  Arg = ExtractedArg.get();
+}
+
 if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
   for (StringRef Arch : llvm::split(Arg->getValue(), ","))
 Archs.insert(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
@@ -4425,8 +4435,7 @@
 // Get the product of all bound architectures and toolchains.
 SmallVector> TCAndArchs;
 for (const ToolChain *TC : ToolChains)
-  for (StringRef Arch : getOffloadArchs(
-   C, C.getArgsForToolChain(TC, "generic", Kind), Kind, TC))
+  for (StringRef Arch : getOffloadArchs(C, Args, Kind, TC))
 TCAndArchs.push_back(std::make_pair(TC, Arch));
 
 for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4355,7 +4355,17 @@
 return KnownArchs.lookup(TC);
 
   llvm::DenseSet Archs;
-  for (auto &Arg : Args) {
+  for (auto *Arg : Args) {
+// Extract any '--[no-]offload-arch' arguments intended for this toolchain.
+std::unique_ptr ExtractedArg = nullptr;
+if (Arg->getOption().matches(options::OPT_Xopenmp_target_EQ) &&
+ToolChain::getOpenMPTriple(Arg->getValue(0)) == TC->getTriple()) {
+  Arg->claim();
+  unsigned Index = Args.getBaseArgs().MakeIndex(Arg->getValue(1));
+  ExtractedArg = getOpts().ParseOneArg(Args, Index);
+  Arg = ExtractedArg.get();
+}
+
 if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
   for (StringRef Arch : llvm::split(Arg->getValue(), ","))
 Archs.insert(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
@@ -4425,8 +4435,7 @@
 // Get the product of all bound architectures and toolchains.
 SmallVector> TCAndArchs;
 for (const ToolChain *TC : ToolChains)
-  for (StringRef Arch : getOffloadArchs(
-   C, C.getArgsForToolChain(TC, "generic", Kind), Kind, TC))
+  for (StringRef Arch : getOffloadArchs(C, Args, Kind, TC))
 TCAndArchs.push_back(std::make_pair(TC, Arch));
 
 for (unsigned I = 0, E = TCAndArchs.size(); I != E; ++I)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits