[SYCL][HIP][CUDA] Use new version of piMemGetNativeHandle and add test #12297

hdelan · 2024-01-04T16:12:36Z

We want to change the signature of piMemGetNativeHandle for reasons explained here oneapi-src/unified-runtime#1199

Corresponding UR PR: oneapi-src/unified-runtime#1226

A previous PR added a new entry point #12199 but it was decided that it is better to modify the existing entry point

hdelan · 2024-01-05T10:01:04Z

Lint failing because of #12199 (comment)

steffenlarsen · 2024-01-05T10:58:26Z

Converting to draft while UR changes are still in flight.

sycl/include/sycl/detail/pi.h

muhammad-tanvir-1211 · 2024-01-12T11:30:22Z

This PR is blocking the following two PRs on oneMKL as all the buffer tests (with accessor interop) are failing for the rocblas backend with PI_ERROR_INVALID_OPERATION.
oneapi-src/oneMKL#428
oneapi-src/oneMKL#425

Logs:
PR_425.txt
PR_428.txt

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp

hdelan · 2024-01-23T16:00:32Z

Friendly ping @intel/dpcpp-nativecpu-pi-reviewers @intel/unified-runtime-reviewers

kbenzie · 2024-01-23T16:03:03Z

oneapi-src/unified-runtime#1226 needs to merged and the UR tag updated before the UR changes can be approved.

hdelan · 2024-01-23T16:10:41Z

OK sure. I will add the ready-to-merge tag in the UR PR

PietroGhg

Native CPU lgtm, thank you

hdelan · 2024-01-23T17:42:44Z

@kbenzie is it not worth reviewing this before oneapi-src/unified-runtime#1226 is merged? oneapi-src/unified-runtime#1226 is an ABI breaking change so once it is merged, this should be merged immediately.

kbenzie · 2024-01-31T15:52:10Z

@intel/llvm-reviewers-cuda @intel/llvm-reviewers-runtime please review

sycl/source/detail/buffer_impl.cpp

sycl/source/detail/memory_manager.cpp

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp

Co-authored-by: ldrumm <ldrumm@rtps.co>

github-actions · 2024-01-31T17:30:28Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Add a comment describing what test4 does and change assert(0) to exit(1) in check helper and also fix clang-format

Remove some redundant parts of test and add descriptive names

hdelan · 2024-02-01T09:32:52Z

@ldrumm do these changes address your concerns?

ldrumm

@ldrumm do these changes address your concerns?

Looks like github auto-resolved some of my comments but I don't see that material changes have been made. Other than them it looks ready

Co-authored-by: ldrumm <ldrumm@rtps.co>

hdelan · 2024-02-01T10:03:40Z

@ldrumm I actually had forgotten to push to the remote last night before finishing. I have just done so and your changes should all have been made. Let me know what you think

kbenzie · 2024-02-01T11:46:58Z

@intel/llvm-reviewers-runtime would really appriciate getting this reviewed ASAP as this is holding up other UR changes.

ldrumm · 2024-02-01T14:06:33Z

@ldrumm I actually had forgotten to push to the remote last night before finishing. I have just done so and your changes should all have been made. Let me know what you think

Thanks. This looks good.

sycl/source/detail/buffer_impl.cpp

sycl/source/detail/memory_manager.cpp

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp

sycl/source/detail/memory_manager.cpp

kbenzie · 2024-02-01T16:38:30Z

@intel/llvm-gatekeepers please merge

hdelan requested review from a team as code owners January 4, 2024 16:12

hdelan requested a review from steffenlarsen January 4, 2024 16:12

steffenlarsen marked this pull request as draft January 5, 2024 10:58

steffenlarsen reviewed Jan 5, 2024

View reviewed changes

sycl/include/sycl/detail/pi.h Show resolved Hide resolved

muhammad-tanvir-1211 mentioned this pull request Jan 12, 2024

[CUDA][HIP] Use device to get native context oneapi-src/oneMKL#425

Merged

muhammad-tanvir-1211 mentioned this pull request Jan 12, 2024

[BLAS][rocBLAS] Update rocBLAS backend oneapi-src/oneMKL#428

Merged

2 tasks

GeorgeWeb reviewed Jan 12, 2024

View reviewed changes

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp Show resolved Hide resolved

hdelan marked this pull request as ready for review January 23, 2024 09:12

hdelan force-pushed the use-updated-get-native-mem branch from 037bbf6 to eb331f1 Compare January 23, 2024 09:21

hdelan had a problem deploying to WindowsCILock January 23, 2024 09:21 — with GitHub Actions Failure

hdelan temporarily deployed to WindowsCILock January 23, 2024 09:41 — with GitHub Actions Inactive

hdelan temporarily deployed to WindowsCILock January 23, 2024 11:20 — with GitHub Actions Inactive

hdelan temporarily deployed to WindowsCILock January 23, 2024 11:51 — with GitHub Actions Inactive

PietroGhg approved these changes Jan 23, 2024

View reviewed changes

hdelan force-pushed the use-updated-get-native-mem branch from 594a185 to b68761c Compare January 30, 2024 16:14

hdelan temporarily deployed to WindowsCILock January 30, 2024 17:15 — with GitHub Actions Inactive

