Skip to content

Conversation

hvdijk
Copy link
Contributor

@hvdijk hvdijk commented Jul 22, 2025

We were creating excessive numbers of threads. When we know we want a given amount of threads, just divide the number of workgroups by the number of threads and have each thread process that many workgroups.

This implementation also means we no longer need to resize workgroups, which was not generally safe.

We were creating excessive numbers of threads. When we know we want a
given amount of threads, just divide the number of workgroups by the
number of threads and have each thread process that many workgroups.
@hvdijk hvdijk requested a review from a team as a code owner July 22, 2025 14:04
@hvdijk hvdijk temporarily deployed to WindowsCILock July 22, 2025 14:04 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 22, 2025 14:25 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 22, 2025 14:25 — with GitHub Actions Inactive
Testing on Pointnet showed that the nesting we originally had is faster.
Copy link
Contributor

@uwedolinsky uwedolinsky left a comment

Choose a reason for hiding this comment

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

It seems this PR also removes the unsafe enqueuing optimization for nd_range kernels. If so it's probably worth also mentioning this in the description.

@hvdijk
Copy link
Contributor Author

hvdijk commented Jul 23, 2025

It seems this PR also removes the unsafe enqueuing optimization for nd_range kernels. If so it's probably worth also mentioning this in the description.

Done, thanks.

@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 12:36 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 12:57 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 12:57 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 14:51 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 15:31 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 15:31 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 15:47 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 16:16 — with GitHub Actions Inactive
@hvdijk hvdijk temporarily deployed to WindowsCILock July 23, 2025 16:16 — with GitHub Actions Inactive
@hvdijk
Copy link
Contributor Author

hvdijk commented Jul 23, 2025

@intel/llvm-gatekeepers This can be merged, thanks.

@igchor igchor merged commit 63c70a1 into intel:sycl Jul 23, 2025
48 of 49 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