From f1e7ecaa18a7b1c77fc95e4b83d3db02c4e2d211 Mon Sep 17 00:00:00 2001 From: Ron Lieberman Date: Sat, 2 Apr 2022 13:25:50 +0000 Subject: [PATCH] Revert "[AMDPU][Sanitizer] Refactor sanitizer options handling for AMDGPU Toolchain" This reverts commit cc2139524f77248c7e147d4cc3befb31fe3e6daa. failed a few buildbots --- clang/include/clang/Basic/DiagnosticDriverKinds.td | 3 -- clang/lib/Driver/ToolChains/AMDGPU.cpp | 36 ------------------ clang/lib/Driver/ToolChains/AMDGPU.h | 6 --- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp | 4 +- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h | 4 -- clang/lib/Driver/ToolChains/HIPAMD.cpp | 44 +++++++++++++++++++++- clang/test/Driver/hip-sanitize-options.hip | 2 +- 7 files changed, 45 insertions(+), 54 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index fa17af5..63913b9 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -68,9 +68,6 @@ def err_drv_no_rocm_device_lib : Error< "cannot find ROCm device library%select{| for %1|for ABI version %1}0; provide its path via " "'--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build " "without ROCm device library">; -def err_drv_no_asan_rt_lib : Error< - "AMDGPU address sanitizer runtime library (asanrtl) is not found. " - "Please install ROCm device library which supports address sanitizer">; def err_drv_no_hip_runtime : Error< "cannot find HIP runtime; provide its path via '--rocm-path', or pass " "'-nogpuinc' to build without HIP runtime">; diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index 56b3a2d..123d926 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -911,42 +911,6 @@ RocmInstallationDetector::getCommonBitcodeLibs( return BCLibs; } -bool AMDGPUToolChain::shouldSkipSanitizeOption( - const ToolChain &TC, const llvm::opt::ArgList &DriverArgs, - StringRef TargetID, const llvm::opt::Arg *A) const { - // For actions without targetID, do nothing. - if (TargetID.empty()) - return false; - Option O = A->getOption(); - if (!O.matches(options::OPT_fsanitize_EQ)) - return false; - - if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize, - options::OPT_fno_gpu_sanitize, true)) - return true; - - auto &Diags = TC.getDriver().getDiags(); - - // For simplicity, we only allow -fsanitize=address - SanitizerMask K = parseSanitizerValue(A->getValue(), /*AllowGroups=*/false); - if (K != SanitizerKind::Address) - return true; - - llvm::StringMap FeatureMap; - auto OptionalGpuArch = parseTargetID(TC.getTriple(), TargetID, &FeatureMap); - - assert(OptionalGpuArch && "Invalid Target ID"); - (void)OptionalGpuArch; - auto Loc = FeatureMap.find("xnack"); - if (Loc == FeatureMap.end() || !Loc->second) { - Diags.Report( - clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature) - << A->getAsString(DriverArgs) << TargetID << "xnack+"; - return true; - } - return false; -} - bool AMDGPUToolChain::shouldSkipArgument(const llvm::opt::Arg *A) const { Option O = A->getOption(); if (O.matches(options::OPT_fPIE) || O.matches(options::OPT_fpie)) diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index 6d8bb82..ddcc124 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -99,12 +99,6 @@ public: /// Needed for translating LTO options. const char *getDefaultLinker() const override { return "ld.lld"; } - /// Should skip Sanitize options - bool shouldSkipSanitizeOption(const ToolChain &TC, - const llvm::opt::ArgList &DriverArgs, - StringRef TargetID, - const llvm::opt::Arg *A) const; - /// Should skip argument. bool shouldSkipArgument(const llvm::opt::Arg *Arg) const; diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp index 2989aab..998bb0b 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp @@ -16,7 +16,6 @@ #include "clang/Driver/DriverDiagnostic.h" #include "clang/Driver/InputInfo.h" #include "clang/Driver/Options.h" -#include "clang/Driver/SanitizerArgs.h" #include "clang/Driver/Tool.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/FileSystem.h" @@ -305,8 +304,7 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs( if (DeviceOffloadKind == Action::OFK_OpenMP) { for (Arg *A : Args) - if (!shouldSkipSanitizeOption(*this, Args, BoundArch, A) && - !llvm::is_contained(*DAL, A)) + if (!llvm::is_contained(*DAL, A)) DAL->append(A); std::string Arch = DAL->getLastArgValue(options::OPT_march_EQ).str(); diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h index ff0a81f..233256b 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h @@ -93,10 +93,6 @@ public: SanitizerMask getSupportedSanitizers() const override; - RocmInstallationDetector getRocmInstallationLoc() const { - return RocmInstallation; - } - VersionTuple computeMSVCVersion(const Driver *D, const llvm::opt::ArgList &Args) const override; diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp index 07d817f..9f0ac62 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp @@ -35,6 +35,43 @@ using namespace llvm::opt; #define NULL_FILE "/dev/null" #endif +static bool shouldSkipSanitizeOption(const ToolChain &TC, + const llvm::opt::ArgList &DriverArgs, + StringRef TargetID, + const llvm::opt::Arg *A) { + // For actions without targetID, do nothing. + if (TargetID.empty()) + return false; + Option O = A->getOption(); + if (!O.matches(options::OPT_fsanitize_EQ)) + return false; + + if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true)) + return true; + + auto &Diags = TC.getDriver().getDiags(); + + // For simplicity, we only allow -fsanitize=address + SanitizerMask K = parseSanitizerValue(A->getValue(), /*AllowGroups=*/false); + if (K != SanitizerKind::Address) + return true; + + llvm::StringMap FeatureMap; + auto OptionalGpuArch = parseTargetID(TC.getTriple(), TargetID, &FeatureMap); + + assert(OptionalGpuArch && "Invalid Target ID"); + (void)OptionalGpuArch; + auto Loc = FeatureMap.find("xnack"); + if (Loc == FeatureMap.end() || !Loc->second) { + Diags.Report( + clang::diag::warn_drv_unsupported_option_for_offload_arch_req_feature) + << A->getAsString(DriverArgs) << TargetID << "xnack+"; + return true; + } + return false; +} + void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA, const InputInfoList &Inputs, const InputInfo &Output, @@ -300,7 +337,12 @@ HIPAMDToolChain::getHIPDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { getSanitizerArgs(DriverArgs).needsAsanRt()) { auto AsanRTL = RocmInstallation.getAsanRTLPath(); if (AsanRTL.empty()) { - getDriver().Diag(diag::err_drv_no_asan_rt_lib); + unsigned DiagID = getDriver().getDiags().getCustomDiagID( + DiagnosticsEngine::Error, + "AMDGPU address sanitizer runtime library (asanrtl) is not found. " + "Please install ROCm device library which supports address " + "sanitizer"); + getDriver().Diag(DiagID); return {}; } else BCLibs.push_back({AsanRTL.str(), /*ShouldInternalize=*/false}); diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip index b166a02..51111d2 100644 --- a/clang/test/Driver/hip-sanitize-options.hip +++ b/clang/test/Driver/hip-sanitize-options.hip @@ -62,7 +62,7 @@ // RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]" // RDC-NOT: {{"[^"]*lld(\.exe){0,1}".*}} "[[OUT]]" {{".*asanrtl.bc" ".*hip.bc"}} -// FAIL: error: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer +// FAIL: AMDGPU address sanitizer runtime library (asanrtl) is not found. Please install ROCm device library which supports address sanitizer // XNACK-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa' // XNACK-DAG: warning: ignoring '-fsanitize=address' option as it is not currently supported for offload arch 'gfx900:xnack-'. Use it with an offload arch containing 'xnack+' instead -- 2.7.4