-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL] Fix host device local accessor alignment #5554
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
Conversation
Local kernel arguments must be aligned to the type size, so simply using `std::vector<char>` doesn't always provide the correct alignment. Instead using the `aligned_allocator` makes sure the allocation is aligned maximum possible required alignment.
I don't see how an alignment could be forced without changing the ABI, and the ABI breaking change is not allowed until major update. @intel/llvm-reviewers-runtime , any idea? |
Can we modify |
6c214c8
to
7feb6d7
Compare
Good shout @romanovvlad , updated as suggested, so it should now fix the alignment without breaking the ABI. Not exactly related to this patch but I'm a little confused about the |
/summary:run |
@npmiller, could you update the PR description, so I can use it as a commit message, please? |
PR description updated to reflect the current patch |
Local kernel arguments must be aligned to the type size, simply using
std::vector<char>
doesn't always provide the correct alignment. So this patch adds extra padding to the vector and ensures that the pointer returned for the accessor is actually aligned to the type size.This issue was exposed by: intel/llvm-test-suite#608, which was a follow up to fixing local accessor alignment for the CUDA plugin.