-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL] Add warning for attributes applied to non-kernel functions #15154
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
Changes from all commits
b4d5226
d129a4b
20c6f73
148c2c5
c0dc57a
9d1b33b
b05cade
89f7b6c
e6f059f
35361dc
8ab3a14
7792dc7
fcf9f9d
5747964
33870e5
b03fafa
3aebde5
e1f9800
1d1f1e6
a4448c5
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 |
---|---|---|
|
@@ -17,7 +17,8 @@ | |
|
||
// Produce a conflicting attribute warning when the args are different. | ||
[[sycl::work_group_size_hint(4, 1, 1)]] void f3(); // expected-note {{previous attribute is here}} | ||
[[sycl::work_group_size_hint(1, 1, 32)]] void f3() {} // expected-warning {{attribute 'work_group_size_hint' is already applied with different arguments}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The additional warnings in all these tests makes the tests a bit messy IMO. I would just remove (if we already test that functionality elsewhere) or rewrite the incorrect tests to not generate this warning. But this might just be me nit-picking. @premanandrao @smanna12 what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in this specific case, I need to remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand. My point was that these test cases are no longer really relevant in this context. For example in this case, if we already have other tests testing I am ok submitting this PR as-is and cleaning up the tests in a separate PR if the other reviewers @premanandrao and @smanna12 want these tests cleaned up as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like the tests cleaned up, but not necessarily as part of this PR. Are you okay with that @elizabethandrews? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I have same opinion. Agree with @elizabethandrews and @premanandrao There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Works for me. @DYNIO-INTEL please clean up the tests in a follow up PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works for me too. I will clean up the tests in a follow up PR. |
||
[[sycl::work_group_size_hint(1, 1, 32)]] void f3() {} // expected-warning {{attribute 'work_group_size_hint' is already applied with different arguments}} \ | ||
dyniols marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// expected-warning {{'work_group_size_hint' attribute can only be applied to a SYCL kernel function}} | ||
|
||
// 1 and 2 dim versions | ||
[[sycl::work_group_size_hint(2)]] void f4(); // ok | ||
|
@@ -70,10 +71,13 @@ void instantiate() { | |
f8<0>(); // expected-note {{in instantiation}} | ||
#endif | ||
|
||
// expected-warning@#f9prev {{'work_group_size_hint' attribute can only be applied to a SYCL kernel function}} | ||
f9<1, 1, 1>(); // OK, args are the same on the redecl. | ||
|
||
// expected-warning@#f9 {{attribute 'work_group_size_hint' is already applied with different arguments}} | ||
// expected-note@#f9prev {{previous attribute is here}} | ||
// expected-warning@#f9prev {{'work_group_size_hint' attribute can only be applied to a SYCL kernel function}} | ||
|
||
f9<1, 2, 3>(); // expected-note {{in instantiation}} | ||
dyniols marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
|
@@ -97,14 +101,14 @@ class Functor16x2x1 { | |
|
||
class Functor4x4x4 { | ||
public: | ||
[[sycl::work_group_size_hint(4, 4, 4)]] void operator()() const {}; | ||
[[sycl::work_group_size_hint(4, 4, 4)]] void operator()() const {}; // expected-warning {{'work_group_size_hint' attribute can only be applied to a SYCL kernel function}} | ||
elizabethandrews marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
// Checking whether propagation of the attribute happens or not, according to the SYCL version. | ||
#if defined(EXPECT_PROP) // if attribute is propagated, then we expect errors here | ||
void f8x8x8(){}; | ||
#else // otherwise no error | ||
[[sycl::work_group_size_hint(8, 8, 8)]] void f8x8x8(){}; | ||
[[sycl::work_group_size_hint(8, 8, 8)]] void f8x8x8(){}; // expected-warning {{'work_group_size_hint' attribute can only be applied to a SYCL kernel function}} | ||
#endif | ||
class FunctorNoProp { | ||
public: | ||
|
Uh oh!
There was an error while loading. Please reload this page.