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.

Sign up for GitHub

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

Jump to bottom

Improvements to align CTS and Spec for Virtual Memory #2272

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Improvements to align CTS and Spec for Virtual Memory #2272

merged 1 commit into from
Dec 12, 2024
+941 −43

Conversation

martygrant
Copy link
Contributor

@martygrant martygrant commented Oct 31, 2024 •
edited
Loading

intel/llvm#15965

  • UR_RESULT_ERROR_INVALID_ENUMERATION test for urVirtualMemSetAccess
  • Add test for urVirtualMemMap with different access flags
  • Add test for urVirtualMemSetAccess with different access flags
  • InvalidEnumeration test for urPhysicalMemCreate and changed some tests to only run once - different size values were not needed for these
  • Add test for urPhysicalMemCreate with different flags (only one placeholder is available, this is just a fixture for future additions)
  • Add new urPhysicalMemGetInfo entry point along with a reference count enum. Added adapter implementations for this
  • Add a test for urPhysicalMemGetInfo and modified urPhysicalMemRelease/Retain to use GetInfo to verify reference count is updated accordingly

@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 native-cpu Native CPU adapter specific issues labels Oct 31, 2024
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch 2 times, most recently from 2dde2cb to 25b5bfe Compare November 1, 2024 17:21
@martygrant martygrant mentioned this pull request Nov 1, 2024
Merged
@martygrant martygrant marked this pull request as ready for review November 4, 2024 09:46
@martygrant martygrant requested review from a team as code owners November 4, 2024 09:46
@martygrant martygrant requested a review from GeorgeWeb November 4, 2024 09:46
GeorgeWeb
GeorgeWeb approved these changes Nov 4, 2024
View reviewed changes
Copy link
Contributor

@GeorgeWeb GeorgeWeb left a comment

Choose a reason for hiding this comment

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

CUDA and HIP adapter changes look good.

@kbenzie kbenzie requested a review from steffenlarsen November 6, 2024 12:01
@kbenzie
Copy link
Contributor

kbenzie commented Nov 6, 2024

@steffenlarsen could you have a look at the spec updates please?

steffenlarsen
steffenlarsen approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable addition to me. 👍

@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 25b5bfe to e825f58 Compare November 6, 2024 17:43
@martygrant martygrant mentioned this pull request Nov 6, 2024
Open
12 tasks
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from ade8572 to a64c93f Compare November 7, 2024 12:07
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from a64c93f to 5ba5c2f Compare November 14, 2024 15:02
@martygrant martygrant mentioned this pull request Nov 14, 2024
Merged
nrspruit
nrspruit approved these changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@nrspruit nrspruit left a comment

Choose a reason for hiding this comment

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

LGTM for L0

@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch 5 times, most recently from db6e1a6 to 3706cbc Compare November 21, 2024 08:56
@martygrant
Copy link
Contributor Author

@oneapi-src/unified-runtime-native-cpu-write could someone take a look at this PR please?
@oneapi-src/unified-runtime-cuda-write would someone mind taking an additional look at the CUDA adapter changes please? I made a small change after @GeorgeWeb approved the changes before

@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 3706cbc to 6779ec3 Compare November 22, 2024 09:30
@github-actions github-actions bot added the opencl OpenCL adapter specific issues label Nov 22, 2024
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from 6779ec3 to dcb99fd Compare November 22, 2024 09:51
aarongreig
aarongreig approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@aarongreig aarongreig left a comment

Choose a reason for hiding this comment

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

LGMT save for a couple of points

test/conformance/virtual_memory/urPhysicalMemGetInfo.cpp Outdated Show resolved Hide resolved
test/conformance/virtual_memory/urPhysicalMemRelease.cpp Outdated Show resolved Hide resolved
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from dcb99fd to bc72267 Compare November 22, 2024 11:19
PietroGhg
PietroGhg approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@PietroGhg PietroGhg left a comment

Choose a reason for hiding this comment

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

Native CPU lgtm, thank you

@martygrant martygrant added the ready to merge Added to PR's which are ready to merge label Nov 25, 2024
@martygrant
Improvements to align CTS and Spec for Virtual Memory:
800b452 
- UR_RESULT_ERROR_INVALID_ENUMERATION test for urVirtualMemSetAccess
- Add test for urVirtualMemMap with different access flags
- Add test for urVirtualMemSetAccess with different access flags
- InvalidEnumeration test for urPhysicalMemCreate and changed some tests to only run once - different size values were not needed for these
- Add test for urPhysicalMemCreate with different flags (only one placeholder is available, this is just a fixture for future additions)
- Add new urPhysicalMemGetInfo entry point along with a reference count enum. Added adapter implementations for this
- Add a test for urPhysicalMemGetInfo and modified urPhysicalMemRelease/Retain to use GetInfo to verify reference count is updated accordingly
@martygrant martygrant force-pushed the martin/virtual-memory-cts-spec-gap branch from bc72267 to 800b452 Compare December 12, 2024 09:38
@martygrant martygrant merged commit 6d4eec8 into oneapi-src:main Dec 12, 2024
73 checks passed
martygrant added a commit to martygrant/llvm that referenced this pull request Dec 12, 2024
@martygrant
[UR] Update tag to 6d4eec8 for:
088d39f 
- oneapi-src/unified-runtime#2272
- oneapi-src/unified-runtime#2336
martygrant added a commit to martygrant/llvm that referenced this pull request Dec 12, 2024
@martygrant
[UR] Update tag to 6d4eec8 for:
7d878cc 
- oneapi-src/unified-runtime#2272
- oneapi-src/unified-runtime#2336
martygrant added a commit to intel/llvm that referenced this pull request Dec 13, 2024
@martygrant
[UR] Update tag to 6d4eec8c for UR#2272 and UR#2336 (#15965)
a37bba8 
oneapi-src/unified-runtime#2272
oneapi-src/unified-runtime#2336
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbenzie kbenzie kbenzie left review comments

@frasercrmck frasercrmck frasercrmck left review comments

@RossBrunton RossBrunton RossBrunton left review comments

@steffenlarsen steffenlarsen steffenlarsen approved these changes

@PietroGhg PietroGhg PietroGhg approved these changes

@aarongreig aarongreig aarongreig approved these changes

@nrspruit nrspruit nrspruit approved these changes

@GeorgeWeb GeorgeWeb GeorgeWeb approved these changes

Assignees
No one assigned
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 native-cpu Native CPU adapter specific issues opencl OpenCL adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@martygrant @kbenzie @RossBrunton @frasercrmck @GeorgeWeb @nrspruit @aarongreig @PietroGhg @steffenlarsen