Skip to content

Make USM pools optional with a device query to report support. #2165

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
Merged
merged 2 commits into from
Nov 4, 2024
Merged

Make USM pools optional with a device query to report support. #2165

merged 2 commits into from
Nov 4, 2024

Conversation

@aarongreig
Copy link
Contributor

@aarongreig aarongreig commented Oct 2, 2024

No description provided.

aarongreig
aarongreig commented Oct 2, 2024
View reviewed changes
test/conformance/usm/usm_adapter_opencl.match
urUSMDeviceAllocAlignmentTest.SuccessAlignedAllocations/Intel_R__OpenCL___{{.*}}___UsePoolEnabled_64_8
urUSMDeviceAllocAlignmentTest.SuccessAlignedAllocations/Intel_R__OpenCL___{{.*}}___UsePoolEnabled_64_512
urUSMDeviceAllocAlignmentTest.SuccessAlignedAllocations/Intel_R__OpenCL___{{.*}}___UsePoolEnabled_64_2048
urUSMGetMemAllocInfoTest.Success/Intel_R__OpenCL___{{.*}}___UR_USM_ALLOC_INFO_POOL
Copy link
Contributor Author

@aarongreig aarongreig Oct 2, 2024

Choose a reason for hiding this comment

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

I'm making this an [optional-query] as part of #1332

@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 2, 2024
@aarongreig aarongreig force-pushed the aaron/makeUSMPoolsOptional branch from f2e6794 to 850e941 Compare October 4, 2024 11:31
@aarongreig
Copy link
Contributor Author

aarongreig commented Oct 4, 2024

LLVM PR intel/llvm#15599

@aarongreig aarongreig marked this pull request as ready for review October 4, 2024 11:34
@aarongreig aarongreig requested review from a team as code owners October 4, 2024 11:34
@aarongreig aarongreig requested a review from ldrumm October 4, 2024 11:34
PietroGhg
PietroGhg approved these changes Oct 4, 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 changes LGTM, do we also want to change the wording in the usm*alloc functions from "This function must support memory pooling." to "This function may support memory pooling." or something like that?

@aarongreig
Copy link
Contributor Author

aarongreig commented Oct 7, 2024

Native CPU changes LGTM, do we also want to change the wording in the usm*alloc functions from "This function must support memory pooling." to "This function may support memory pooling." or something like that?

oh good catch, I'm just going to delete those lines - I think the UNSUPPORTED_FEATURE return + explanation are sufficient to imply pooling is optional

@aarongreig aarongreig force-pushed the aaron/makeUSMPoolsOptional branch from b79c317 to 89692a9 Compare October 7, 2024 10:28
@aarongreig
Copy link
Contributor Author

aarongreig commented Oct 11, 2024

ping @oneapi-src/unified-runtime-maintain @oneapi-src/unified-runtime-opencl-write @oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-hip-write @oneapi-src/unified-runtime-level-zero-write

@aarongreig
Copy link
Contributor Author

aarongreig commented Oct 29, 2024

@oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-hip-write @oneapi-src/unified-runtime-level-zero-write please take a look, the adapter changes are pretty trivial

nrspruit
nrspruit approved these changes Oct 29, 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

@aarongreig aarongreig force-pushed the aaron/makeUSMPoolsOptional branch from 89692a9 to a0cf2ce Compare October 30, 2024 11:22
@aarongreig aarongreig added the ready to merge Added to PR's which are ready to merge label Oct 30, 2024
@callumfare callumfare merged commit b0a9e2b into oneapi-src:main Nov 4, 2024
75 of 78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nrspruit nrspruit nrspruit approved these changes

@ldrumm ldrumm Awaiting requested review from ldrumm ldrumm was automatically assigned from oneapi-src/unified-runtime-hip-write

+3 more reviewers

@kbenzie kbenzie kbenzie approved these changes

@PietroGhg PietroGhg PietroGhg approved these changes

@hdelan hdelan hdelan approved these changes

Reviewers whose approvals may not affect merge requirements

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 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.

6 participants

@aarongreig @kbenzie @nrspruit @PietroGhg @hdelan @callumfare