[HIP] Check compatibility of -fgpu-sanitize with offload arch

[yaxunl]

Summary

-fgpu-sanitize is incompatible with offload arch containing xnack-.

This patch checks that.

[tra]

clang/lib/Driver/ToolChains/AMDGPU.h

114–115

I'd use a struct instead. It makes things more readable in all the other other places where we don't have the exact return type spelled out.

clang/lib/Driver/ToolChains/HIP.cpp

473

What if FeatureMap is None?
Considering that it's derived from user input, this should be properly checked and diagnosed, if we didn't get the feature map.
At the very least there should be an assert, if we ensure somewhere else that we always get a valid FeatureMap here.

[yaxunl]

revised by Artem's comments

clang/lib/Driver/ToolChains/AMDGPU.h

public:
 
   /// Uses amdgpu_arch tool to get arch of the system GPU. Will return error
   /// if unable to find one.
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
                                std::string &GPUArch) const;
 
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
-  void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
+  virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
+
+  /// The struct type returned by getParsedTargetID.
+  struct ParsedTargetIDType {
+    Optional<std::string> OptionalTargetID;
+    Optional<std::string> OptionalGPUArch;
+    Optional<llvm::StringMap<bool>> OptionalFeatures;
+  };
+
+  /// Get target ID, GPU arch, and target ID features if the target ID is
+  /// specified and valid.
+  ParsedTargetIDType
+  getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const;
 
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
   llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
                                SmallVector<std::string, 1> &GPUArchs) const;
 };
 

clang/lib/Driver/ToolChains/AMDGPU.cpp

 }
 
 StringRef
 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const {
   return getProcessorFromTargetID(
       getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ));
 }
 
-void AMDGPUToolChain::checkTargetID(
-    const llvm::opt::ArgList &DriverArgs) const {
+AMDGPUToolChain::ParsedTargetIDType
+AMDGPUToolChain::getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const {
   StringRef TargetID = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
   if (TargetID.empty())
-    return;
+    return {None, None, None};
 
   llvm::StringMap<bool> FeatureMap;
   auto OptionalGpuArch = parseTargetID(getTriple(), TargetID, &FeatureMap);
-  if (!OptionalGpuArch) {
-    getDriver().Diag(clang::diag::err_drv_bad_target_id) << TargetID;
+  if (!OptionalGpuArch)
+    return {TargetID.str(), None, None};
+
+  return {TargetID.str(), OptionalGpuArch.getValue().str(), FeatureMap};
+}
+
+void AMDGPUToolChain::checkTargetID(
+    const llvm::opt::ArgList &DriverArgs) const {
+  auto PTID = getParsedTargetID(DriverArgs);
+  if (PTID.OptionalTargetID && !PTID.OptionalGPUArch) {
+    getDriver().Diag(clang::diag::err_drv_bad_target_id)
+        << PTID.OptionalTargetID.getValue();
   }
 }
 
 llvm::Error
 AMDGPUToolChain::detectSystemGPUs(const ArgList &Args,
                                   SmallVector<std::string, 1> &GPUArchs) const {
   std::string Program;
   if (Arg *A = Args.getLastArg(options::OPT_amdgpu_arch_tool_EQ))

clang/lib/Driver/ToolChains/HIP.h

public:
 
   VersionTuple
   computeMSVCVersion(const Driver *D,
                      const llvm::opt::ArgList &Args) const override;
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
   const ToolChain &HostTC;
+  void checkTargetID(const llvm::opt::ArgList &DriverArgs) const override;
 
 protected:
   Tool *buildLinker() const override;
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_HIP_H

clang/lib/Driver/ToolChains/HIP.cpp

if (!BCLibArgs.empty()) {
     if (llvm::sys::fs::exists(InstLib))
       BCLibs.push_back(InstLib.str());
     else
       getDriver().Diag(diag::err_drv_no_such_file) << InstLib;
   }
 
   return BCLibs;
 }
+
+void HIPToolChain::checkTargetID(const llvm::opt::ArgList &DriverArgs) const {
+  auto PTID = getParsedTargetID(DriverArgs);
+  if (PTID.OptionalTargetID && !PTID.OptionalGPUArch) {
+    getDriver().Diag(clang::diag::err_drv_bad_target_id)
+        << PTID.OptionalTargetID.getValue();
+    return;
+  }
+
+  assert(PTID.OptionalFeatures && "Invalid return from getParsedTargetID");
+  auto &FeatureMap = PTID.OptionalFeatures.getValue();
+  // Sanitizer is not supported with xnack-.
+  if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
+                         options::OPT_fno_gpu_sanitize, false)) {
+    auto Loc = FeatureMap.find("xnack");
+    if (Loc != FeatureMap.end() && !Loc->second) {
+      auto &Diags = getDriver().getDiags();
+      auto DiagID = Diags.getCustomDiagID(
+          DiagnosticsEngine::Error,
+          "'-fgpu-sanitize' is not compatible with offload arch '%0'. "
+          "Use an offload arch without 'xnack-' instead");
+      Diags.Report(DiagID) << PTID.OptionalTargetID.getValue();
+    }
+  }
+}

clang/test/Driver/hip-sanitize-options.hip

 // RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
 // RUN:   %s 2>&1 | FileCheck -check-prefixes=RDC %s
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900 \
 // RUN:   -fsanitize=address -fgpu-sanitize \
 // RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \
 // RUN:   %s 2>&1 | FileCheck -check-prefixes=FAIL %s
 
+// RUN: %clang -### -target x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack- \
+// RUN:   -fsanitize=address -fgpu-sanitize \
+// RUN:   -nogpuinc --rocm-path=%S/Inputs/rocm \
+// RUN:   %s 2>&1 | FileCheck -check-prefix=XNACK %s
+
 // CHECK-NOT: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-fsanitize=address"}}
 // CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}}
 // CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
 
 // NORDC: {{"[^"]*clang[^"]*".* "-fcuda-is-device".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
 // NORDC: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
 // NORDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
 
 // RDC: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}}
 // RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]"
 // RDC: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}}
 
 // FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer
+
+// XNACK: error: '-fgpu-sanitize' is not compatible with offload arch 'gfx900:xnack-'. Use an offload arch without 'xnack-' instead