-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL][Docs] Add proposed event mode extension #15704
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
Merged
steffenlarsen
merged 8 commits into
intel:sycl
from
steffenlarsen:steffen/low_pow_ev_ext
Oct 31, 2024
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9110431
[SYCL][Docs] Add proposed low-power event extension
steffenlarsen 7c45672
Change to properties on submit
steffenlarsen 19cdc17
Update sycl/doc/extensions/proposed/sycl_ext_intel_low_power_event.as…
steffenlarsen 276cc80
Move properties argument and fix editorial issues
steffenlarsen 56129d1
Fix new overloads
steffenlarsen f96675e
Switch to event-mode property
steffenlarsen 7cf93b4
Update sycl_ext_intel_event_mode.asciidoc
steffenlarsen 197d433
Switch to event_mode_enum
steffenlarsen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
165 changes: 165 additions & 0 deletions
165
sycl/doc/extensions/proposed/sycl_ext_intel_event_mode.asciidoc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| = sycl_ext_intel_event_mode | ||
|
|
||
| :source-highlighter: coderay | ||
| :coderay-linenums-mode: table | ||
|
|
||
| // This section needs to be after the document title. | ||
| :doctype: book | ||
| :toc2: | ||
| :toc: left | ||
| :encoding: utf-8 | ||
| :lang: en | ||
| :dpcpp: pass:[DPC++] | ||
| :endnote: —{nbsp}end{nbsp}note | ||
|
|
||
| // Set the default source code type in this document to C++, | ||
| // for syntax highlighting purposes. This is needed because | ||
| // docbook uses c++ and html5 uses cpp. | ||
| :language: {basebackend@docbook:c++:cpp} | ||
|
|
||
| :common_ref_sem: https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:reference-semantics | ||
|
|
||
| == Notice | ||
|
|
||
| [%hardbreaks] | ||
| Copyright (C) 2024 Intel Corporation. All rights reserved. | ||
|
|
||
| Khronos(R) is a registered trademark and SYCL(TM) and SPIR(TM) are trademarks | ||
| of The Khronos Group Inc. OpenCL(TM) is a trademark of Apple Inc. used by | ||
| permission by Khronos. | ||
|
|
||
|
|
||
| == Contact | ||
|
|
||
| To report problems with this extension, please open a new issue at: | ||
|
|
||
| https://github.com/intel/llvm/issues | ||
|
|
||
|
|
||
| == Dependencies | ||
|
|
||
| This extension is written against the SYCL 2020 revision 9 specification. All | ||
| references below to the "core SYCL specification" or to section numbers in the | ||
| SYCL specification refer to that revision. | ||
|
|
||
| This extension also depends on the following other SYCL extensions: | ||
|
|
||
| * link:../experimental/sycl_ext_oneapi_enqueue_functions.asciidoc[ | ||
| sycl_ext_oneapi_enqueue_functions] | ||
| * link:../experimental/sycl_ext_oneapi_properties.asciidoc[ | ||
| sycl_ext_oneapi_properties] | ||
|
|
||
|
|
||
| == Status | ||
|
|
||
| This is a proposed extension specification, intended to gather community | ||
| feedback. Interfaces defined in this specification may not be implemented yet | ||
| or may be in a preliminary state. The specification itself may also change in | ||
| incompatible ways before it is finalized. *Shipping software products should | ||
| not rely on APIs defined in this specification.* | ||
|
|
||
|
|
||
| == Overview | ||
|
|
||
| On some backends, calling `wait()` on an `event` will synchronize using a | ||
| busy-waiting implementation. Though this comes at a low latency for the | ||
| synchronization of the event, it has the downside of consuming high amounts of | ||
| CPU time for no meaningful work. This extension introduces a new property for | ||
| SYCL commands that allow users to pick modes for the associated events, one of | ||
| these modes being a "low-power" event. These new low-power events will, if | ||
| possible, yield the thread that the `wait()` member function is called on and | ||
| only wake up occasionally to check if the event has finished. This reduces the | ||
| time the CPU spends checking finish condition of the wait, at the cost of | ||
| latency. | ||
|
|
||
|
|
||
| == Specification | ||
|
|
||
| === Feature test macro | ||
|
|
||
| This extension provides a feature-test macro as described in the core SYCL | ||
| specification. An implementation supporting this extension must predefine the | ||
| macro `SYCL_EXT_INTEL_EVENT_MODE` to one of the values defined in the table | ||
| below. Applications can test for the existence of this macro to determine if | ||
| the implementation supports this feature, or applications can test the macro's | ||
| value to determine which of the extension's features the implementation | ||
| supports. | ||
|
|
||
| [%header,cols="1,5"] | ||
| |=== | ||
| |Value | ||
| |Description | ||
|
|
||
| |1 | ||
| |The APIs of this experimental extension are not versioned, so the | ||
| feature-test macro always has this value. | ||
| |=== | ||
|
|
||
|
|
||
| === Event mode property | ||
|
|
||
| This extension adds a new property `event_mode` which can be used with the | ||
| `submit_with_event` free function from | ||
| link:../experimental/sycl_ext_oneapi_enqueue_functions.asciidoc[sycl_ext_oneapi_enqueue_functions], | ||
| allowing the user some control over how the resulting event is created and | ||
| managed. | ||
|
|
||
| ``` | ||
| namespace sycl::ext::intel::experimental { | ||
|
|
||
| enum class event_modes { none, low_power }; | ||
|
|
||
| struct event_mode { | ||
| event_mode(event_modes mode); | ||
|
|
||
| event_modes value; | ||
| }; | ||
|
|
||
| using event_mode_key = event_mode; | ||
|
|
||
| } // namespace sycl::ext::intel::experimental | ||
| ``` | ||
|
|
||
|
|
||
| === Low power event mode | ||
|
|
||
| Passing the `event_mode` property with `event_modes::low_power` to | ||
| `submit_with_event` will act as a hint to the `event` created from the | ||
| corresponding commands to do low-power synchronization. If the backend is able | ||
| to handle low-power events, calling `event::wait()` or `event::wait_and_throw()` | ||
| will cause the thread to yield and only do occasional wake-ups to check the | ||
| event progress. | ||
|
|
||
| [_Note:_ The low-power event mode currently only has an effect on `barrier` and | ||
| `partial_barrier` commands enqueued on queues that return | ||
| `backend::ext_oneapi_level_zero` from `queue::get_backend()`. | ||
| _{endnote}_] | ||
|
|
||
|
|
||
| === New property usage example | ||
|
|
||
| As an example of how to use the new `low_power_event` property, see the | ||
steffenlarsen marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| following code: | ||
|
|
||
| ``` | ||
| #include <sycl/sycl.hpp> | ||
|
|
||
| namespace oneapiex = sycl::ext::oneapi::experimental; | ||
| namespace intelex = sycl::ext::intel::experimental; | ||
|
|
||
| int main() { | ||
| sycl::queue Q; | ||
|
|
||
| // Submit some work to the queue. | ||
| oneapiex::submit(Q, [&](sycl::handler &CGH) {...}); | ||
|
|
||
| // Submit a command with the low-power event property. | ||
| oneapiex::properties Props{intelex::low_power_event}; | ||
| sycl::event E = oneapiex::submit_with_event(Q, Props, [&](sycl::handler &CGH) { | ||
| ... | ||
| }); | ||
|
|
||
| // Waiting for the resulting event will use low-power waiting if possible. | ||
| E.wait(); | ||
| } | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@Pennycook: Do you like this naming convention where the enum name is plural and the property name is singular?
The other alternative we discussed is to name the property
event_modeand name the enumevent_mode_enum.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.
A quick search of the ISO C++ specification suggests that their convention is to use a plural when the enum is a bitmask (e.g.,
std::filesystem::copy_options) but a singular when it is not (e.g.,std::filesystem::file_type).If we want to keep the plural here, then I think we should define this as a bitmask. But that also suggests that the property name should also be a plural. Didn't we recently discuss reintroducing a nested namespace for properties? That would allow
sycl::event_modesto be a bitmask of event modes, andsycl::property::event_modesto be a property for setting an event's modes.I don't really like the idea of encoding
_enumin a name, because of the redundancy there.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 don't think we should make this a bitmask.
We have already used the
_enumsuffix for some other properties, so we have a precedent.If we did add the namespace, why wouldn't we put the property and the enum in the same namespace? The enum is only used in conjunction with the property.
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.
@steffenlarsen, I saw that you re-requested my review. I think the naming issue in this thread is not yet resolved.
If we can't think of a better solution, I suggest we go with the name
event_mode_enum, which is consistent with existing properties that take an enum.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.
Ah, apologies. I glanced over this one.
event_mode_enumis fine with me. 👍