jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, tra, yaxunl.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
If an invalid architecture is set we currently return an empty string.
This will cause the offloading toolchain to continue to be built and
hit an assertion elsewhere due to the invalid architecture. This patch
fixes that so we now correctly exit.
Repository:
rG LLVM Github Monorepo
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,7 +4198,7 @@
(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)
@@ -4208,7 +4207,7 @@
}
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
