-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL] Disable dead arg elimination for free function kernels #19776
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 23 commits
34a11ca
edec2b4
c9680a2
b932fe6
6b6da56
ceaabfd
714839e
01b6936
dbc7f3e
3d3a618
20bf581
a95f22d
1060e14
90422fb
2cb0ca7
964e5af
6704b61
e6740fd
4f76afe
54b372b
17b7eda
dc92d56
8723efe
28569aa
01e13bc
06bc971
e83e510
5dccb9b
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 |
|---|---|---|
|
|
@@ -93,18 +93,6 @@ void F<float>(int X) { | |
| volatile float Y = static_cast<float>(X); | ||
| } | ||
|
|
||
| template <typename... Args> | ||
|
Contributor
Author
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. Variadic arguments are not allowed in free function kernels as per the spec here.
Contributor
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. parameter pack is different from variadic arguments
Contributor
Author
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 was referring to line 99. I put the note on line 96 since its the start of the deleted section.
Contributor
Author
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.
Just to err on the safe side, I'm tagging @gmlueck. In the section of the spec that I have linked in my first comment in this thread, is the term "variadic argument" referring to the construct used in line 99 of this test that I've deleted or is it referring to some other construct?
Contributor
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 agree with @aelovikov-intel. The code on line 96 is not a function with variadic arguments. It is a function template with a parameter pack. When the template is instantiated, there is a fixed number of template arguments (defined by this particular instantiation), and therefore the instantiated function has a fixed number of arguments. Such a function is expected to be a legal free function kernel. If it does not work, we should investigate why.
Contributor
Author
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. Ok, I see. There is already a tracker for this issue. In the meantime, to get this merged, I'm commenting out only this test case and adding a TODO to uncomment it once the tracker is solved. It's of course less than ideal to reduce test coverage but given that its important to have this change for free function kernels and the fact that the bug is exposed by this PR rather than caused by it, I think it's an acceptable compromise in light of the approaching deadline. Also tagging @AlexeySachkov for awareness. |
||
| SYCL_EXT_ONEAPI_FUNCTION_PROPERTY( | ||
| (ext::oneapi::experimental::single_task_kernel)) | ||
| void variadic_templated(Args... args) {} | ||
|
|
||
| template <> | ||
| SYCL_EXT_ONEAPI_FUNCTION_PROPERTY( | ||
| (ext::oneapi::experimental::single_task_kernel)) | ||
| void variadic_templated<double>(double b) { | ||
| b = 20.0f; | ||
| } | ||
|
|
||
| template <auto *Func, typename T> void test_func() { | ||
| queue Q; | ||
| kernel_bundle bundle = | ||
|
|
@@ -173,7 +161,6 @@ int main() { | |
| test_func_custom_type<A::B::C::TestClass>(); | ||
| test_func<F<float>, float>(); | ||
| test_func<F<uint32_t>, uint32_t>(); | ||
| test_func<variadic_templated<double>, double>(); | ||
| test_func<sum1<3, float>, float>(); | ||
| test_accessor(); | ||
| test_shared(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.