Skip to content

Conversation

callumfare
Copy link
Contributor

#1226 added a device handle parameter to urMemGetNativeHandle, but some uses in SYCL pass a null device handle, which causes unexpected failures when the validation layer is enabled, particularly for adapters where the device isn't required (OpenCL and L0).

This makes the device optional, with adapters allowed to fail when given a null device if they require it to get the native handle. This should preserve the intended implementation of all the adapters, and just moves the null handle check out of the validation layer.

@callumfare callumfare requested review from a team as code owners June 24, 2024 16:12
@callumfare callumfare requested a review from keyradical June 24, 2024 16:12
@github-actions github-actions bot added loader Loader related feature/bug conformance Conformance test suite issues. specification Changes or additions to the specification cuda CUDA adapter specific issues hip HIP adapter specific issues labels Jun 24, 2024
Copy link
Contributor

@hdelan hdelan left a comment

Choose a reason for hiding this comment

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

Sorry I should have done this in the original patch. LGTM

@callumfare callumfare force-pushed the callum/mem_getnative_optional_device branch from 8d82df7 to 27e030f Compare June 26, 2024 08:59
@callumfare
Copy link
Contributor Author

intel-llvm PR: intel/llvm#14299

@callumfare callumfare added the ready to merge Added to PR's which are ready to merge label Jun 26, 2024
@hdelan hdelan mentioned this pull request Jun 28, 2024
@callumfare
Copy link
Contributor Author

This was included in #1804 which has now merged

@callumfare callumfare closed this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues loader Loader related feature/bug ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants