Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL][L0] Fix pinned memory allocation test. #215

Merged
merged 3 commits into from
Apr 5, 2021

Conversation

rbegam
Copy link

@rbegam rbegam commented Apr 2, 2021

Pinned memory allocation is not currently supported by L0.

Signed-off-by: rbegam [email protected]

Pinned memory allocation is not currently supported by L0.

Signed-off-by: rbegam <[email protected]>
@@ -1,4 +1,4 @@
// REQUIRES: level_zero || cuda
// REQUIRES: cuda

Choose a reason for hiding this comment

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

Does the test failing on Level_Zero satbily? If it is I would prefer to mark test as XFAIL: level_zero

Copy link
Author

@rbegam rbegam Apr 2, 2021

Choose a reason for hiding this comment

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

modified the CHECK

@smaslov-intel
Copy link

@rbegam : why does it fail? I known we don't truly support pinned memory, but my expectation is that it is just a nop for Level-Zero for now. So why this is failing?

@rbegam
Copy link
Author

rbegam commented Apr 2, 2021

@smaslov-intel, this test checks SYCL_PI_TRACE for zeMemAllocHost() (https://github.com/intel/llvm-test-suite/pull/215/files#diff-fb8c60f542fd5b66745807155566683a3d084c409b5375d523268bb46f06d222L43) while buffer create. This is failing for the discrete gpu as we don't call zeMemAllocHost() anymore. For integrated it's passing.

@smaslov-intel
Copy link

@smaslov-intel, this test checks SYCL_PI_TRACE for zeMemAllocHost() (https://github.com/intel/llvm-test-suite/pull/215/files#diff-fb8c60f542fd5b66745807155566683a3d084c409b5375d523268bb46f06d222L43) while buffer create. This is failing for the discrete gpu as we don't call zeMemAllocHost() anymore. For integrated it's passing.

Ah, OK. Then I suggest that we remove the check for zeMemAllocHost (since we know now it is unrelated to pinning) and make this test to check that {ext::oneapi::property::buffer::use_pinned_host_memory()} is accepted.

@rbegam
Copy link
Author

rbegam commented Apr 2, 2021

@smaslov-intel cuda has that call checked. let's separate the tests for level_zero (remove zeMemAllocHost) and cuda (as is).

@smaslov-intel
Copy link

@smaslov-intel cuda has that call checked. let's separate the tests for level_zero (remove zeMemAllocHost) and cuda (as is).

But there is no zeMemAllocHost in CUDA. This is Level-Zero interface

@rbegam rbegam requested a review from vladimirlaz April 3, 2021 00:03
@vladimirlaz vladimirlaz merged commit 7962850 into intel:intel Apr 5, 2021
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants