-
Notifications
You must be signed in to change notification settings - Fork 124
[UR] Implement extension mechanism #458
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
Conversation
| - Extensions must have globally unique extension names reported from ${x}PlatformGetExtensionProperties | ||
| - All extensions must be defined in this specification | ||
| - Extensions must not break backwards compatibility of the core APIs | ||
| - Standard extension versions must be backwards compatible with prior versions |
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 copied this from L0 extension scheme, but perhaps we could use semantic versioning with extensions with major/minor versions?
ef696fc to
5534b9d
Compare
68764b5 to
ccb757e
Compare
| - Extensions must use globally unique names for macros, enums, structures and functions | ||
| - Extensions must have globally unique extension names reported from ${x}PlatformGetExtensionProperties | ||
| - All extensions must be defined in this specification | ||
| - Extensions must not break backwards compatibility of the core APIs |
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.
Does this also include semantic breaks as well as interface breaks in the core APIs, i.e. does this forbid an extension from altering the behavior of an existing API?
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 an extension can alter the behavour of an existing API. For example you might want to chain an additional structure to the input or use extension specific enums, but default usage should remain the same.
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.
Hmm, I'm not sure I'm so confortable with extensions that change semantics because we have no way to opt-out with this architecture. If we were able to enable extensions, like Vulkan, maybe I'd be more conformtable with that.
| - Extensions must have globally unique extension names reported from ${x}PlatformGetExtensionProperties | ||
| - All extensions must be defined in this specification | ||
| - Extensions must not break backwards compatibility of the core APIs | ||
| - Standard extension versions must be backwards compatible with prior versions |
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.
Does this mean that experimental extensions are permitted to break backwards compatibility?
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.
Yes I think that makes sense. Experimental extensions should have no ABI or backwards compatibility requirements.
| There are two types of extensions defined by this specification: | ||
|
|
||
| 1. **Standard** - extension will be included into the current and all future version of the specification. | ||
| 2. **Experimental** - extensions require additional experimentation and development, before becoming a standard extension. |
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.
Will there be implementation or coverage requirements for when an extension should move from experimental to standard?
| * Document the extension in `EXT_<ext-name>.rst` file based on the `EXT_Template.rst`, ensuring it is added to the list of | ||
| extensions below. | ||
|
|
||
| * The extension will only be accepted if all conditions are met and its addition is agreed upon by the Working Group. |
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.
Does this refer to an extension becoming a standard extension?
| * The extension will only be accepted if all conditions are met and its addition is agreed upon by the Working Group. | ||
|
|
||
|
|
||
| List of Standard Extensions |
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.
Just a thought, maybe these lists should also track the implementation status of the extensions, so whether they are implemented, and then which version of UR supports which version of the extension, particularly for the experimental extensions which may change, so high-level languages know what they can use when updating the UR interface.
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| In this section describe in detail the purpose of the extension, along with its | ||
| valid usage. |
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.
Quite like it when extensions have a version history at the bottom, could we add that to the template?
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 useful info, I do wonder if having the ability to view different extension versions side-by-side would also be useful though (other than looking at the git history)?
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.
That would be really good. @kbenzie I've seen versioned docs before on the Zephyr project link - there is a dropdown in the bottom left where you can select the version of the API you want to view. I'm not really sure how this is implemented, but perhaps we can have something like that as well as a changelog (like ewan suggested).
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.
Looking at the Zephyr page I think that's for the entire page rather than just the extension specs, my gut feeling is you'd need to have a separate Sphinx build for each extension to get something exapctly like that.
|
In the WG there was a strong response against implementing extensions in this way - especially standard extensions. Instead we will create a |
|
Closing this in favour of Experimental Features |
Rather than the user deciding whether to use the emulation mode or backend command-buffer mode, it is less error prone to do it programmatically as a user can't select an unsupported config. This also fixes #90 where currently the `SYCL_EXT_ONEAPI_GRAPH` macro isn't defined when in emulation mode. Done by using a PI device info query in finalization. This also corresponds to a new UR device info query until the extension mechanism is decided oneapi-src/unified-runtime#458 but will be superceeded by whatever extension reporting mechanism is decided on. The only PI backend which reports support for command-buffer implementation is Level Zero, the other backends/adapters report no support. The vendor test macro test-e2e test is re-enabled with this change.
Rather than the user deciding whether to use the emulation mode or backend command-buffer mode, it is less error prone to do it programmatically as a user can't select an unsupported config. This also fixes #90 where currently the `SYCL_EXT_ONEAPI_GRAPH` macro isn't defined when in emulation mode. Done by using a PI device info query in finalization. This also corresponds to a new UR device info query until the extension mechanism is decided oneapi-src/unified-runtime#458 but will be superceeded by whatever extension reporting mechanism is decided on. The only PI backend which reports support for command-buffer implementation is Level Zero, the other backends/adapters report no support. The vendor test macro test-e2e test is re-enabled with this change.
This MR:
urPlatformGetExtensionPropertiesto query which extensions are supported.