-
Notifications
You must be signed in to change notification settings - Fork 738
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
[SYCL] Provide extension to query for unsupported platforms #15368
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
= sycl_ext_oneapi_get_unsupported_platforms | ||
|
||
: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++] | ||
|
||
// 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} | ||
|
||
|
||
== 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 8 specification. All | ||
references below to the "core SYCL specification" or to section numbers in the | ||
SYCL specification refer to that revision. | ||
|
||
|
||
== Status | ||
|
||
This is an experimental extension specification, intended to provide early | ||
access to features and gather community feedback. Interfaces defined in this | ||
specification are implemented in {dpcpp}, but they are not finalized and may | ||
change incompatibly in future versions of {dpcpp} without prior notice. | ||
*Shipping software products should not rely on APIs defined in this | ||
specification.* | ||
|
||
|
||
== Overview | ||
|
||
The implementation of `sycl::platform` already allows querying for supported | ||
platforms - `static std::vector<platform> get_platforms()` - which returns all | ||
available SYCL platforms in the system (the resulting vector always contains a | ||
single SYCL host platform instance). This extension adds an API entry to list | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: Is "host" a supported platform? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was, that's a comment that I've lifted from source code without thinking too much about it. I'll rephrase it and remove the original one as well. Done in 413dd39 |
||
all unsupported platforms; the platform is considered unsupported if it is | ||
non-SYCL, or does not have any devices associated. | ||
steffenlarsen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
This extension exists to solve a specific problem: listing of all platforms | ||
(supported and not) in verbose mode of `sycl-ls` binary. | ||
|
||
|
||
== 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_ONEAPI_GET_UNSUPPORTED_PLATFORMS` 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 API of this experimental extension is not versioned, so the feature-test | ||
macro always has this value. | ||
|=== | ||
|
||
=== New SYCL platform API | ||
|
||
This extension adds the following new API to the existing `sycl::platform` class: | ||
|
||
[source, c++] | ||
---- | ||
namespace sycl { | ||
|
||
class platform { | ||
... | ||
|
||
static std::vector<platform> ext_oneapi_get_unsupported_platforms(); | ||
|
||
... | ||
} | ||
|
||
} // namespace sycl | ||
---- | ||
|
||
The new API has the following behaviour: | ||
|
||
-- | ||
[options="header"] | ||
|==== | ||
| Function Definition | Description | ||
a| | ||
[source, c++] | ||
---- | ||
static std::vector<platform> ext_oneapi_get_unsupported_platforms(); | ||
---- | ||
| Returns a vector containing all unsupported (non-SYCL, or device-less) | ||
platforms in the system. | ||
|
||
|==== | ||
-- |
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.
Adding a general comment here so it can be threaded.
I think the goal is to allow
sycl-ls
to list platforms that could potentially be supported by DPC++ even if those platforms are not actually available on the current system. Is that right? How would we decide what platforms are potentially supported? Will there be a hard-coded list someplace? For example, will we list CUDA as a potential platform even if there is no Nvidia card installed on the system?I'm also curious if there is user request for this feature.
If we do want to support this feature, I think this extension is not the right way. The Khronos WG discussed this a while ago and agreed to a different direction. Some WG members wanted to expose "potential" platforms from their implementations, so the WG clarified the SYCL specification to allow a
platform
object to have no devices. An implementation can use this to expose platforms that could potentially be supported, even if there is no support in the current installation.Intel was weakly opposed because we did not see a reason to do this. However, we did not feel strongly because we did not expect DPC++ to make use of this feature. There is no requirement in the spec to expose empty platforms like this, so we did not expect DPC++ to do so.
If DPC++ does want to expose "potential" platforms, it should do so by exposing empty
platform
objects rather than adding a new oneAPI 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.
I guess I owe a bit of an explanation here; the goal of this API is not to introduce logic to decide which platforms to mark as unsupported, this is already handled in
IsBannedPlatform
helper, the sole purpose of the new entry is to provide a way of communicating which platforms have been banned.I suppose it's a quality of life kind of a thing more than a strict user requirement. FWIW it came my way through a discussion here.
I'm not sure if the ability to list banned platforms is worth going through the motions of WG and discussions with other vendors, I'm OK with closing this PR (and removing the entry that required this extension in the first place).
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 comment there indicates that the UR intentionally does not expose certain OpenCL platforms because they don't work with DPC++. Does this feature expose them anyway via a new API? That does not make sense to me.
If there is no user request for this feature, I think we should not add any SYCL API that exposes platforms for OpenCL installations that are known to not work. If this means they are not displayed in
sycl-ls
, I think that is OK.If we really want to list these in
sycl-ls
(or some other tool), I'd rather expose some API via the UR and call that API directly. However, it's not clear to me whysycl-ls
should show this information. It's also not clear to me how we would decide which "banned" things to list here. For example, some implementations of SYCL layer on top of OpenMP as a backend. Shouldsycl-ls
list all OpenMP installations as "banned"?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 never though about the "exposing" aspect of this API; if the question is: can this API provide us with an instance of
sycl::platform
that otherwise would not be accessible and is not valid, than the answer is yes. I am not in a position to judge if this is acceptable or not, fwiw, the entry clearly states that the returned platforms are unsupported (perhaps the wording in the doc could be stronger).I'm not sure if moving this functionality to UR would work, the logic of which platforms are marked as banned is currently handled by the
platform
(IsBannedPlatform) and that seems to be well placed.I could imagine it being useful from the infrastructure perspective, seeing what the system has installed, for instance, on my machine, along 4 supported platforms, I have an unsupported
OpenCL 3.0
from cuda:As this has always been a developer request, I'm happy to follow your advice and close it, if adding an extension is not suitable.
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 like the idea of adding an API that returns a
platform
object that is not supported. Since there is no user request for this feature, let's close this PR and also removeget_unsupported_platforms
that was added in #15166.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.
Sure, done in: #15415