Skip to content

Reapply "[Driver][ROCm][OpenMP] Fix default ockl linking for OpenMP."… #126671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 12, 2025
Merged

Reapply "[Driver][ROCm][OpenMP] Fix default ockl linking for OpenMP."… #126671

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8367c38
Reapply "[Driver][ROCm][OpenMP] Fix default ockl linking for OpenMP."
ampandey-1995 Feb 11, 2025
d0b644e
Address Comments of @arsenm.
ampandey-1995 Feb 11, 2025
da6fbed
Address @jhuber6 comments.
ampandey-1995 Feb 11, 2025
f738420
[NFC] Remove std::tuple for GPUSan.
ampandey-1995 Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions clang/lib/Driver/ToolChains/AMDGPU.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,25 +1014,27 @@ RocmInstallationDetector::getCommonBitcodeLibs(
bool isOpenMP = false) const {
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12> BCLibs;

auto GPUSanEnabled = [GPUSan]() { return std::get<bool>(GPUSan); };
// GPU Sanitizer currently only supports ASan and is enabled through host
// ASan.
bool GPUSanEnabled = std::get<bool>(GPUSan) &&
std::get<const SanitizerArgs>(GPUSan).needsAsanRt();

auto AddBCLib = [&](ToolChain::BitCodeLibraryInfo BCLib,
bool Internalize = true) {
BCLib.ShouldInternalize = Internalize;
BCLibs.emplace_back(BCLib);
};
auto AddSanBCLibs = [&]() {
if (GPUSanEnabled()) {
auto SanArgs = std::get<const SanitizerArgs>(GPUSan);
if (SanArgs.needsAsanRt())
AddBCLib(getAsanRTLPath(), false);
if (GPUSanEnabled) {
AddBCLib(getAsanRTLPath(), false);
}
};

AddSanBCLibs();
AddBCLib(getOCMLPath());
if (!isOpenMP)
AddBCLib(getOCKLPath());
else if (GPUSanEnabled() && isOpenMP)
else if (GPUSanEnabled && isOpenMP)
AddBCLib(getOCKLPath(), false);
AddBCLib(getDenormalsAreZeroPath(DAZ));
AddBCLib(getUnsafeMathPath(UnsafeMathOpt || FastRelaxedMath));
Expand Down Expand Up @@ -1066,7 +1068,7 @@ ROCMToolChain::getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs,
// them all?
std::tuple<bool, const SanitizerArgs> GPUSan(
DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, false),
options::OPT_fno_gpu_sanitize, true),
getSanitizerArgs(DriverArgs));
bool DAZ = DriverArgs.hasFlag(options::OPT_fgpu_flush_denormals_to_zero,
options::OPT_fno_gpu_flush_denormals_to_zero,
Expand Down Expand Up @@ -1099,7 +1101,7 @@ bool AMDGPUToolChain::shouldSkipSanitizeOption(
return false;

if (!DriverArgs.hasFlag(options::OPT_fgpu_sanitize,
options::OPT_fno_gpu_sanitize, false))
options::OPT_fno_gpu_sanitize, true))
return true;

