[clang] [Clang] Fix sema checks thinking kernels aren't kernels (PR #104460)
Joseph Huber via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 15 11:51:46 PDT 2024
https://github.com/jhuber6 updated https://github.com/llvm/llvm-project/pull/104460
>From 84c89feaea8135f7dfaba488b442818974b51c9d Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Thu, 15 Aug 2024 10:34:17 -0500
Subject: [PATCH 1/3] [Clang] Fix sema checks thinking kernels aren't kernels
Summary:
Currently we have some sema checks to make sure users don't apply
kernel-only attributes to non-kernel functions. However, this currently
did not correctly check for bare NVPTX / AMDGPU kernel attributes,
making it impossible to use them at all w/o CUDA enabled. This patch
fixes that by checking for the calling convention / attributes directly.
---
clang/lib/Sema/SemaDeclAttr.cpp | 7 +++++--
clang/test/CodeGenCXX/amdgpu-kernel-arg-pointer-type.cpp | 5 +++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3b5e984f4ee773..96c3759de7edc1 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7147,7 +7147,9 @@ void Sema::ProcessDeclAttributeList(
// good to have a way to specify "these attributes must appear as a group",
// for these. Additionally, it would be good to have a way to specify "these
// attribute must never appear as a group" for attributes like cold and hot.
- if (!D->hasAttr<OpenCLKernelAttr>()) {
+ const FunctionType *FnTy = D->getFunctionType();
+ if (!D->hasAttr<OpenCLKernelAttr>() && FnTy &&
+ FnTy->getCallConv() != CallingConv::CC_AMDGPUKernelCall) {
// These attributes cannot be applied to a non-kernel function.
if (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
// FIXME: This emits a different error message than
@@ -7163,7 +7165,8 @@ void Sema::ProcessDeclAttributeList(
} else if (const auto *A = D->getAttr<OpenCLIntelReqdSubGroupSizeAttr>()) {
Diag(D->getLocation(), diag::err_opencl_kernel_attr) << A;
D->setInvalidDecl();
- } else if (!D->hasAttr<CUDAGlobalAttr>()) {
+ } else if (!D->hasAttr<CUDAGlobalAttr>() &&
+ !D->hasAttr<NVPTXKernelAttr>()) {
if (const auto *A = D->getAttr<AMDGPUFlatWorkGroupSizeAttr>()) {
Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
<< A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
diff --git a/clang/test/CodeGenCXX/amdgpu-kernel-arg-pointer-type.cpp b/clang/test/CodeGenCXX/amdgpu-kernel-arg-pointer-type.cpp
index c8fb12315d0eec..8df1d75e9ec8fa 100644
--- a/clang/test/CodeGenCXX/amdgpu-kernel-arg-pointer-type.cpp
+++ b/clang/test/CodeGenCXX/amdgpu-kernel-arg-pointer-type.cpp
@@ -6,6 +6,10 @@
// The original test passes the result through opt O2, but that seems to introduce invalid
// addrspace casts which are not being fixed as part of the present change.
+// COMMON: define{{.*}} amdgpu_kernel void @_Z6kernelv() #[[ATTR:[0-9]+]]
+__attribute__((amdgpu_kernel, amdgpu_flat_work_group_size(1, 256))) void
+ kernel() {}
+
// COMMON-LABEL: define{{.*}} amdgpu_kernel void @_Z7kernel1Pi(ptr {{.*}} %x)
// CHECK-NOT: ={{.*}} addrspacecast [[TYPE:.*]] addrspace(1)* %{{.*}} to ptr
__attribute__((amdgpu_kernel)) void kernel1(int *x) {
@@ -81,3 +85,4 @@ __attribute__((amdgpu_kernel)) void kernel8(struct SS a) {
*a.x += 3.f;
}
+// COMMON: attributes #[[ATTR]] = { {{.*}}"amdgpu-flat-work-group-size"="1,256"{{.*}} }
>From 25012dc87e9cb694b74f5f5c1e4105cca9954e20 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Thu, 15 Aug 2024 11:07:49 -0500
Subject: [PATCH 2/3] Address comments
---
clang/lib/Sema/SemaDeclAttr.cpp | 44 ++++++++++++++++-----------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 96c3759de7edc1..c10fd87488b2b4 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7147,9 +7147,7 @@ void Sema::ProcessDeclAttributeList(
// good to have a way to specify "these attributes must appear as a group",
// for these. Additionally, it would be good to have a way to specify "these
// attribute must never appear as a group" for attributes like cold and hot.
- const FunctionType *FnTy = D->getFunctionType();
- if (!D->hasAttr<OpenCLKernelAttr>() && FnTy &&
- FnTy->getCallConv() != CallingConv::CC_AMDGPUKernelCall) {
+ if (!D->hasAttr<OpenCLKernelAttr>()) {
// These attributes cannot be applied to a non-kernel function.
if (const auto *A = D->getAttr<ReqdWorkGroupSizeAttr>()) {
// FIXME: This emits a different error message than
@@ -7165,25 +7163,27 @@ void Sema::ProcessDeclAttributeList(
} else if (const auto *A = D->getAttr<OpenCLIntelReqdSubGroupSizeAttr>()) {
Diag(D->getLocation(), diag::err_opencl_kernel_attr) << A;
D->setInvalidDecl();
- } else if (!D->hasAttr<CUDAGlobalAttr>() &&
- !D->hasAttr<NVPTXKernelAttr>()) {
- if (const auto *A = D->getAttr<AMDGPUFlatWorkGroupSizeAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUWavesPerEUAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUNumSGPRAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUNumVGPRAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- }
+ }
+ }
+ const FunctionType *FnTy = D->getFunctionType();
+ if (!D->hasAttr<CUDAGlobalAttr>() && !D->hasAttr<OpenCLKernelAttr>() &&
+ FnTy && FnTy->getCallConv() != CallingConv::CC_AMDGPUKernelCall) {
+ if (const auto *A = D->getAttr<AMDGPUFlatWorkGroupSizeAttr>()) {
+ Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+ << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
+ D->setInvalidDecl();
+ } else if (const auto *A = D->getAttr<AMDGPUWavesPerEUAttr>()) {
+ Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+ << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
+ D->setInvalidDecl();
+ } else if (const auto *A = D->getAttr<AMDGPUNumSGPRAttr>()) {
+ Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+ << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
+ D->setInvalidDecl();
+ } else if (const auto *A = D->getAttr<AMDGPUNumVGPRAttr>()) {
+ Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+ << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
+ D->setInvalidDecl();
}
}
>From c774307fff2b8648b4376003f59f745251ebde03 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Thu, 15 Aug 2024 13:48:51 -0500
Subject: [PATCH 3/3] Address comments
---
clang/lib/Sema/SemaDeclAttr.cpp | 37 ++++++++++++++++-----------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c10fd87488b2b4..f1ff0a2f7f9c6e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -7123,6 +7123,13 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
}
}
+static bool isKernelDecl(Decl *D) {
+ const FunctionType *FnTy = D->getFunctionType();
+ return D->hasAttr<OpenCLKernelAttr>() ||
+ (FnTy && FnTy->getCallConv() == CallingConv::CC_AMDGPUKernelCall) ||
+ D->hasAttr<CUDAGlobalAttr>() || D->getAttr<NVPTXKernelAttr>();
+}
+
void Sema::ProcessDeclAttributeList(
Scope *S, Decl *D, const ParsedAttributesView &AttrList,
const ProcessDeclAttributeOptions &Options) {
@@ -7165,25 +7172,17 @@ void Sema::ProcessDeclAttributeList(
D->setInvalidDecl();
}
}
- const FunctionType *FnTy = D->getFunctionType();
- if (!D->hasAttr<CUDAGlobalAttr>() && !D->hasAttr<OpenCLKernelAttr>() &&
- FnTy && FnTy->getCallConv() != CallingConv::CC_AMDGPUKernelCall) {
- if (const auto *A = D->getAttr<AMDGPUFlatWorkGroupSizeAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUWavesPerEUAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUNumSGPRAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
- } else if (const auto *A = D->getAttr<AMDGPUNumVGPRAttr>()) {
- Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
- << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
- D->setInvalidDecl();
+ if (!isKernelDecl(D)) {
+ for (const Attr *A :
+ llvm::ArrayRef<Attr *>({D->getAttr<AMDGPUFlatWorkGroupSizeAttr>(),
+ D->getAttr<AMDGPUWavesPerEUAttr>(),
+ D->getAttr<AMDGPUNumSGPRAttr>(),
+ D->getAttr<AMDGPUNumVGPRAttr>()})) {
+ if (A) {
+ Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
+ << A << A->isRegularKeywordAttribute() << ExpectedKernelFunction;
+ D->setInvalidDecl();
+ }
}
}
More information about the cfe-commits
mailing list