Skip to content

[WIP][SYCL] Update usage rules of SYCL_EXTERNAL #962

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[WIP][SYCL] Update usage rules of SYCL_EXTERNAL #962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/AttrDocs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2070,7 +2070,7 @@ This attribute can only be applied to functions and indicates that the
function must be treated as a device function and must be emitted even if it has
no direct uses from other SYCL device functions. However, it cannot be applied
to functions marked as 'static', functions declared within an anonymous
namespace or class member functions.
namespace or virtual/pure virtual class member functions.

It also means that function should be available externally and
cannot be optimized out due to reachability analysis or by any other
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10293,7 +10293,7 @@ def err_intel_attribute_argument_is_not_in_range: Error<
def err_sycl_attibute_cannot_be_applied_here
: Error<"%0 attribute cannot be applied to a "
"%select{static function or function in an anonymous namespace"
"|class member function}1">;
"|virtual or pure virtual class member function}1">;
def warn_sycl_attibute_function_raw_ptr
: Warning<"SYCL 1.2.1 specification does not allow %0 attribute applied "
"to a function with a raw pointer "
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10078,6 +10078,12 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
}
}

// Methods explcitly marked with 'sycl_device' attribute (via SYCL_EXTERNAL)
// must be emitted regardless of number of actual uses
if (LangOpts.SYCLIsDevice && isa<CXXMethodDecl>(D))
if (auto *A = D->getAttr<SYCLDeviceAttr>())
return !A->isImplicit();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it worth to check for explicit SYCLDeviceAttr. Also, it would be good to hear feedback about this if in general: is it the right place and proper if?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with the place. I believe that in a happy future we will outline device code in other way: do this in CodeGen like OpenMP. We already try to do this in llorg, and we do some changes in this function for this.
So, could you please make your changes to this function not conflicting with https://reviews.llvm.org/D71016#change-cNmcQq4FKHx8 ?
And, I'm not sure that checking for explicit SYCLDeviceAttr is needed.


GVALinkage Linkage = GetGVALinkageForFunction(FD);

// static, static inline, always_inline, and extern inline functions can
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4488,9 +4488,9 @@ static void handleSYCLDeviceAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
<< AL << 0 /* static function or anonymous namespace */;
return;
}
if (isa<CXXMethodDecl>(FD)) {
if (FD->isVirtualAsWritten() || FD->isPure()) {
S.Diag(AL.getLoc(), diag::err_sycl_attibute_cannot_be_applied_here)
<< AL << 1 /* class member function */;
<< AL << 1 /* virtual or pure virtual class member function */;
return;
}
if (FD->getReturnType()->isPointerType()) {
Expand All @@ -4515,9 +4515,9 @@ static void handleSYCLDeviceIndirectlyCallableAttr(Sema &S, Decl *D,
<< AL << 0 /* static function or anonymous namespace */;
return;
}
if (isa<CXXMethodDecl>(FD)) {
if (FD->isVirtualAsWritten() || FD->isPure()) {
S.Diag(AL.getLoc(), diag::err_sycl_attibute_cannot_be_applied_here)
<< AL << 1 /* class member function */;
<< AL << 1 /* virtual or pure virtual class member function */;
return;
}

Expand Down
9 changes: 9 additions & 0 deletions clang/test/CodeGenSYCL/sycl-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@

int bar(int b);

class A {
public:
// TODO: check for constructor/destructor as well
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more TODO: check how it works for class templates and class member templates

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more TODO: check how it works for class templates and class member templates

Not sure that it's so hard to defer it to another patch.


// CHECK-DAG: define linkonce_odr spir_func void @_ZN1A3fooEv
__attribute__((sycl_device))
void foo() {}
};

// CHECK-DAG: define spir_func i32 @_Z3fooii
__attribute__((sycl_device))
int foo(int a, int b) { return a + bar(b); }
Expand Down
12 changes: 10 additions & 2 deletions clang/test/SemaSYCL/device-indirectly-callable-attr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,21 @@ namespace {
}

class A {
[[intel::device_indirectly_callable]] // expected-error {{'device_indirectly_callable' attribute cannot be applied to a class member function}}
[[intel::device_indirectly_callable]]
A() {}

[[intel::device_indirectly_callable]] // expected-error {{'device_indirectly_callable' attribute cannot be applied to a class member function}}
[[intel::device_indirectly_callable]]
int func3() {}
};

class B {
[[intel::device_indirectly_callable]] // expected-error {{'device_indirectly_callable' attribute cannot be applied to a virtual or pure virtual class member function}}
virtual int foo() {}

[[intel::device_indirectly_callable]] // expected-error {{'device_indirectly_callable' attribute cannot be applied to a virtual or pure virtual class member function}}
virtual int bar() = 0;
};

void helper() {}

[[intel::device_indirectly_callable]]
Expand Down
12 changes: 10 additions & 2 deletions clang/test/SemaSYCL/sycl-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,21 @@ namespace {
}

class A {
__attribute__((sycl_device)) // expected-error {{'sycl_device' attribute cannot be applied to a class member function}}
__attribute__((sycl_device))
A() {}

__attribute__((sycl_device)) // expected-error {{'sycl_device' attribute cannot be applied to a class member function}}
__attribute__((sycl_device))
int func3() {}
};

class B {
__attribute__((sycl_device)) // expected-error {{'sycl_device' attribute cannot be applied to a virtual or pure virtual class member function}}
virtual int foo() {}

__attribute__((sycl_device)) // expected-error {{'sycl_device' attribute cannot be applied to a virtual or pure virtual class member function}}
virtual int bar() = 0;
};

#if defined(NOT_STRICT)
__attribute__((sycl_device))
int* func3() { return nullptr; }
Expand Down