-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL] Disallow passing kernel by value for sycl_kernel attribute #15702
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
base: sycl
Are you sure you want to change the base?
Changes from 1 commit
af03188
1c5c2a4
3d724b4
c1477b8
e5d1286
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1122,18 +1122,14 @@ getKernelInvocationKind(FunctionDecl *KernelCallerFunc) { | |
|
|
||
| // The SYCL kernel's 'object type' used for diagnostics and naming/mangling is | ||
| // the first parameter to a function template using the sycl_kernel | ||
| // attribute. In SYCL 1.2.1, this was passed by value, | ||
| // and in SYCL 2020, it is passed by reference. | ||
| // attribute. In SYCL 2020, this is passed by reference. | ||
| static QualType GetSYCLKernelObjectType(const FunctionDecl *KernelCaller) { | ||
| assert(KernelCaller->getNumParams() > 0 && "Insufficient kernel parameters"); | ||
| QualType KernelParamTy = KernelCaller->getParamDecl(0)->getType(); | ||
|
|
||
| // SYCL 2020 kernels are passed by reference. | ||
| if (KernelParamTy->isReferenceType()) | ||
| KernelParamTy = KernelParamTy->getPointeeType(); | ||
|
|
||
| // SYCL 1.2.1 | ||
| return KernelParamTy.getUnqualifiedType(); | ||
| assert(KernelParamTy->isReferenceType() && "Since SYCL 2020 kernels must be passed by reference."); | ||
| return KernelParamTy = KernelParamTy->getPointeeType(); | ||
| } | ||
|
|
||
| /// \return the target of given SYCL accessor type | ||
|
|
@@ -5038,7 +5034,7 @@ void SemaSYCL::CheckSYCLKernelCall(FunctionDecl *KernelFunc, | |
|
|
||
| // check that calling kernel conforms to spec | ||
| QualType KernelParamTy = KernelFunc->getParamDecl(0)->getType(); | ||
| if (not KernelParamTy->isReferenceType()) { | ||
| if (! KernelParamTy->isReferenceType()) { | ||
| // passing by value. emit warning if using SYCL 2020 or greater | ||
| if (SemaRef.LangOpts.getSYCLVersion() >= LangOptions::SYCL_2020) | ||
| Diag(KernelFunc->getLocation(), diag::warn_sycl_pass_by_value_deprecated); | ||
|
||
|
|
||
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 not a fan of the assert message. I think you can just say kernel must be passed by reference.
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've changed it back. It seems like this assert gets hit before we issue a proper diagnostic for this kind of misuse.