[PATCH] D88377: Diagnose invalid target ID for AMDGPU toolchain for assembler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 295163.
yaxunl marked an inline comment as done.
yaxunl added a comment.

fix bug


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

https://reviews.llvm.org/D88377

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/amdgpu-invalid-target-id.s


Index: clang/test/Driver/amdgpu-invalid-target-id.s
===================================================================
--- /dev/null
+++ clang/test/Driver/amdgpu-invalid-target-id.s
@@ -0,0 +1,45 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: not %clang -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx908xnack -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=NOPLUS %s
+
+// NOPLUS: error: Invalid target ID: gfx908xnack
+
+// RUN: not %clang -target amdgcn-amd-amdpal \
+// RUN:   -mcpu=gfx908:xnack+:xnack+ -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=ORDER %s
+
+// ORDER: error: Invalid target ID: gfx908:xnack+:xnack+
+
+// RUN: not %clang -target amdgcn--mesa3d \
+// RUN:   -mcpu=gfx908:unknown+ -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=UNK %s
+
+// UNK: error: Invalid target ID: gfx908:unknown+
+
+// RUN: not %clang -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx908:sram-ecc+:unknown+ -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=MIXED %s
+
+// MIXED: error: Invalid target ID: gfx908:sram-ecc+:unknown+
+
+// RUN: not %clang -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx900:sram-ecc+ -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=UNSUP %s
+
+// UNSUP: error: Invalid target ID: gfx900:sram-ecc+
+
+// RUN: not %clang -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx900:xnack -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=NOSIGN %s
+
+// NOSIGN: error: Invalid target ID: gfx900:xnack
+
+// RUN: not %clang -target amdgcn-amd-amdhsa \
+// RUN:   -mcpu=gfx900+xnack -nostdlib \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=NOCOLON %s
+
+// NOCOLON: error: Invalid target ID: gfx900+xnack
Index: clang/lib/Driver/ToolChains/HIP.cpp
===================================================================
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -235,8 +235,7 @@
     Action::OffloadKind DeviceOffloadingKind) const {
   HostTC.addClangTargetOptions(DriverArgs, CC1Args, DeviceOffloadingKind);
 
-  // Allow using target ID in --offload-arch.
-  StringRef GpuArch = translateTargetID(DriverArgs, CC1Args);
+  StringRef GpuArch = getGPUArch(DriverArgs);
   assert(!GpuArch.empty() && "Must have an explicit GPU arch.");
   (void) GpuArch;
   assert(DeviceOffloadingKind == Action::OFK_HIP &&
@@ -348,6 +347,7 @@
   if (!BoundArch.empty()) {
     DAL->eraseArg(options::OPT_mcpu_EQ);
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ), BoundArch);
+    checkTargetID(*DAL);
   }
 
   return DAL;
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===================================================================
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -94,11 +94,10 @@
   bool shouldSkipArgument(const llvm::opt::Arg *Arg) const;
 
 protected:
-  /// Translate -mcpu option containing target ID to cc1 options.
-  /// Returns the GPU name.
-  StringRef translateTargetID(const llvm::opt::ArgList &DriverArgs,
-                              llvm::opt::ArgStringList &CC1Args) const;
+  /// Check and diagnose invalid target ID specified by -mcpu.
+  void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
 
+  /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 };
 
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===================================================================
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -433,6 +433,8 @@
       DAL->append(A);
   }
 
+  checkTargetID(*DAL);
+
   if (!Args.getLastArgValue(options::OPT_x).equals("cl"))
     return DAL;
 
@@ -525,8 +527,6 @@
     const llvm::opt::ArgList &DriverArgs,
     llvm::opt::ArgStringList &CC1Args,
     Action::OffloadKind DeviceOffloadingKind) const {
-  // Allow using target ID in -mcpu.
-  translateTargetID(DriverArgs, CC1Args);
   // Default to "hidden" visibility, as object level linking will not be
   // supported for the foreseeable future.
   if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
@@ -543,21 +543,17 @@
       getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ));
 }
 
-StringRef
-AMDGPUToolChain::translateTargetID(const llvm::opt::ArgList &DriverArgs,
-                                   llvm::opt::ArgStringList &CC1Args) const {
+void AMDGPUToolChain::checkTargetID(
+    const llvm::opt::ArgList &DriverArgs) const {
   StringRef TargetID = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
   if (TargetID.empty())
-    return StringRef();
+    return;
 
   llvm::StringMap<bool> FeatureMap;
   auto OptionalGpuArch = parseTargetID(getTriple(), TargetID, &FeatureMap);
   if (!OptionalGpuArch) {
     getDriver().Diag(clang::diag::err_drv_bad_target_id) << TargetID;
-    return StringRef();
   }
-
-  return OptionalGpuArch.getValue();
 }
 
 void ROCMToolChain::addClangTargetOptions(

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