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

[UR] Initial spec for the enqueue allocation API #2295

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

Conversation

kswiecicki
Copy link
Contributor

Those changes are based on the initial PR (#2180) with minor changes, wording and CI testing fixes.

@github-actions github-actions bot added loader Loader related feature/bug common Changes or additions to common utilities specification Changes or additions to the specification experimental Experimental feature additions/changes/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 Nov 7, 2024
@kswiecicki kswiecicki marked this pull request as ready for review November 12, 2024 13:17
@kswiecicki kswiecicki requested review from a team as code owners November 12, 2024 13:17
@kswiecicki kswiecicki force-pushed the async-spec branch 2 times, most recently from 2ba7bc6 to bc9d8a4 Compare November 19, 2024 12:11
First basic work in progress spec.
@github-actions github-actions bot added the opencl OpenCL adapter specific issues label Nov 22, 2024
@kswiecicki
Copy link
Contributor Author

@oneapi-src/unified-runtime-native-cpu-write, @oneapi-src/unified-runtime-opencl-write, @oneapi-src/unified-runtime-hip-write, @oneapi-src/unified-runtime-cuda-write, @oneapi-src/unified-runtime-level-zero-write.
Changes in the respective adapters are minimal, containing only the placeholder function definitions that return NOT_SUPPORTED and serve as stubs for future implementation.

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.

LGTM, just a couple of typos

UR command enqueues without forcing synchronization points in the asynchronous
command DAG associated with a queue. This can allow applications to compose
memory allocation and command execution asynchronously, which can improve
performancet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
performancet.
performance.

name: hQueue
desc: "[in] handle of the queue object"
- type: $x_usm_pool_handle_t
desc: "[in][optional] handle of the USM memory pooliptor"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
desc: "[in][optional] handle of the USM memory pooliptor"
desc: "[in][optional] handle of the USM memory pool"

- type: $x_queue_handle_t
name: hQueue
desc: "[in] handle of the queue object"
- type: $x_usm_pool_handle_t
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure, are we using the same pool handle as for regular allocations, instead of introducing a new pool type? I don't remember what was decided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Changes or additions to common utilities cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification 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 specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants