Skip to content

Conversation

@steffenlarsen
Copy link
Contributor

The submit method on queue has caused multiple divergences throughout its lifetime, when additional information was required for new functionality. However, that design means an exponential growth in functions every time a new optional argument is needed. To battle this, this patch adds a new pimpl class with the optional submission info, which will only be initialized if needed. This boils the submit function paths down to one with event and one without.

The submit method on queue has caused multiple divergences throughout
its lifetime, when additional information was required for new
functionality. However, that design means an exponential growth in
functions every time a new optional argument is needed. To battle this,
this patch adds a new pimpl class with the optional submission info,
which will only be initialized if needed. This boils the submit
function paths down to one with event and one without.

Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen marked this pull request as ready for review October 22, 2024 07:46
@steffenlarsen steffenlarsen requested a review from a team as a code owner October 22, 2024 07:46
@steffenlarsen
Copy link
Contributor Author

Friendly ping @intel/llvm-reviewers-runtime (@bso-intel)

1 similar comment
@steffenlarsen
Copy link
Contributor Author

Friendly ping @intel/llvm-reviewers-runtime (@bso-intel)

Comment on lines +2784 to +2785
const detail::code_location &CodeLoc,
bool IsTopCodeLoc);
Copy link
Contributor

@aelovikov-intel aelovikov-intel Oct 30, 2024

Choose a reason for hiding this comment

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

Why aren't these two part of SubmitInfo? Maybe even CGH itself should go there...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention was for SubmissionInfo to be "free" if none of the optional arguments were passed. I suppose it could be part of the SubmissionInfo class itself, rather than its impl. Is that what you were thinking?

Of course, if there are non-optional arguments added in the future, we either have to add them to the impl of SubmissionInfo anyway, break ABI or add yet another API.

Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
@steffenlarsen steffenlarsen merged commit 528d43a into intel:sycl Nov 15, 2024
13 checks passed
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.

3 participants