Skip to content
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

Add two new properties to ur_kernel_group_info_t #1996

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

frasercrmck
Copy link
Contributor

@frasercrmck frasercrmck commented Aug 20, 2024

These two properties allow the program to specify a maximum work-group size in various ways.

They are intended to be targeted from languages such as SYCL (see intel/llvm#14518).

This PR implements them for CUDA and Native CPU. It should also be able support them for HIP, in the same fashion. Other adapters using SPIR-V and/or Level Zero will need further changes to both of those specifications.

@frasercrmck frasercrmck requested review from a team as code owners August 20, 2024 16:49
@github-actions github-actions bot added loader Loader related feature/bug specification Changes or additions to the specification cuda CUDA adapter specific issues labels Aug 20, 2024
@frasercrmck frasercrmck requested review from a team as code owners August 21, 2024 15:11
@github-actions github-actions bot added level-zero L0 adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues labels Aug 21, 2024
@github-actions github-actions bot added the conformance Conformance test suite issues. label Aug 22, 2024
@hdelan
Copy link
Contributor

hdelan commented Aug 27, 2024

LGTM but I think it'd be a good idea to add a SYCL kernel here that uses this SYCL property.

@frasercrmck
Copy link
Contributor Author

LGTM but I think it'd be a good idea to add a SYCL kernel here that uses this SYCL property.

I think that's a good idea but would depend on my DPC++ PR to be merged first, so that will have to be future work.

@frasercrmck frasercrmck changed the title [Draft] Add two new properties to ur_kernel_group_info_t Add two new properties to ur_kernel_group_info_t Aug 28, 2024
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU LGTM, thank you

@frasercrmck
Copy link
Contributor Author

Ping @oneapi-src/unified-runtime-opencl-write and @oneapi-src/level-zero-maintainers, thanks

@frasercrmck
Copy link
Contributor Author

Ping @oneapi-src/level-zero-maintainers, thanks

Copy link
Contributor

@pbalcer pbalcer left a comment

Choose a reason for hiding this comment

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

lgtm, just one nit.

Also, the windows build is failing - a rebase should fix that.

@pbalcer pbalcer added the ready to merge Added to PR's which are ready to merge label Sep 10, 2024
@frasercrmck frasercrmck force-pushed the ur-max-wg-size-props branch 2 times, most recently from f71a91e to bcca15a Compare September 12, 2024 13:42
These two properties allow the program to specify a maximum work-group
size in various ways.

They are intended to be targeted from languages such as SYCL (see
intel/llvm#14518).

This PR implements them for CUDA and Native CPU. It should also be able
support them for HIP, in the same fashion. Other adapters using SPIR-V
and/or Level Zero would require further changes to both of those
specifications.
@aarongreig aarongreig merged commit 7ecf64d into oneapi-src:main Sep 25, 2024
74 of 77 checks passed
@frasercrmck frasercrmck deleted the ur-max-wg-size-props branch September 25, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants