https://github.com/yxsamliu updated
https://github.com/llvm/llvm-project/pull/140185
>From 1d8a6ceb3eb3f9e899efde6bf6e86d23dfdfc430 Mon Sep 17 00:00:00 2001
From: "Yaxun (Sam) Liu" <yaxun....@amd.com>
Date: Fri, 16 May 2025 00:04:12 -0400
Subject: [PATCH] [HIP] Add warning for -mwavefrontsize64 on gfx10+
architectures
Wavefront size 64 is not fully supported for HIP on gfx10+ architectures
and is not tested. Previously, HIP headers relied on predefined macros
to detect wavefront size and emit pragma errors for unsupported
configurations. However, due to a design change where Clang no longer
emits predefined macros for wavefront size (replacing them with builtin
functions), HIP headers can no longer detect and diagnose invalid
wavefront size usage.
This patch adds driver-level diagnostics to fill this gap by emitting
a warning when -mwavefrontsize64 is used with gfx10+ architectures in
HIP compilation:
"wavefront size 64 is not supported by HIP runtime on AMD GPU architecture
gfx10 and above"
The warning uses the "unsupported-wave64" group, allowing:
- HIP runtime to enforce errors via -Werror=unsupported-wave64 in CMake
- Users to suppress warnings with -Wno-unsupported-wave64 if needed
- Compilation to proceed while informing users of potential issues
This approach provides the necessary diagnostic capability while maintaining
flexibility for both HIP runtime policy enforcement and user control.
Existing HIP builds continue to work unchanged, and future ROCm releases
can adopt this warning-based approach when available.
---
.../clang/Basic/DiagnosticDriverKinds.td | 3 ++
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
clang/lib/Driver/ToolChains/AMDGPU.cpp | 33 ++++++++++++-------
clang/lib/Driver/ToolChains/AMDGPU.h | 9 +++--
clang/lib/Driver/ToolChains/HIPAMD.cpp | 3 ++
clang/test/Driver/hip-toolchain-features.hip | 6 ++++
6 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 34b6c0d7a8acd..c0d25ee359359 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -200,6 +200,9 @@ def err_drv_opt_unsupported_input_type : Error<
"'%0' invalid for input of type %1">;
def err_drv_amdgpu_ieee_without_no_honor_nans : Error<
"invalid argument '-mno-amdgpu-ieee' only allowed with relaxed NaN
handling">;
+def warn_drv_unsupported_wave64 : Warning<
+ "wavefront size 64 is not supported by HIP runtime on AMD GPU architecture "
+ "gfx10 and above">, InGroup<UnsupportedWave64>;
def err_drv_argument_not_allowed_with : Error<
"invalid argument '%0' not allowed with '%1'">;
def err_drv_cannot_open_randomize_layout_seed_file : Error<
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index ace8663b73a4a..a1929ccc0836e 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -148,6 +148,7 @@ def UnsupportedFPOpt :
DiagGroup<"unsupported-floating-point-opt">;
def UnsupportedCB : DiagGroup<"unsupported-cb">;
def UnsupportedGPOpt : DiagGroup<"unsupported-gpopt">;
def UnsupportedTargetOpt : DiagGroup<"unsupported-target-opt">;
+def UnsupportedWave64 : DiagGroup<"unsupported-wave64">;
def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">;
def NullConversion : DiagGroup<"null-conversion">;
def ImplicitConversionFloatingPointToBool :
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index b7564a0495da8..fb250dfc77750 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -606,6 +606,7 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver &D,
// Add target ID features to -target-feature options. No diagnostics should
// be emitted here since invalid target ID is diagnosed at other places.
StringRef TargetID;
+ StringRef GpuArch;
if (Args.hasArg(options::OPT_mcpu_EQ))
TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
else if (Args.hasArg(options::OPT_march_EQ))
@@ -614,7 +615,7 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver &D,
llvm::StringMap<bool> FeatureMap;
auto OptionalGpuArch = parseTargetID(Triple, TargetID, &FeatureMap);
if (OptionalGpuArch) {
- StringRef GpuArch = *OptionalGpuArch;
+ GpuArch = *OptionalGpuArch;
// Iterate through all possible target ID features for the given GPU.
// If it is mapped to true, add +feature.
// If it is mapped to false, add -feature.
@@ -628,9 +629,10 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver &D,
}
}
}
-
- if (Args.hasFlag(options::OPT_mwavefrontsize64,
- options::OPT_mno_wavefrontsize64, false))
+ if (toolchains::AMDGPUToolChain::isWave64(
+ D, Args, llvm::AMDGPU::parseArchAMDGCN(GpuArch),
+ /*DiagInvalid=*/false,
+ /*Explicit=*/true))
Features.push_back("+wavefrontsize64");
if (Args.hasFlag(options::OPT_mamdgpu_precise_memory_op,
@@ -767,15 +769,24 @@ llvm::DenormalMode
AMDGPUToolChain::getDefaultDenormalModeForType(
llvm::DenormalMode::getIEEE();
}
-bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList &DriverArgs,
- llvm::AMDGPU::GPUKind Kind) {
+bool AMDGPUToolChain::isWave64(const Driver &D,
+ const llvm::opt::ArgList &DriverArgs,
+ llvm::AMDGPU::GPUKind Kind, bool DiagInvalid,
+ bool Explicit) {
const unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(Kind);
bool HasWave32 = (ArchAttr & llvm::AMDGPU::FEATURE_WAVE32);
- return !HasWave32 || DriverArgs.hasFlag(
- options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
-}
+ bool EnableWave64 = DriverArgs.hasFlag(
+ options::OPT_mwavefrontsize64, options::OPT_mno_wavefrontsize64, false);
+
+ if (DiagInvalid && HasWave32 && EnableWave64)
+ D.Diag(diag::warn_drv_unsupported_wave64);
+ if (Explicit)
+ return EnableWave64;
+
+ return !HasWave32 || EnableWave64;
+}
/// ROCM Toolchain
ROCMToolChain::ROCMToolChain(const Driver &D, const llvm::Triple &Triple,
@@ -884,7 +895,7 @@ void ROCMToolChain::addClangTargetOptions(
ABIVer))
return;
- bool Wave64 = isWave64(DriverArgs, Kind);
+ bool Wave64 = isWave64(getDriver(), DriverArgs, Kind);
// TODO: There are way too many flags that change this. Do we need to check
// them all?
bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
@@ -1012,7 +1023,7 @@ ROCMToolChain::getCommonDeviceLibNames(const
llvm::opt::ArgList &DriverArgs,
bool CorrectSqrt = DriverArgs.hasFlag(
options::OPT_fhip_fp32_correctly_rounded_divide_sqrt,
options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt, true);
- bool Wave64 = isWave64(DriverArgs, Kind);
+ bool Wave64 = isWave64(getDriver(), DriverArgs, Kind);
// GPU Sanitizer currently only supports ASan and is enabled through host
// ASan.
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h
b/clang/lib/Driver/ToolChains/AMDGPU.h
index 08bd4fa556f78..d9d0fac71eb91 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -89,8 +89,13 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public
Generic_ELF {
const llvm::opt::ArgList &DriverArgs, const JobAction &JA,
const llvm::fltSemantics *FPType = nullptr) const override;
- static bool isWave64(const llvm::opt::ArgList &DriverArgs,
- llvm::AMDGPU::GPUKind Kind);
+ /// Return whether the GPU of kind \p Kind has wavefront size 64 by driver
+ /// arguments \p DriverArgs. When \p Explicit is true, only return true
+ /// if wavefront size is set to 64 by arguments explicitly. When
+ /// \p DiagInvalid is true, diagnose invalid -mwavefrontsize64 arguments.
+ static bool isWave64(const Driver &D, const llvm::opt::ArgList &DriverArgs,
+ llvm::AMDGPU::GPUKind Kind, bool DiagInvalid = false,
+ bool Explicit = false);
/// Needed for using lto.
bool HasNativeLLVMSupport() const override {
return true;
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp
b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 5fe0f85ef09af..dc02f176e5bd8 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -227,6 +227,9 @@ void HIPAMDToolChain::addClangTargetOptions(
assert(DeviceOffloadingKind == Action::OFK_HIP &&
"Only HIP offloading kinds are supported for GPUs.");
+ const StringRef GpuArch = getGPUArch(DriverArgs);
+ auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
+ (void)isWave64(getDriver(), DriverArgs, Kind, /*DiagInvalid=*/true);
CC1Args.append({"-fcuda-is-device", "-fno-threadsafe-statics"});
diff --git a/clang/test/Driver/hip-toolchain-features.hip
b/clang/test/Driver/hip-toolchain-features.hip
index dbc007ac1335b..adac722dac712 100644
--- a/clang/test/Driver/hip-toolchain-features.hip
+++ b/clang/test/Driver/hip-toolchain-features.hip
@@ -67,6 +67,12 @@
// DUP-NOT: "-target-feature" "{{.*}}wavefrontsize64"
// DUP: {{.*}}lld{{.*}} "-plugin-opt=-mattr=+cumode"
+// RUN: %clang -### --target=x86_64-linux-gnu -fgpu-rdc -nogpulib \
+// RUN: -nogpuinc --offload-arch=gfx1010 --no-offload-new-driver %s \
+// RUN: -mno-wavefrontsize64 -mwavefrontsize64 2>&1 \
+// RUN: | FileCheck %s -check-prefix=WAVE64-WARN
+// WAVE64-WARN: warning: wavefront size 64 is not supported by HIP runtime on
AMD GPU architecture gfx10 and above
+
// RUN: %clang -### --target=x86_64-linux-gnu -fgpu-rdc -nogpulib \
// RUN: -nogpuinc --offload-arch=gfx1010 --no-offload-new-driver %s \
// RUN: -mno-wavefrontsize64 -mwavefrontsize64 2>&1 \
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
