Skip to content

[SYCL][DOC] Initial commit for queue priority extension#7228

Closed
jbrodman wants to merge 4 commits intointel:syclfrom
jbrodman:ext_queue_priority
Closed

[SYCL][DOC] Initial commit for queue priority extension#7228
jbrodman wants to merge 4 commits intointel:syclfrom
jbrodman:ext_queue_priority

Conversation

@jbrodman
Copy link
Contributor

Signed-off-by: James Brodman james.brodman@intel.com

Signed-off-by: James Brodman <james.brodman@intel.com>
@jbrodman jbrodman requested a review from a team as a code owner October 28, 2022 19:42
jbrodman and others added 3 commits February 28, 2023 14:42
…ciidoc

Co-authored-by: John Pennycook <john.pennycook@intel.com>
Signed-off-by: James Brodman <james.brodman@intel.com>
Signed-off-by: James Brodman <james.brodman@intel.com>

| `sycl::ext::oneapi::info::queue::priority_range`
| Returns a `std::array<int,2>` that represents the `{leastPriority,greatestPriority}`
for this queue. Lower numbers imply greater priorities. The default priority has value `0`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Lower numbers imply greater priorities

I think this will be confusing. Can we flip this, so that higher numbers mean higher priority? This would match the Posix sched_get_priority_max / sched_get_priority_min APIs, and it would match the Windows SetThreadPriority API. In all of those, higher numbers mean higher priority,

| `sycl::ext::oneapi::property::queue::priority(int priority=0)`
| Specifies that the queue should be constructed with the specified `priority`.
Commands executed by this queue may take priority over commands with lower priority
on this queue's device. The default priority has value `0`. If the user specifies
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should not have a default value. Users should be required to specify a priority. There is no reason to use the priority property if you want to get default behavior If you want default behavior, you don't need the property at all. Code that uses property wants some non-default behavior, so we should require that code to specify which priority they want.

Otherwise, I can imagine someone writing code like this, thinking that they created a queue with high priority:

sycl::queue q{priority{}};

== Notice

[%hardbreaks]
Copyright (C) 2022-2023 Intel Corporation. All rights reserved.
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
Copyright (C) 2022-2023 Intel Corporation. All rights reserved.
Copyright (C) 2022 Intel Corporation. All rights reserved.

We just use the first date of publication now.


== Dependencies

This extension is written against the SYCL 2020 revision 6 specification. All
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
This extension is written against the SYCL 2020 revision 6 specification. All
This extension is written against the SYCL 2020 revision 8 specification. All

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2024

This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days.

@github-actions github-actions bot added the Stale label Dec 8, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

This pull request was closed because it has been stalled for 30 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments