Skip to content
This repository was archived by the owner on Mar 2, 2026. It is now read-only.

[SYCL][Graphs] Change queue state query#162

Merged
EwanC merged 1 commit intosycl-graph-updatefrom
ewan/queue_state_query
May 16, 2023
Merged

[SYCL][Graphs] Change queue state query#162
EwanC merged 1 commit intosycl-graph-updatefrom
ewan/queue_state_query

Conversation

@EwanC
Copy link
Collaborator

@EwanC EwanC commented May 8, 2023

The current provided mechanism for querying the state of a queue is to use get_info<info::queue::state>. However, I think this has a couple of drawbacks:

  1. The info::queue::state value which this extension defines isn't in an experimental namespace. Although we could define it with an extra experimental namespace, e.g sycl::queue::ext_oneapi_graph::state

  2. The get_info queries tend to map to underlying backend properties, in the implementation, see https://github.com/intel/llvm/tree/sycl/sycl/include/sycl/info. We are not defining a query for something that's backend specific, but backend agnostic, so get_info I don't think is the right API.

Instead, I've changed the query to add a new queue::ext_oneapi_get_state() query. This has an experimental prefix, so won't conflict with any future core functionality, and can be easily implemented without fighting against the current get_info() mechanism.

The current provided mechanism for querying the state of a queue is
to use `get_info<info::queue::state>`. However, I think this
has a couple of drawbacks:

1) The `info::queue::state` value which this extension defines isn't in an experimental
   namespace.

2) The `get_info` queries tend to map to underlying backend properties,
   in the implementation, see
   https://github.com/intel/llvm/tree/sycl/sycl/include/sycl/info . We
   are not defining a query for something that's backend specific, but
   backend agnostic, so `get_info` might not be the right API.

Instead, I've changed the query to add a new
`queue::ext_oneapi_get_state()` query. This has an experimental prefix,
so won't conflict with any future core functionality, and can be easily
implemented without fighting against the current `get_info()`
mechanism.
@EwanC EwanC added the Graph Specification Extension Specification related label May 8, 2023
@EwanC EwanC requested review from Bensuo, julianmi and reble May 8, 2023 09:34
@EwanC EwanC marked this pull request as ready for review May 9, 2023 12:05
Copy link
Collaborator

@Bensuo Bensuo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@julianmi julianmi left a comment

Choose a reason for hiding this comment

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

LGTM

@EwanC EwanC merged commit 70cf19a into sycl-graph-update May 16, 2023
EwanC pushed a commit that referenced this pull request May 16, 2023
Implements query for queue state from #162

Closes #93
EwanC pushed a commit that referenced this pull request May 16, 2023
Implements query for queue state from #162

Adds symbol for queue query entry-point and update test to use test-e2e.

Closes #93
EwanC pushed a commit that referenced this pull request May 16, 2023
Implements query for queue state from #162

Adds symbol for queue query entry-point and update test to use test-e2e.

Closes #93
@Bensuo Bensuo deleted the ewan/queue_state_query branch August 7, 2023 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Graph Specification Extension Specification related Spec Revision 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants