jhuber6 updated this revision to Diff 467188.
jhuber6 added a comment.

Fix missing `None` return on HIP.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135791

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cuda-phases.cu

Index: clang/test/Driver/cuda-phases.cu
===================================================================
--- clang/test/Driver/cuda-phases.cu
+++ clang/test/Driver/cuda-phases.cu
@@ -318,3 +318,16 @@
 // LTO-NEXT: 14: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {2}, "device-cuda (powerpc64le-ibm-linux-gnu)" {13}, ir
 // LTO-NEXT: 15: backend, {14}, assembler, (host-cuda)
 // LTO-NEXT: 16: assembler, {15}, object, (host-cuda)
+
+//
+// Test that the new driver does not create actions for invalid architectures.
+//
+// RUN: %clang -### -target powerpc64le-ibm-linux-gnu --offload-new-driver \
+// RUN:        -ccc-print-phases --offload-arch=sm_999 -fgpu-rdc -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=INVALID-ARCH %s
+//      INVALID-ARCH: error: unsupported CUDA gpu architecture: sm_999
+// INVALID-ARCH-NEXT: 0: input, "[[INPUT:.+]]", cuda, (host-cuda)
+// INVALID-ARCH-NEXT: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
+// INVALID-ARCH-NEXT: 2: compiler, {1}, ir, (host-cuda)
+// INVALID-ARCH-NEXT: 3: backend, {2}, assembler, (host-cuda)
+// INVALID-ARCH-NEXT: 4: assembler, {3}, object, (host-cuda)
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4188,10 +4188,9 @@
 
 /// Returns the canonical name for the offloading architecture when using a HIP
 /// or CUDA architecture.
-static StringRef getCanonicalArchString(Compilation &C,
-                                        const llvm::opt::DerivedArgList &Args,
-                                        StringRef ArchStr,
-                                        const llvm::Triple &Triple) {
+static Optional<StringRef>
+getCanonicalArchString(Compilation &C, const llvm::opt::DerivedArgList &Args,
+                       StringRef ArchStr, const llvm::Triple &Triple) {
   // Lookup the CUDA / HIP architecture string. Only report an error if we were
   // expecting the triple to be only NVPTX / AMDGPU.
   CudaArch Arch = StringToCudaArch(getProcessorFromTargetID(Triple, ArchStr));
@@ -4199,16 +4198,16 @@
       (Arch == CudaArch::UNKNOWN || !IsNVIDIAGpuArch(Arch))) {
     C.getDriver().Diag(clang::diag::err_drv_offload_bad_gpu_arch)
         << "CUDA" << ArchStr;
-    return StringRef();
+    return None;
   } else if (Triple.isAMDGPU() &&
              (Arch == CudaArch::UNKNOWN || !IsAMDGpuArch(Arch))) {
     C.getDriver().Diag(clang::diag::err_drv_offload_bad_gpu_arch)
         << "HIP" << ArchStr;
-    return StringRef();
+    return None;
   }
 
   if (IsNVIDIAGpuArch(Arch))
-    return Args.MakeArgStringRef(CudaArchToString(Arch));
+    return StringRef(Args.MakeArgStringRef(CudaArchToString(Arch)));
 
   if (IsAMDGpuArch(Arch)) {
     llvm::StringMap<bool> Features;
@@ -4221,7 +4220,8 @@
       C.setContainsError();
       return StringRef();
     }
-    return Args.MakeArgStringRef(getCanonicalTargetID(*Arch, Features));
+    return StringRef(
+        Args.MakeArgStringRef(getCanonicalTargetID(*Arch, Features)));
   }
 
   // If the input isn't CUDA or HIP just return the architecture.
@@ -4273,15 +4273,28 @@
       Arg = ExtractedArg.get();
     }
 
+    // Add or remove the seen architectures in order of appearance. If an
+    // invalid architecture is given we simply exit.
     if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
-      for (StringRef Arch : llvm::split(Arg->getValue(), ","))
-        Archs.insert(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
+      for (StringRef Arch : llvm::split(Arg->getValue(), ",")) {
+        if (auto ArchStr =
+                getCanonicalArchString(C, Args, Arch, TC->getTriple()))
+          Archs.insert(*ArchStr);
+        else
+          return Archs;
+      }
     } else if (Arg->getOption().matches(options::OPT_no_offload_arch_EQ)) {
       for (StringRef Arch : llvm::split(Arg->getValue(), ",")) {
-        if (Arch == StringRef("all"))
+        if (Arch == StringRef("all")) {
           Archs.clear();
-        else
-          Archs.erase(getCanonicalArchString(C, Args, Arch, TC->getTriple()));
+        } else if (auto Str =
+                       getCanonicalArchString(C, Args, Arch, TC->getTriple())) {
+          auto ArchStr = getCanonicalArchString(C, Args, Arch, TC->getTriple());
+          Archs.erase(*ArchStr);
+        } else {
+
+          return Archs;
+        }
       }
     }
   }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to