Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[SYCL] Implement max_num_work_groups from the launch queries extension #14333
[SYCL] Implement max_num_work_groups from the launch queries extension #14333
Changes from 42 commits
aead3e3
e172c1e
a840be1
a9e17b4
4f81d0a
b2756c9
28b09f4
fd51cfb
377cf3b
3a8f3bf
b5b3d43
594727e
efcf44f
448b191
54c1b57
eb60b1c
b4b355e
20aa7c5
8fa7b09
e5910fa
a7411c8
7de06c1
e50e837
dc2dde4
b7807f9
4344064
0ef4baa
2fa280b
da8cde2
27b2416
b51f965
ef4cd8b
6483da7
4fc7353
7636f78
ea1e525
1698bb8
529caa5
2e190a2
762c7e1
2169579
c2788f6
6c5485f
cb5e47e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#7598 has been merged a while ago, this comment should be removed. I also suggest that we deprecate
max_num_work_group_sync
info trait right away in favor ofmax_num_work_groups
. The former is not documented so we should aim to remove it as soon as possible to avoid wide adoption of it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am aware and was thinking the same. However, I saw it as doing two separate things in one PR that eventually ultimately lands as a single squashed commit and have opted to follow-up with a separate PR solely deprecating
max_num_work_group_sync
. Of course, I am not arguing against your suggestion and preferences. Having said that, let me know how you'd prefer it done. I am fine either way. Thanks!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with doing so in a separate PR to have it a separate commit in our history
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, #7598 has been already merged. Any APIs which do not correspond to root group or launch queries extension should be marked as deprecated together with introduction of a proper API that is documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the other response wrt this. I see you suggested the introduction of a replacement and deprecating the replaced one go hand-in-hand together. So would you prefer that to happen in separate PR in order for the changes to land in separate squashed commits or you want them in one and just expand on the description?