Skip to content

[SYCL] Runtime property lists on kernel invocation functions #3416

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

Closed
wants to merge 1 commit into from

Conversation

againull
Copy link
Contributor

@againull againull commented Mar 25, 2021

Add an optional parameter to kernel invocation functions which accepts
property list. Such properties are going to affect the way the kernel
is invoked. Example is a property to set cache/SLM size for a kernel
invocation which is going to be implemented in upcoming PR.

Test: intel/llvm-test-suite#197
Draft PR for GPU Cache Config property: #3417

Add an optional parameter to kernel invocation functions which accepts
property list. Such properties are going to affect the way the kernel
is invoked. Example is a property to set cache/SLM size for a kernel
invocation which is going to be implemented in upcoming PR.
@againull againull requested a review from gmlueck March 25, 2021 19:49
@againull againull requested a review from a team as a code owner March 25, 2021 19:49
@againull againull requested a review from smaslov-intel March 25, 2021 19:49
@illuhad
Copy link

illuhad commented Mar 25, 2021

I would suggest to be careful with adding more overloads to the kernel invocation functions for this functionality. They are already exceedingly complicated because of reductions, and I expect they will become even more complicated in the future with the addition of a library-only capable replacement mechanism for kernel attributes.
Because of this, we have instead decided to attach a property_list to queue::submit() already some time ago in hipSYCL to provide similar functionality:
https://github.com/illuhad/hipSYCL/blob/develop/doc/extensions.md#hipsycl_ext_cg_property_-command-group-properties

This also retains the current convention that the kernel always comes as last argument of the invocation function, which I personally prefer for aesthetic reasons ;)

Comment on lines -189 to -190
// CHECK-NEXT: | [sizeof=560, dsize=560, align=8,
// CHECK-NEXT: | nvsize=560, nvalign=8]
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't breaking ABI avoidable?
Tagging @romanovvlad .

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, Artur, please avoid breaking ABI. Similar problem is being solved here: #3375

@s-kanaev s-kanaev requested a review from romanovvlad March 26, 2021 17:08
@againull
Copy link
Contributor Author

againull commented Apr 1, 2021

This PR is going to be redone.

@againull againull closed this Apr 1, 2021
@againull againull deleted the runtime_property_lists branch December 3, 2022 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants