-
Notifications
You must be signed in to change notification settings - Fork 813
[SYCL][DOC] Queue priority extension document#21284
[SYCL][DOC] Queue priority extension document #21284slawekptak wants to merge 14 commits intointel:sycl from
Conversation
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>
There was a problem hiding this comment.
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{}};
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
againull
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?
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_priority.asciidoc
Outdated
Show resolved
Hide resolved
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.
...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>
Uh oh!
There was an error while loading. Please reload this page.
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