auto &Diags = TC.getDriver().getDiags();
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs(
Action::OffloadKind DeviceOffloadKind) const {
DerivedArgList *DAL =
HostTC.TranslateArgs(Args, BoundArch, DeviceOffloadKind);
if (!DAL)
if (!DAL || Args.hasArg(options::OPT_fsanitize_EQ))
Copy link
Contributor

@jhuber6 jhuber6 Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this leak memory? If DAL is not null we replace it with a new one.

DAL = new DerivedArgList(Args.getBaseArgs());

const OptTable &Opts = getDriver().getOpts();
Expand Down
57 changes: 28 additions & 29 deletions clang/test/Driver/amdgpu-openmp-sanitize-options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,65 +1,64 @@
// REQUIRES: x86-registered-target, amdgpu-registered-target

// Fail on invalid ROCm Path.
// RUN: not %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fgpu-sanitize -nogpuinc --rocm-path=%S/Inputs/rocm-invalid %s 2>&1 \
// RUN: not %clang -no-canonical-prefixes -### -mcode-object-version=5 --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fgpu-sanitize -nogpuinc --rocm-path=%S/Inputs/rocm-invalid %s 2>&1 \
// RUN: | FileCheck --check-prefix=FAIL %s

// Enable multiple sanitizer's apart from ASan with invalid rocm-path.
// RUN: not %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fsanitize=leak -fgpu-sanitize --rocm-path=%S/Inputs/rocm-invalid -nogpuinc %s 2>&1 \
// RUN: not %clang -no-canonical-prefixes -### -mcode-object-version=5 --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fsanitize=leak -fgpu-sanitize --rocm-path=%S/Inputs/rocm-invalid -nogpuinc %s 2>&1 \
// RUN: | FileCheck --check-prefixes=NOTSUPPORTED,FAIL %s

// Memory, Leak, UndefinedBehaviour and Thread Sanitizer are not supported on AMDGPU.
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fsanitize=leak -fgpu-sanitize --rocm-path=%S/Inputs/rocm -nogpuinc %s 2>&1 \
// RUN: | FileCheck --check-prefix=NOTSUPPORTED %s

// GPU ASan Enabled Test Cases
// ASan enabled for amdgpu-arch [gfx908]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908 -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOXNACK,GPUSAN %s

// GPU ASan enabled for amdgpu-arch [gfx908:xnack-]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack- -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=XNACKNEG,GPUSAN %s

// GPU ASan enabled for amdgpu-arch [gfx908:xnack+]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=GPUSAN %s
// RUN: | FileCheck -check-prefixes=HOSTSAN,GPUSAN,SAN %s

// GPU ASan enabled through '-fsanitize=address' flag without '-fgpu-sanitize' for amdgpu-arch [gfx908:xnack+]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=HOSTSAN,GPUSAN,SAN %s

// ASan enabled for multiple amdgpu-arch [gfx908:xnack+,gfx900:xnack+]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ --offload-arch=gfx900:xnack+ -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=GPUSAN %s
// RUN: | FileCheck -check-prefixes=HOSTSAN,GPUSAN,SAN %s

// GPU ASan Disabled Test Cases
// ASan disabled for amdgpu-arch [gfx908]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908 -fsanitize=address -fno-gpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOGPUSAN %s

// GPU ASan disabled through '-fsanitize=address' without '-fgpu-sanitize' flag for amdgpu-arch [gfx908]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908 -fsanitize=address --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOXNACK,HOSTSAN,NOGPUSAN,SAN %s

// GPU ASan disabled for amdgpu-arch [gfx908]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908 -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOXNACK,HOSTSAN,NOGPUSAN,SAN %s

// GPU ASan disabled for amdgpu-arch [gfx908:xnack-]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack- -fsanitize=address -fno-gpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOGPUSAN %s
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack- -fsanitize=address -fgpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=XNACKNEG,HOSTSAN,NOGPUSAN,SAN %s

// GPU ASan disabled for amdgpu-arch [gfx908:xnack+]
// GPU ASan disabled using '-fno-gpu-sanitize' for amdgpu-arch [gfx908:xnack+]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ -fsanitize=address -fno-gpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOGPUSAN %s
// RUN: | FileCheck -check-prefixes=HOSTSAN,NOGPUSAN,SAN %s

// ASan disabled for amdgpu-arch [gfx908:xnack+,gfx900:xnack+]
// GPU ASan disabled for multiple amdgpu-arch [gfx908:xnack+,gfx900:xnack+]
// RUN: %clang -no-canonical-prefixes -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp --offload-arch=gfx908:xnack+ --offload-arch=gfx900:xnack+ -fsanitize=address -fno-gpu-sanitize --rocm-path=%S/Inputs/rocm %s 2>&1 \
// RUN: | FileCheck -check-prefixes=NOGPUSAN %s
// RUN: | FileCheck -check-prefixes=HOSTSAN,NOGPUSAN,SAN %s

// FAIL-DAG: 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
// NOTSUPPORTED-DAG: warning: ignoring '-fsanitize=leak' option as it is not currently supported for target 'amdgcn-amd-amdhsa'

// NOXNACK: warning: ignoring '-fsanitize=address' option for offload arch 'gfx908' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead
// XNACKNEG: warning: ignoring '-fsanitize=address' option for offload arch 'gfx908:xnack-' as it is not currently supported there. Use it with an offload arch containing 'xnack+' instead

// GPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "c".*}}
// GPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu".* "-emit-llvm-bc".* "-target-cpu" "(gfx908|gfx900)".* "-fopenmp".* "-fsanitize=address".* "-x" "c".*}}
// GPUSAN: {{"[^"]*clang-offload-packager[^"]*" "-o".* "--image=file=.*.bc,triple=amdgcn-amd-amdhsa,arch=gfx908(:xnack\-|:xnack\+)?,kind=openmp(,feature=(\-xnack|\+xnack))?"}}
// GPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "ir".*}}
// GPUSAN: {{"[^"]*clang-linker-wrapper[^"]*".* "--host-triple=x86_64-unknown-linux-gnu".* "--linker-path=[^"]*".* "--whole-archive" "[^"]*(libclang_rt.asan_static.a|libclang_rt.asan_static-x86_64.a)".* "--whole-archive" "[^"]*(libclang_rt.asan.a|libclang_rt.asan-x86_64.a)".*}}
// HOSTSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "c".*}}

// NOGPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "c".*}}
// GPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu".* "-emit-llvm-bc".* "-mlink-bitcode-file" "[^"]*asanrtl.bc".* "-mlink-bitcode-file" "[^"]*ockl.bc".* "-target-cpu" "(gfx908|gfx900)".* "-fopenmp".* "-fsanitize=address".* "-x" "c".*}}
// NOGPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu".* "-emit-llvm-bc".* "-target-cpu" "(gfx908|gfx900)".* "-fopenmp".* "-x" "c".*}}
// NOGPUSAN: {{"[^"]*clang-offload-packager[^"]*" "-o".* "--image=file=.*.bc,triple=amdgcn-amd-amdhsa,arch=gfx908(:xnack\-|:xnack\+)?,kind=openmp(,feature=(\-xnack|\+xnack))?"}}
// NOGPUSAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "ir".*}}
// NOGPUSAN: {{"[^"]*clang-linker-wrapper[^"]*".* "--host-triple=x86_64-unknown-linux-gnu".* "--linker-path=[^"]*".* "--whole-archive" "[^"]*(libclang_rt.asan_static.a|libclang_rt.asan_static-x86_64.a)".* "--whole-archive" "[^"]*(libclang_rt.asan.a|libclang_rt.asan-x86_64.a)".*}}

// SAN: {{"[^"]*clang-offload-packager[^"]*" "-o".* "--image=file=.*.bc,triple=amdgcn-amd-amdhsa,arch=gfx908(:xnack\-|:xnack\+)?,kind=openmp(,feature=(\-xnack|\+xnack))?"}}
// SAN: {{"[^"]*clang[^"]*" "-cc1" "-triple" "x86_64-unknown-linux-gnu".* "-fopenmp".* "-fsanitize=address".* "-fopenmp-targets=amdgcn-amd-amdhsa".* "-x" "ir".*}}
// SAN: {{"[^"]*clang-linker-wrapper[^"]*".* "--host-triple=x86_64-unknown-linux-gnu".* "--linker-path=[^"]*".* "--whole-archive" "[^"]*(libclang_rt.asan_static.a|libclang_rt.asan_static-x86_64.a)".* "--whole-archive" "[^"]*(libclang_rt.asan.a|libclang_rt.asan-x86_64.a)".*}}
2 changes: 1 addition & 1 deletion clang/test/Driver/hip-sanitize-options.hip
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang -### --target=x86_64-unknown-linux-gnu --offload-arch=gfx900:xnack+ \
// RUN: -fsanitize=address -fgpu-sanitize \
// RUN: -fsanitize=address \
// RUN: -nogpuinc --rocm-path=%S/Inputs/rocm \
// RUN: %s 2>&1 | FileCheck -check-prefixes=NORDC %s

Expand Down