[SYCL] Small patch on annotated USM allocation #11801
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dm-vodopyanov
approved these changes
Nov 7, 2023
|
@wangdi4 |
againull
requested changes
Nov 9, 2023
wangdi4
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
wangdi4
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
wangdi4
force-pushed
the
annotated_malloc_patch
branch
from
7de8a33 to
a063dd2
Compare
wangdi4
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
wangdi4
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
wangdi4
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
wangdi4
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
wangdi4
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
wangdi4
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
The e2e test of the annotated USM API has been updated to fix the CI failure |
|
Hi @dm-vodopyanov @againull, the PR is updated. |
againull
approved these changes
Nov 16, 2023
wangdi4
added a commit
to wangdi4/llvm
that referenced
this pull request
Dec 13, 2023
This PR is a small change on the implementation of `aligned_alloc_annotated<T>`, which is a variant of the SYCL annotated USM allocation: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_usm_malloc_properties.asciidoc Right now `aligned_alloc_annotated<T>` is implemented by calling `sycl::aligned_alloc` (allocation in bytes) with a specified alignment (which is a combination (i.e. least common multiple) of the compile-time `alignment<K>` property and the runtime argument), and then cast the returned pointer type from `void *` to `T*` This does not cover the case where T has a extended alignment as follows ``` struct alignas(256) MyStruct { int x; }; ``` in this case, both the combined alignment and `alignof(T)` should be considered, and the greater value of the two should be the final alignment passed to the SYCL runtime. Therefore, this PR changes the impl by directly calling the SYCL core usm API: `aligned_alloc<T>`. Some e2e tests are updated with this header change: when `alignof(T)` is large enough, the `aligned_alloc_annotated<T>` is no longer expected to return null when the specified alignment is not a power of 2, because `alignof(T)` (always a power of 2) is the final alignment passed to the SYCL runtime. Therefore, we bump up the specified alignments in these cases to make sure they are always larger the `alignof(T)`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a small change on the implementation of
aligned_alloc_annotated<T>, which is a variant of the SYCL annotated USM allocation:https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_usm_malloc_properties.asciidoc
Right now
aligned_alloc_annotated<T>is implemented by callingsycl::aligned_alloc(allocation in bytes) with a specified alignment (which is a combination (i.e. least common multiple) of the compile-timealignment<K>property and the runtime argument), and then cast the returned pointer type fromvoid *toT*This does not cover the case where T has a extended alignment as follows
in this case, both the combined alignment and
alignof(T)should be considered, and the greater value of the two should be the final alignment passed to the SYCL runtime. Therefore, this PR changes the impl by directly calling the SYCL core usm API:aligned_alloc<T>.Some e2e tests are updated with this header change: when
alignof(T)is large enough, thealigned_alloc_annotated<T>is no longer expected to return null when the specified alignment is not a power of 2, becausealignof(T)(always a power of 2) is the final alignment passed to the SYCL runtime. Therefore, we bump up the specified alignments in these cases to make sure they are always larger thealignof(T)