-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang] Fix device_kernel attribute crash on unsupported targets when not using AMDGPU spelling #161687
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.
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
Conversation
… not using AMDGPU spelling
@llvm/pr-subscribers-clang Author: None (camc) ChangesResolves #161077 For non-AMDGPU spellings, modify sema attr handling to ignore and diagnose the device_kernel attribute when not on a supported target. For the AMDGPU spelling, this is already performed via getCCForDeclaratorChunk, unfortunately not simple to unify these cases, (discussion). Full diff: https://github.com/llvm/llvm-project/pull/161687.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index a8dfa4d7df2d5..979ca03e9867e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5204,7 +5204,11 @@ static void handleCallConvAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
static void handleDeviceKernelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
const auto *FD = dyn_cast_or_null<FunctionDecl>(D);
bool IsFunctionTemplate = FD && FD->getDescribedFunctionTemplate();
- if (S.getLangOpts().SYCLIsDevice) {
+ if (!S.getLangOpts().OpenCL && !S.getLangOpts().CUDA && !DeviceKernelAttr::isAMDGPUSpelling(AL)) {
+ // This is already diagnosed for AMDGPU in getCCForDeclaratorChunk
+ S.Diag(AL.getLoc(), diag::warn_cconv_unsupported)
+ << AL << (int)Sema::CallingConventionIgnoredReason::ForThisTarget;
+ } else if (S.getLangOpts().SYCLIsDevice) {
if (!IsFunctionTemplate) {
S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str)
<< AL << AL.isRegularKeywordAttribute() << "function templates";
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index bee613aa5f1c5..aaa92b238376c 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3780,7 +3780,7 @@ static CallingConv getCCForDeclaratorChunk(
}
}
}
- if (!S.getLangOpts().isSYCL()) {
+ if (S.getLangOpts().OpenCL || S.getLangOpts().CUDA) {
for (const ParsedAttr &AL : D.getDeclSpec().getAttributes()) {
if (AL.getKind() == ParsedAttr::AT_DeviceKernel) {
CC = CC_DeviceKernel;
diff --git a/clang/test/Sema/callingconv.c b/clang/test/Sema/callingconv.c
index f0b8b80a32974..2ea8757526666 100644
--- a/clang/test/Sema/callingconv.c
+++ b/clang/test/Sema/callingconv.c
@@ -55,6 +55,10 @@ int __attribute__((aarch64_vector_pcs)) aavpcs(void); // expected-warning {{'aar
int __attribute__((aarch64_sve_pcs)) aasvepcs(void); // expected-warning {{'aarch64_sve_pcs' calling convention is not supported for this target}}
int __attribute__((amdgpu_kernel)) amdgpu_kernel(void); // expected-warning {{'amdgpu_kernel' calling convention is not supported for this target}}
+int __attribute__((device_kernel)) device_kernel(void) { // expected-warning {{'device_kernel' calling convention is not supported for this target}}
+}
+int __attribute__((sycl_kernel)) sycl_kernel(void) { // expected-warning {{'sycl_kernel' calling convention is not supported for this target}}
+}
// PR6361
void ctest3();
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
cc @sarnex @Fznamznon, GitHub wouldn't allow reopening the previous PR. |
const auto *FD = dyn_cast_or_null<FunctionDecl>(D); | ||
bool IsFunctionTemplate = FD && FD->getDescribedFunctionTemplate(); | ||
if (S.getLangOpts().SYCLIsDevice) { | ||
if (!S.getLangOpts().OpenCL && !S.getLangOpts().CUDA && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doing a test to see if we need the AMDGPU
special casing, will post my results in the other PR. probably we should wait for that before moving forward with this
int __attribute__((aarch64_sve_pcs)) aasvepcs(void); // expected-warning {{'aarch64_sve_pcs' calling convention is not supported for this target}} | ||
|
||
int __attribute__((amdgpu_kernel)) amdgpu_kernel(void); // expected-warning {{'amdgpu_kernel' calling convention is not supported for this target}} | ||
int __attribute__((device_kernel)) device_kernel(void) { // expected-warning {{'device_kernel' calling convention is not supported for this target}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi i added this test in locally so once i get to PR-level we'll have a lock down for the crash
Resolves #161077
For non-AMDGPU spellings, modify sema attr handling to ignore and diagnose the device_kernel attribute when not on a supported target. For the AMDGPU spelling, this is already performed via getCCForDeclaratorChunk, unfortunately not simple to unify these cases, (discussion).