hdelan temporarily deployed to WindowsCILock January 30, 2024 17:43 — with GitHub Actions Inactive

hdelan force-pushed the use-updated-get-native-mem branch from b68761c to 0c9cf59 Compare January 31, 2024 10:13

hdelan had a problem deploying to WindowsCILock January 31, 2024 10:16 — with GitHub Actions Error

hdelan had a problem deploying to WindowsCILock January 31, 2024 10:31 — with GitHub Actions Error

hdelan temporarily deployed to WindowsCILock January 31, 2024 10:46 — with GitHub Actions Inactive

hdelan had a problem deploying to WindowsCILock January 31, 2024 11:46 — with GitHub Actions Failure

hdelan temporarily deployed to WindowsCILock January 31, 2024 14:51 — with GitHub Actions Inactive

ldrumm requested changes Jan 31, 2024

View reviewed changes

Apply suggestions from code review

2a05505

Co-authored-by: ldrumm <ldrumm@rtps.co>

hdelan had a problem deploying to WindowsCILock January 31, 2024 17:26 — with GitHub Actions Error

hdelan had a problem deploying to WindowsCILock January 31, 2024 17:32 — with GitHub Actions Error

Update test

32f957c

Add a comment describing what test4 does and change assert(0) to exit(1) in check helper and also fix clang-format

hdelan force-pushed the use-updated-get-native-mem branch from db726dd to 32f957c Compare January 31, 2024 17:36

hdelan temporarily deployed to WindowsCILock January 31, 2024 17:36 — with GitHub Actions Inactive

Update test

14e351e

Remove some redundant parts of test and add descriptive names

hdelan temporarily deployed to WindowsCILock January 31, 2024 18:06 — with GitHub Actions Inactive

ldrumm approved these changes Feb 1, 2024

View reviewed changes

Update sycl/test-e2e/HostInteropTask/interop-task-hip.cpp

7725cc9

Co-authored-by: ldrumm <ldrumm@rtps.co>

hdelan temporarily deployed to WindowsCILock February 1, 2024 10:03 — with GitHub Actions Inactive

hdelan temporarily deployed to WindowsCILock February 1, 2024 10:30 — with GitHub Actions Inactive

maarquitos14 reviewed Feb 1, 2024

View reviewed changes

sycl/source/detail/buffer_impl.cpp Show resolved Hide resolved

sycl/source/detail/memory_manager.cpp Show resolved Hide resolved

sycl/test-e2e/HostInteropTask/interop-task-hip.cpp Show resolved Hide resolved

sycl/source/detail/memory_manager.cpp Show resolved Hide resolved

Add comments explaining why nullptr is passed for dev arg

3245288

hdelan temporarily deployed to WindowsCILock February 1, 2024 14:47 — with GitHub Actions Inactive

hdelan temporarily deployed to WindowsCILock February 1, 2024 15:15 — with GitHub Actions Inactive

maarquitos14 approved these changes Feb 1, 2024

View reviewed changes

ldrumm merged commit 8427bd2 into intel:sycl Feb 1, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][HIP][CUDA] Use new version of piMemGetNativeHandle and add test #12297

[SYCL][HIP][CUDA] Use new version of piMemGetNativeHandle and add test #12297

hdelan commented Jan 4, 2024 •

edited

Loading

hdelan commented Jan 5, 2024

steffenlarsen commented Jan 5, 2024

muhammad-tanvir-1211 commented Jan 12, 2024

hdelan commented Jan 23, 2024

kbenzie commented Jan 23, 2024

hdelan commented Jan 23, 2024

PietroGhg left a comment

hdelan commented Jan 23, 2024

kbenzie commented Jan 31, 2024

github-actions bot commented Jan 31, 2024 •

edited

Loading

hdelan commented Feb 1, 2024

ldrumm left a comment

hdelan commented Feb 1, 2024 •

edited

Loading

kbenzie commented Feb 1, 2024

ldrumm commented Feb 1, 2024

kbenzie commented Feb 1, 2024

[SYCL][HIP][CUDA] Use new version of piMemGetNativeHandle and add test #12297

[SYCL][HIP][CUDA] Use new version of piMemGetNativeHandle and add test #12297

Conversation

hdelan commented Jan 4, 2024 • edited Loading

hdelan commented Jan 5, 2024

steffenlarsen commented Jan 5, 2024

muhammad-tanvir-1211 commented Jan 12, 2024

hdelan commented Jan 23, 2024

kbenzie commented Jan 23, 2024

hdelan commented Jan 23, 2024

PietroGhg left a comment

Choose a reason for hiding this comment

hdelan commented Jan 23, 2024

kbenzie commented Jan 31, 2024

github-actions bot commented Jan 31, 2024 • edited Loading

hdelan commented Feb 1, 2024

ldrumm left a comment

Choose a reason for hiding this comment

hdelan commented Feb 1, 2024 • edited Loading

kbenzie commented Feb 1, 2024

ldrumm commented Feb 1, 2024

kbenzie commented Feb 1, 2024

hdelan commented Jan 4, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited

Loading

hdelan commented Feb 1, 2024 •

edited

Loading