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

[SYCL] Fix for alignment requests > 64KB. #4263

Merged
merged 5 commits into from
Aug 10, 2021
Merged

[SYCL] Fix for alignment requests > 64KB. #4263

merged 5 commits into from
Aug 10, 2021

Conversation

rdeodhar
Copy link
Contributor

@rdeodhar rdeodhar commented Aug 5, 2021

This change caps alignment of allocation requests at 64KB. That is the default alignment supported by level_zero, and no other alignments are supported.

Signed-off-by: rdeodhar rajiv.deodhar@intel.com

Signed-off-by: rdeodhar <rajiv.deodhar@intel.com>
@rdeodhar rdeodhar marked this pull request as ready for review August 5, 2021 16:01
@@ -617,6 +617,10 @@ void *USMAllocContext::USMAllocImpl::allocate(size_t Size, size_t Alignment) {
if (Alignment <= 1)
return allocate(Size);

// L0 does not support alignments > 64KB, so we don't either.
if (Alignment > 65536)
Alignment = 65536;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will silently NOT satisfy the requested alignment. Should we give an error instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was also debating with myself about that.
What type of error would that be? There is no existing PI_ERROR_ that could be used, other than UNKNOWN.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could just use PI_INVALID_VALUE or even create a new PI error code (later will need some design thought how to not overlap with existing errors still based of OpenCL errors).

smaslov-intel
smaslov-intel previously approved these changes Aug 5, 2021
Copy link
Contributor

@smaslov-intel smaslov-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader merged commit 7dfaf3b into intel:sycl Aug 10, 2021
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