https://github.com/ampandey-1995 updated https://github.com/llvm/llvm-project/pull/123922
>From 55582eeada535b101ec43dadbad3922021951343 Mon Sep 17 00:00:00 2001 From: Amit Pandey <pandey.kumaramit2...@gmail.com> Date: Wed, 22 Jan 2025 15:41:41 +0530 Subject: [PATCH] [Driver][ASan] Refactor Clang-Driver "Sanitizer Bitcode" linking. ASan bitcode linking is currently available for HIPAMD,OpenMP and OpenCL. Moving sanitizer specific common parts of logic to appropriate API's so as to reduce code redundancy and maintainability. --- clang/lib/Driver/ToolChains/AMDGPU.cpp | 57 ++++++++++++++------ clang/lib/Driver/ToolChains/AMDGPU.h | 2 +- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp | 7 +-- clang/lib/Driver/ToolChains/HIPAMD.cpp | 25 ++------- clang/lib/Driver/ToolChains/ROCm.h | 12 ++--- clang/test/Driver/hip-sanitize-options.hip | 8 +-- clang/test/Driver/rocm-device-libs.cl | 2 +- 7 files changed, 57 insertions(+), 56 deletions(-) diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index a8061ffd9321f5..4fb77ecedaf4a5 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -946,6 +946,10 @@ void ROCMToolChain::addClangTargetOptions( StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(CanonArch); auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion( getAMDGPUCodeObjectVersion(getDriver(), DriverArgs)); + std::tuple<bool, const SanitizerArgs> GPUSan( + DriverArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true), + getSanitizerArgs(DriverArgs)); if (!RocmInstallation->checkCommonBitcodeLibs(CanonArch, LibDeviceFile, ABIVer)) return; @@ -965,21 +969,19 @@ void ROCMToolChain::addClangTargetOptions( DriverArgs.hasArg(options::OPT_cl_fp32_correctly_rounded_divide_sqrt); // Add the OpenCL specific bitcode library. - llvm::SmallVector<std::string, 12> BCLibs; - BCLibs.push_back(RocmInstallation->getOpenCLPath().str()); + llvm::SmallVector<BitCodeLibraryInfo, 12> BCLibs; + BCLibs.emplace_back(RocmInstallation->getOpenCLPath().str()); // Add the generic set of libraries. BCLibs.append(RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt, - FastRelaxedMath, CorrectSqrt, ABIVer, false)); + FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, false)); - if (getSanitizerArgs(DriverArgs).needsAsanRt()) { - CC1Args.push_back("-mlink-bitcode-file"); - CC1Args.push_back( - DriverArgs.MakeArgString(RocmInstallation->getAsanRTLPath())); - } - for (StringRef BCFile : BCLibs) { - CC1Args.push_back("-mlink-builtin-bitcode"); + for (auto [BCFile, Internalize] : BCLibs) { + if (Internalize) + CC1Args.push_back("-mlink-builtin-bitcode"); + else + CC1Args.push_back("-mlink-bitcode-file"); CC1Args.push_back(DriverArgs.MakeArgString(BCFile)); } } @@ -1002,18 +1004,35 @@ bool RocmInstallationDetector::checkCommonBitcodeLibs( return true; } -llvm::SmallVector<std::string, 12> +llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> RocmInstallationDetector::getCommonBitcodeLibs( const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, bool Wave64, bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, bool FastRelaxedMath, - bool CorrectSqrt, DeviceLibABIVersion ABIVer, bool isOpenMP = false) const { - llvm::SmallVector<std::string, 12> BCLibs; - - auto AddBCLib = [&](StringRef BCFile) { BCLibs.push_back(BCFile.str()); }; + bool CorrectSqrt, DeviceLibABIVersion ABIVer, + const std::tuple<bool, const SanitizerArgs> &GPUSan, + bool isOpenMP = false) const { + llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs; + + auto GPUSanEnabled = [GPUSan]() { return std::get<bool>(GPUSan); }; + auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib, + bool Internalize = true) { + BCLib.ShouldInternalize = Internalize; + BCLibs.emplace_back(BCLib); + }; + auto AddSanBCLibs = [&]() { + auto SanArgs = std::get<const SanitizerArgs>(GPUSan); + if (GPUSanEnabled()) { + if (SanArgs.needsAsanRt()) + AddBCLib(getAsanRTLPath(), false); + } + }; + AddSanBCLibs(); AddBCLib(getOCMLPath()); if (!isOpenMP) AddBCLib(getOCKLPath()); + else if (GPUSanEnabled() && isOpenMP) + AddBCLib(getOCKLPath(), false); AddBCLib(getDenormalsAreZeroPath(DAZ)); AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath)); AddBCLib(getFiniteOnlyPath(FiniteOnly || FastRelaxedMath)); @@ -1027,7 +1046,7 @@ RocmInstallationDetector::getCommonBitcodeLibs( return BCLibs; } -llvm::SmallVector<std::string, 12> +llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, const std::string &GPUArch, bool isOpenMP) const { @@ -1037,6 +1056,10 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile = RocmInstallation->getLibDeviceFile(CanonArch); auto ABIVer = DeviceLibABIVersion::fromCodeObjectVersion( getAMDGPUCodeObjectVersion(getDriver(), DriverArgs)); + std::tuple<bool, const SanitizerArgs> GPUSan( + DriverArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true), + getSanitizerArgs(DriverArgs)); if (!RocmInstallation->checkCommonBitcodeLibs(CanonArch, LibDeviceFile, ABIVer)) return {}; @@ -1061,7 +1084,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, return RocmInstallation->getCommonBitcodeLibs( DriverArgs, LibDeviceFile, Wave64, DAZ, FiniteOnly, UnsafeMathOpt, - FastRelaxedMath, CorrectSqrt, ABIVer, isOpenMP); + FastRelaxedMath, CorrectSqrt, ABIVer, GPUSan, isOpenMP); } bool AMDGPUToolChain::shouldSkipSanitizeOption( diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index a9b4552a1f91a4..aad6bc75dffafc 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -142,7 +142,7 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { Action::OffloadKind DeviceOffloadKind) const override; // Returns a list of device library names shared by different languages - llvm::SmallVector<std::string, 12> + llvm::SmallVector<BitCodeLibraryInfo, 12> getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, const std::string &GPUArch, bool isOpenMP = false) const; diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp index 3f0b3f2d86b3ed..2b8917106afc14 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp @@ -9,7 +9,7 @@ #include "AMDGPUOpenMP.h" #include "AMDGPU.h" #include "CommonArgs.h" -#include "ToolChains/ROCm.h" +#include "ROCm.h" #include "clang/Basic/DiagnosticDriver.h" #include "clang/Driver/Compilation.h" #include "clang/Driver/Driver.h" @@ -159,11 +159,6 @@ AMDGPUOpenMPToolChain::getDeviceLibs(const llvm::opt::ArgList &Args) const { if (Args.hasArg(options::OPT_nogpulib)) return {}; - if (!RocmInstallation->hasDeviceLibrary()) { - getDriver().Diag(diag::err_drv_no_rocm_device_lib) << 0; - return {}; - } - StringRef GpuArch = getProcessorFromTargetID( getTriple(), Args.getLastArgValue(options::OPT_march_EQ)); diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp index ccee065b590647..158a2520759846 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp @@ -382,7 +382,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { llvm::sys::path::append(Path, BCName); FullName = Path; if (llvm::sys::fs::exists(FullName)) { - BCLibs.push_back(FullName); + BCLibs.emplace_back(FullName); return; } } @@ -396,28 +396,11 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { StringRef GpuArch = getGPUArch(DriverArgs); assert(!GpuArch.empty() && "Must have an explicit GPU arch."); - // If --hip-device-lib is not set, add the default bitcode libraries. - if (DriverArgs.hasFlag(options::OPT_fgpu_sanitize, - options::OPT_fno_gpu_sanitize, true) && - getSanitizerArgs(DriverArgs).needsAsanRt()) { - auto AsanRTL = RocmInstallation->getAsanRTLPath(); - if (AsanRTL.empty()) { - 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.emplace_back(AsanRTL, /*ShouldInternalize=*/false); - } - // Add the HIP specific bitcode library. - BCLibs.push_back(RocmInstallation->getHIPPath()); + BCLibs.emplace_back(RocmInstallation->getHIPPath()); // Add common device libraries like ocml etc. - for (StringRef N : getCommonDeviceLibNames(DriverArgs, GpuArch.str())) + for (auto N : getCommonDeviceLibNames(DriverArgs, GpuArch.str())) BCLibs.emplace_back(N); // Add instrument lib. @@ -426,7 +409,7 @@ HIPAMDToolChain::getDeviceLibs(const llvm::opt::ArgList &DriverArgs) const { if (InstLib.empty()) return BCLibs; if (llvm::sys::fs::exists(InstLib)) - BCLibs.push_back(InstLib); + BCLibs.emplace_back(InstLib); else getDriver().Diag(diag::err_drv_no_such_file) << InstLib; } diff --git a/clang/lib/Driver/ToolChains/ROCm.h b/clang/lib/Driver/ToolChains/ROCm.h index dceb0ab0366933..681c242b0678e2 100644 --- a/clang/lib/Driver/ToolChains/ROCm.h +++ b/clang/lib/Driver/ToolChains/ROCm.h @@ -13,6 +13,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Driver/Driver.h" #include "clang/Driver/Options.h" +#include "clang/Driver/SanitizerArgs.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringMap.h" #include "llvm/Option/ArgList.h" @@ -173,12 +174,11 @@ class RocmInstallationDetector { /// Get file paths of default bitcode libraries common to AMDGPU based /// toolchains. - llvm::SmallVector<std::string, 12> - getCommonBitcodeLibs(const llvm::opt::ArgList &DriverArgs, - StringRef LibDeviceFile, bool Wave64, bool DAZ, - bool FiniteOnly, bool UnsafeMathOpt, - bool FastRelaxedMath, bool CorrectSqrt, - DeviceLibABIVersion ABIVer, bool isOpenMP) const; + llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> getCommonBitcodeLibs( + const llvm::opt::ArgList &DriverArgs, StringRef LibDeviceFile, + bool Wave64, bool DAZ, bool FiniteOnly, bool UnsafeMathOpt, + bool FastRelaxedMath, bool CorrectSqrt, DeviceLibABIVersion ABIVer, + const std::tuple<bool, const SanitizerArgs> &GPUSan, bool isOpenMP) const; /// Check file paths of default bitcode libraries common to AMDGPU based /// toolchains. \returns false if there are invalid or missing files. bool checkCommonBitcodeLibs(StringRef GPUArch, StringRef LibDeviceFile, diff --git a/clang/test/Driver/hip-sanitize-options.hip b/clang/test/Driver/hip-sanitize-options.hip index d94cbdacdaeb3a..8de0ee9e18426b 100644 --- a/clang/test/Driver/hip-sanitize-options.hip +++ b/clang/test/Driver/hip-sanitize-options.hip @@ -18,7 +18,7 @@ // RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \ // RUN: %s 2>&1 | FileCheck -check-prefixes=RDC %s -// RUN: not %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \ +// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -mcode-object-version=5 --offload-arch=gfx900:xnack+ \ // RUN: -fsanitize=address -fgpu-sanitize \ // RUN: -nogpuinc --rocm-path=%S/Inputs/rocm-invalid \ // RUN: %s 2>&1 | FileCheck -check-prefixes=FAIL %s @@ -52,15 +52,15 @@ // CHECK-NOT: {{"[^"]*lld(\.exe){0,1}".* ".*hip.bc"}} // CHECK: {{"[^"]*clang[^"]*".* "-triple" "x86_64-unknown-linux-gnu".* "-fsanitize=address"}} -// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]" +// NORDC: {{"[^"]*clang[^"]*".* "-emit-obj".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.o]]" // NORDC-NOT: {{"[^"]*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".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-mlink-builtin-bitcode" ".*hip.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]" +// RDC: {{"[^"]*clang[^"]*".* "-emit-llvm-bc".* "-fcuda-is-device".* "-mlink-builtin-bitcode" ".*hip.bc".* "-mlink-bitcode-file" ".*asanrtl.bc".* "-fsanitize=address".*}} "-o" "[[OUT:[^"]*.bc]]" // RDC-NOT: {{"[^"]*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 +// FAIL: error: cannot find ROCm device library for ABI version 5; provide its path via '--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build without ROCm device library // 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 for offload arch 'gfx900:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead diff --git a/clang/test/Driver/rocm-device-libs.cl b/clang/test/Driver/rocm-device-libs.cl index 6837e219dc35de..f9766e6fa4d996 100644 --- a/clang/test/Driver/rocm-device-libs.cl +++ b/clang/test/Driver/rocm-device-libs.cl @@ -145,8 +145,8 @@ // RUN: 2>&1 | FileCheck --check-prefixes=NOASAN %s // COMMON: "-triple" "amdgcn-amd-amdhsa" -// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/opencl.bc" +// ASAN-SAME: "-mlink-bitcode-file" "{{.*}}/amdgcn/bitcode/asanrtl.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ocml.bc" // COMMON-SAME: "-mlink-builtin-bitcode" "{{.*}}/amdgcn/bitcode/ockl.bc" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits