[clang] 255922b - [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-01-20T17:33:56-06:00
New Revision: 255922be7f008362bae16c708a2d90f60b4aba35

URL: https://github.com/llvm/llvm-project/commit/255922be7f008362bae16c708a2d90f60b4aba35
DIFF: https://github.com/llvm/llvm-project/commit/255922be7f008362bae16c708a2d90f60b4aba35.diff

LOG: [OpenMP] Clean up AMD handling for `-fopenmp-targets=amdgcn` arch inference

Previously we had some special handling here that errored out if
multiple architectures were detected. This isn't a problem anymore as
the runtime can handle multi-archicture binaries automatically. So it's
safe to simply take the first architecture that we know works. If users
use `--offload-arch=native` instead it will build for all the
architectures at the same time rather than just picking one. This patch
makes it consisten with the NVPTX version.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D142138

Added: 
    

Modified: 
    clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
    clang/test/Driver/amdgpu-openmp-system-arch-fail.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index c18394fc7355..ceef7b8cc8ee 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -29,32 +29,6 @@ using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
-namespace {
-
-static bool checkSystemForAMDGPU(const ArgList &Args, const AMDGPUToolChain &TC,
-                                 std::string &GPUArch) {
-  auto CheckError = [&](llvm::Error Err) -> bool {
-    std::string ErrMsg =
-        llvm::formatv("{0}", llvm::fmt_consume(std::move(Err)));
-    TC.getDriver().Diag(diag::err_drv_undetermined_gpu_arch)
-        << llvm::Triple::getArchTypeName(TC.getArch()) << ErrMsg << "-march";
-    return false;
-  };
-
-  auto ArchsOrErr = TC.getSystemGPUArchs(Args);
-  if (!ArchsOrErr)
-    return CheckError(ArchsOrErr.takeError());
-
-  if (ArchsOrErr->size() > 1)
-    if (!llvm::all_equal(*ArchsOrErr))
-      return CheckError(llvm::createStringError(
-          std::error_code(), "Multiple AMD GPUs found with 
diff erent archs"));
-
-  GPUArch = ArchsOrErr->front();
-  return true;
-}
-} // namespace
-
 AMDGPUOpenMPToolChain::AMDGPUOpenMPToolChain(const Driver &D,
                                              const llvm::Triple &Triple,
                                              const ToolChain &HostTC,
@@ -70,11 +44,8 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions(
     Action::OffloadKind DeviceOffloadingKind) const {
   HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
 
-  std::string GPUArch = DriverArgs.getLastArgValue(options::OPT_march_EQ).str();
-  if (GPUArch.empty()) {
-    if (!checkSystemForAMDGPU(DriverArgs, *this, GPUArch))
-      return;
-  }
+  StringRef GPUArch = DriverArgs.getLastArgValue(options::OPT_march_EQ);
+  assert(!GPUArch.empty() && "Must have an explicit GPU arch.");
 
   assert(DeviceOffloadingKind == Action::OFK_OpenMP &&
          "Only OpenMP offloading kinds are supported.");
@@ -115,9 +86,19 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(
         DAL->append(A);
 
     if (!DAL->hasArg(options::OPT_march_EQ)) {
-      std::string Arch = BoundArch.str();
-      if (BoundArch.empty())
-        checkSystemForAMDGPU(Args, *this, Arch);
+      StringRef Arch = BoundArch;
+      if (Arch.empty()) {
+        auto ArchsOrErr = getSystemGPUArchs(Args);
+        if (!ArchsOrErr) {
+          std::string ErrMsg =
+              llvm::formatv("{0}", llvm::fmt_consume(ArchsOrErr.takeError()));
+          getDriver().Diag(diag::err_drv_undetermined_gpu_arch)
+              << llvm::Triple::getArchTypeName(getArch()) << ErrMsg << "-march";
+          Arch = CudaArchToString(CudaArch::HIPDefault);
+        } else {
+          Arch = Args.MakeArgString(ArchsOrErr->front());
+        }
+      }
       DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), Arch);
     }
 

diff  --git a/clang/test/Driver/amdgpu-openmp-system-arch-fail.c b/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
index c8fd2472e04b..06f0b736b233 100644
--- a/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
+++ b/clang/test/Driver/amdgpu-openmp-system-arch-fail.c
@@ -17,11 +17,6 @@
 // RUN:   | FileCheck %s --check-prefix=NO-OUTPUT-ERROR
 // NO-OUTPUT-ERROR: error: cannot determine amdgcn architecture{{.*}}; consider passing it via '-march'
 
-// case when amdgpu_arch returns multiple gpus but all are 
diff erent
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp -fopenmp-targets=amdgcn-amd-amdhsa -nogpulib --amdgpu-arch-tool=%t/amdgpu_arch_
diff erent %s 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=MULTIPLE-OUTPUT-ERROR
-// MULTIPLE-OUTPUT-ERROR: error: cannot determine amdgcn architecture: Multiple AMD GPUs found with 
diff erent archs; consider passing it via '-march'
-
 // case when amdgpu_arch does not return anything with successful execution
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp -fopenmp-targets=amdgcn-amd-amdhsa -nogpulib --amdgpu-arch-tool=%t/amdgpu_arch_empty %s 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=EMPTY-OUTPUT