Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[SYCL][DOC] Queue priority extension document#21284

Open
slawekptak wants to merge 14 commits intointel:sycl from
slawekptak:ext_queue_priority
Open

[SYCL][DOC] Queue priority extension document #21284
slawekptak wants to merge 14 commits intointel:sycl from
slawekptak:ext_queue_priority

Conversation

@slawekptak
Copy link
Contributor

@slawekptak slawekptak commented Feb 13, 2026
edited
Loading

Added a document describing the queue priority extension. The extension allows to specify a priority as a number when the queue is created, and allows to query the supported priority range.

The original PR: #7228

James Brodman and others added 5 commits October 28, 2022 15:42
Signed-off-by: James Brodman <james.brodman@intel.com>
...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>
Copy link
Contributor Author

@slawekptak slawekptak Feb 13, 2026

Choose a reason for hiding this comment

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

The original comment from @gmlueck :
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{}};

Copy link
Contributor Author

@slawekptak slawekptak Feb 13, 2026

Choose a reason for hiding this comment

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

This behavior is consistent with L0, but I think it makes sense to just not specify the property if default is requested, so I think we might change this on the SYCL level.

Copy link
Contributor Author

@slawekptak slawekptak Feb 13, 2026

Choose a reason for hiding this comment

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

Original comment from @gmlueck :

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,

Copy link
Contributor Author

@slawekptak slawekptak Feb 13, 2026

Choose a reason for hiding this comment

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

This is also consistent with the L0 APIs - and I'm not sure if changing it on the SYCL level wouldn't create confusion..

Copy link
Contributor

Choose a reason for hiding this comment

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

Which way is it in Level Zero? Does a numerically smaller number imply lower priority or higher priority?

Copy link
Contributor Author

@slawekptak slawekptak Feb 16, 2026

Choose a reason for hiding this comment

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

Numerically smaller number imply higher priority.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, let's keep it that way in the SYCL spec too. CUDA has it this way also.

@slawekptak slawekptak marked this pull request as ready for review February 13, 2026 14:58
@slawekptak slawekptak requested a review from a team as a code owner February 13, 2026 14:58
@slawekptak slawekptak marked this pull request as draft February 13, 2026 15:27
Copy link
Contributor

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 we should explain the priority values in this section:

  • Explain that numerically lower values represent higher priorities.
  • Explain that the value 0 represents a queue with default priority. (I assume this is the case?)

Copy link
Contributor Author

@slawekptak slawekptak Feb 16, 2026

Choose a reason for hiding this comment

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

In terms of the default priority - L0 defines a default value, but as you've mentioned in the other comment, maybe we don't need it in SYCL? Can we just say, that if default is requested, don't define this property at all?

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 people need to know what the numerical priority is for a queue that is created without this property. Otherwise, you can't tell whether a queue using property::queue::priority has higher or lower priority.

Copy link
Contributor

gmlueck commented Feb 13, 2026

Is this extension going to replace https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/supported/sycl_ext_oneapi_queue_priority.asciidoc?

Yes, I think we will want to deprecate that in favor of this new one.

slawekptak and others added 4 commits February 16, 2026 09:59
...ciidoc
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
@slawekptak slawekptak marked this pull request as ready for review February 16, 2026 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmlueck gmlueck gmlueck left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Comments

AltStyle によって変換されたページ (->オリジナル) /