Add sycl_external attribute #140282
Add sycl_external attribute #140282
Conversation
There was a problem hiding this comment.
Thanks for working on this @schittir! I completed an initial pass of all of the code, but still need to look more closely at the documentation updates.
| def err_sycl_attribute_internal_decl | ||
| : Error<"%0 attribute cannot be applied to a %select{function|variable}1" | ||
| " without external linkage">; |
There was a problem hiding this comment.
The only other uses of this diagnostic in downstream Intel repositories are for Intel extensions, so I don't think we need a generalized diagnostic.
The SYCL 2020 specification doesn't support use of the SYCL_EXTERNAL macro with variables, so we don't need to support that (if we need such an extension, we might want to introduce a different attribute for it later).
The suggested change below includes renaming, removal of applicability to variables, rephrasing inspired from err_attribute_selectany_non_extern_data, and style changes to match other nearby declarations.
| def err_sycl_attribute_internal_decl | |
| : Error<"%0 attribute cannot be applied to a %select{function|variable}1" | |
| " without external linkage">; | |
| def err_sycl_external_invalid_linkage : Error< | |
| "'sycl_external' can only be applied to functions with external linkage">; |
We'll eventually need to add diagnostics for at least the following scenarios as indicated in the SYCL 2020 specification in section 5.10.1. I haven't checked whether diagnostics are already present in downstream Intel report. For scenarios that are straight forward to diagnose, we should add diagnostics now. For others, we need to ensure we file an issue or somehow track adding diagnostics later. Ideally, we would add tests for each of these now with appropriate FIXME annotations as needed.
- The
sycl_externalattribute must be present on the first declaration of a function and may optionally be present on subsequent declarations. - If the device target does not support the generic address space, then functions declared with
sycl_externalcannot have parameter types or a return type that involves a raw pointer. - Functions declared with
sycl_externalcannot callgroup::parallel_for_work_item(). - Functions declared with
sycl_externalcannot be called from aparallel_for_work_groupscope.
| __attribute__((sycl_external)) // expected-warning {{'sycl_external' attribute ignored}} | ||
| void baz() {} | ||
|
|
||
| #endif |
There was a problem hiding this comment.
There are a few different concerns being exercised by this test. We have a precedent in place for exercising appertainment, grammar, and relevance by different test files in the sycl-kernel-entry-point-attr-* tests. Can we follow that pattern (and associated naming conventions) here? I'm not so concerned about the testing being as extensive as it is in those other tests, but splitting these concerns will allow additional cases to be added later with less disruption.
Thank you for the initial pass review, Tom! |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions h,cpp -- clang/test/SemaSYCL/sycl-external-attribute.cpp clang/include/clang/Sema/SemaSYCL.h clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaSYCL.cpp clang/test/CodeGenSYCL/address-space-deduction.cpp clang/test/CodeGenSYCL/address-space-mangling.cpp clang/test/CodeGenSYCL/debug-info-kernel-variables.cpp clang/test/CodeGenSYCL/field-annotate-addr-space.cpp clang/test/CodeGenSYCL/functionptr-addrspace.cpp clang/test/CodeGenSYCL/kernel-caller-entry-point.cppView the diff from clang-format here.diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index e22adf0aa..cd154ef47 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12965,8 +12965,8 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
if (D->hasAttr<WeakRefAttr>())
return false;
- if (LangOpts.SYCLIsDevice &&
- !D->hasAttr<SYCLKernelEntryPointAttr>() && !D->hasAttr<SYCLExternalAttr>())
+ if (LangOpts.SYCLIsDevice && !D->hasAttr<SYCLKernelEntryPointAttr>() &&
+ !D->hasAttr<SYCLExternalAttr>())
return false;
// Aliases and used decls are required.
|
No description provided.