[SYCL][Doc] Introduce test plan for virtual mem extension #15509
Conversation
This file contains 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
| Functionality provided by the extension is optional and it is only available if | ||
| corresponding aspect (`ext_oneapi_virtual_mem`) is supported by a device. | ||
|
|
||
| However, we still need to check that that we return a proper exception (with |
There was a problem hiding this comment.
Suggested change
| However, we still need to check that that we return a proper exception (with | |
| However, we still need to check that we return a proper exception (with |
0x12CC
reviewed
Oct 4, 2024
| This test case is intended to check that memory accesses to contiguous | ||
| virtual memory ranges are performed correctly. | ||
|
|
||
| A test reserve a number of virtual memory ranges which comprsise a contiguous |
There was a problem hiding this comment.
Suggested change
| A test reserve a number of virtual memory ranges which comprsise a contiguous | |
| A test reserve a number of virtual memory ranges which comprise a contiguous |
lbushi25
reviewed
Oct 4, 2024
| - that we can copy _from_ a buffer via accessor _to_ virtual memory | ||
| - that we can use `memset` on virtual memory | ||
| - that we can use `fill` on virtual memory | ||
|
|
There was a problem hiding this comment.
What about calling sycl::free on a virtual memory pointer?
If it's allowed, and the spec does say that it is allowed, then should we also check it has same effect as the unmap function?
There was a problem hiding this comment.
I think it should be an explicit exception in the spec about sycl::free because the core spec says the pointer must have been allocated using one of the USM allocation functions.
There was a problem hiding this comment.
I agree, free should be excluded by the extension. Maybe add another todo to mention this open?
| passed to it contains multiple devices and not every of them have | ||
| `ext_oneapi_virtual_mem` aspect | ||
| - **TODO**: do we need to check that returned granularity is not bigger than | ||
| maximum allocation size on a device? |
There was a problem hiding this comment.
Interesting question. It's not necessarily guaranteed by the spec, but maybe it would be good to make sure. I would assume the max allocation still applies to these.
| APIs results in UB and therefore there is nothing to test. This is probably an | ||
| incorrect approach of the spec, because we should be able to perform some checks | ||
| and inform user about an error. If/when the spec is updated, this test plan | ||
| should be updated as well to cover those new scenarios. |
There was a problem hiding this comment.
I think this was intentionally left out to avoid the overhead of calculating these things and requesting information that the user should have already requested. That said, this is not something I expect the user to call 1M times, so maybe the overhead would be fine.
| - that we can copy _from_ a buffer via accessor _to_ virtual memory | ||
| - that we can use `memset` on virtual memory | ||
| - that we can use `fill` on virtual memory | ||
|
|
There was a problem hiding this comment.
I agree, free should be excluded by the extension. Maybe add another todo to mention this open?
This was referenced Oct 22, 2024
steffenlarsen
pushed a commit
that referenced
this pull request
Oct 28, 2024
…tension (#15848) Based on the test plan #15509, this PR adds an e2e test checking memory granularities returned by `get_mem_granularity`. --------- Co-authored-by: Alexey Sachkov <[email protected]> Co-authored-by: Marcos Maronas <[email protected]>
YixingZhang007
pushed a commit
to YixingZhang007/llvm
that referenced
this pull request
Oct 30, 2024
…tension (intel#15848) Based on the test plan intel#15509, this PR adds an e2e test checking memory granularities returned by `get_mem_granularity`. --------- Co-authored-by: Alexey Sachkov <[email protected]> Co-authored-by: Marcos Maronas <[email protected]>
YixingZhang007
pushed a commit
to YixingZhang007/llvm
that referenced
this pull request
Oct 30, 2024
…eapi_virtual_mem` extension (intel#15887) Based on the test plan intel#15509, this PR adds an e2e test checking whether virtual memory range can correctly be accessed even if it was re-mapped to a different physical range.
martygrant
pushed a commit
that referenced
this pull request
Nov 4, 2024
) Based on the test plan #15509, this PR adds an e2e test described as `Basic access from a kernel`. It introduces the `helpers.hpp` common header file, which includes the `GetLCMGranularity` function, to be used in upcoming e2e tests for `sycl_ext_oneapi_virtual_mem` extension. --------- Co-authored-by: Alexey Sachkov <[email protected]> Co-authored-by: Steffen Larsen <[email protected]>
This was referenced Nov 4, 2024
steffenlarsen
pushed a commit
that referenced
this pull request
Nov 8, 2024
…irtual_mem` extension (#15988) Based on the test plan #15509, this PR adds an e2e test that performs following checks: - A check should be performed that we can successfully perform and immediately release a valid reservation of virtual memory. - A check should be performed that methods `get_context()`, `get_device()` and `size()` return correct values (i.e. ones which were passed to physical_mem constructor). - A check should be performed that a value returned from a valid call to `map()` is the same as `reinterpret_cast<void *>(ptr)`. - A check should be performed that we can change access mode of a virtual memory range and immediately see it changed. - A check should be performed that we can successfully map and immediately unmap a virtual memory range.
steffenlarsen
pushed a commit
that referenced
this pull request
Nov 12, 2024
…em` extension (#15974) Based on the test plan #15509, this PR adds an E2E test to ensure that USM APIs also accept the pointer to virtual memory. --------- Co-authored-by: Udit Agarwal <[email protected]>
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.
No description provided.