Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to align CTS and Spec for Memory #2233

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

martygrant
Copy link
Contributor

@martygrant martygrant commented Oct 23, 2024

intel/llvm#15619

  • Add tests for UR_RESULT_ERROR_INVALID_NULL/HOST_POINTER for urMemImageCreate and urMemBufferCreate
  • Add missing error condition to spec for urMemImageCreate for checking type of image description struct
  • Add tests for UR_RESULT_ERROR_INVALID_NULL_HANDLE/POINTER for urMemBuffer/ImageCreateWithnativeHandle
  • Remove skip for urMemImageCreateWithNativeHandle - buffer was invalid as it was never instantiated in fixture struct
  • Update image format used in urMemImageTest fixture as it was invalid and likely to cause previously skipped test to fail (which now pass)
  • Add missing DDI table entry for urMemImageCreateWithNativeHandle for OpenCL
  • Add test for using different ur_mem_flag_t flags with urMemBufferPartition
  • Add missing UR_MEM_INFO_REFERENCE_COUNT to spec for urMemGetInfo and added test for this in urMemRetain/Release
  • Removed assert in L0 urMemGetInfo which would fail if mem type is not image or the query is not a context query
  • Fixed potential bug with HIP urMemGetNativeHandle which would fail if the mem handle was an image because use of std::variant

@github-actions github-actions bot added loader Loader related feature/bug conformance Conformance test suite issues. specification Changes or additions to the specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues labels Oct 23, 2024
@martygrant martygrant marked this pull request as ready for review October 24, 2024 09:09
@martygrant martygrant requested review from a team as code owners October 24, 2024 09:09
@martygrant
Copy link
Contributor Author

@RossBrunton @hdelan @PietroGhg apologies you all reviewed a previous PR (#2177) for this change but I rebased a number of times fighting with CI failures and made some changes so though it would be good to just have it reviewed afresh. The main notable change is in the hip adapter so image buffer types can be handled in urMemGetNativeHandle, and not letting some tests pass when entry points are unsupported if they aren't optional.

- Add tests for UR_RESULT_ERROR_INVALID_NULL/HOST_POINTER for urMemImageCreate and urMemBufferCreate
- Add missing error condition to spec for urMemImageCreate for checking type of image description struct
- Add tests for UR_RESULT_ERROR_INVALID_NULL_HANDLE/POINTER for urMemBuffer/ImageCreateWithnativeHandle
- Remove skip for urMemImageCreateWithNativeHandle - buffer was invalid as it was never instantiated in fixture struct
- Update image format used in urMemImageTest fixture as it was invalid and likely to cause previously skipped test to fail (which now pass)
- Add missing DDI table entry for urMemImageCreateWithNativeHandle for OpenCL
- Add test for using different ur_mem_flag_t flags with urMemBufferPartition
- Add missing UR_MEM_INFO_REFERENCE_COUNT to spec for urMemGetInfo and added test for this in urMemRetain/Release
- Removed assert in L0 urMemGetInfo which would fail if mem type is not image or the query is not a context query
- Fixed potential bug with HIP urMemGetNativeHandle which would fail if the mem handle was an image because use of std::variant
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 level-zero L0 adapter specific issues loader Loader related feature/bug specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants