Skip to content

[UR][CUDA][HIP] Refactor setKernelParams #18518

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from
Open

[UR][CUDA][HIP] Refactor setKernelParams #18518

wants to merge 1 commit into from

Conversation

npmiller
Copy link
Contributor

@npmiller npmiller commented May 16, 2025 •
edited
Loading

  • Remove unused Context parameters
  • Avoid unnecessary copy in guessLocalWorkSize
  • Simplify the control flow in setKernelParams
  • Move cached properties fetching code to constructors
  • Query HIP for occupancy in guessLocalWorkSize

@npmiller npmiller temporarily deployed to WindowsCILock May 16, 2025 17:16 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 16, 2025 19:12 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 16, 2025 19:12 — with GitHub Actions Inactive
@npmiller
[UR][CUDA][HIP] Refactor setKernelParams
226e80e 
- Remove unused Context parameters
- Avoid unnecessary copy in `guessLocalWorkSize`
- Simplify the control flow in setKernelParams
- Move cached properties fetching code to constructors
- Query HIP for occupancy in `guessLocalWorkSize`
@npmiller npmiller force-pushed the set-kernel-params branch from 9598ff2 to 226e80e Compare May 20, 2025 14:50
@npmiller npmiller temporarily deployed to WindowsCILock May 20, 2025 14:50 — with GitHub Actions Inactive
@npmiller npmiller marked this pull request as ready for review May 20, 2025 14:51
@npmiller npmiller requested review from a team as code owners May 20, 2025 14:51
@npmiller npmiller requested review from Bensuo and frasercrmck May 20, 2025 14:51
@npmiller npmiller temporarily deployed to WindowsCILock May 20, 2025 15:53 — with GitHub Actions Inactive
@npmiller npmiller temporarily deployed to WindowsCILock May 20, 2025 15:53 — with GitHub Actions Inactive
Bensuo
Bensuo approved these changes May 20, 2025
View reviewed changes
Copy link
Contributor

@Bensuo Bensuo left a comment

Choose a reason for hiding this comment

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

Command-buffer changes LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bensuo Bensuo Bensuo approved these changes

@frasercrmck frasercrmck Awaiting requested review from frasercrmck frasercrmck is a code owner automatically assigned from intel/llvm-reviewers-cuda

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@npmiller @Bensuo