Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SYCL] Provide extension to query for unsupported platforms #15368

Closed
wants to merge 2 commits into from
Closed

[SYCL] Provide extension to query for unsupported platforms #15368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34b4aa6
[SYCL] Provide extension to query for unsupported platforms
jchlanda Sep 12, 2024
413dd39
Remove mentions of host platform
jchlanda Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions .../doc/extensions/experimental/sycl_ext_oneapi_get_unsupported_platforms.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
= sycl_ext_oneapi_get_unsupported_platforms
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 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'm also curious if there is user request for this feature.

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).

Copy link
Contributor

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.

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 why sycl-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. Should sycl-ls list all OpenMP installations as "banned"?

Copy link
Contributor Author

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.

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).

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. It's also not clear to me how we would decide which "banned" things to list here.

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.

However, it's not clear to me why sycl-ls should show this information.

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:

Unsupported Platforms: 1
Platform [#1]:
    Version  : OpenCL 3.0 CUDA 12.5.85
    Name     : NVIDIA CUDA
    Vendor   : NVIDIA Corporation
    Devices  : 1
        Device [#0]:
        Type              : gpu
        Version           : OpenCL 3.0 CUDA
        Name              : NVIDIA GeForce GTX 1650
        Vendor            : NVIDIA Corporation
        Driver            : 555.58.02
        UUID              : 2073250284612179411136216771208211227
        Num SubDevices    : 0
        Num SubSubDevices : 0

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.

Copy link
Contributor

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 remove get_unsupported_platforms that was added in #15166.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done in: #15415


: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. This extension adds an API entry to
list 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.

|====
--
2 changes: 1 addition & 1 deletion sycl/include/sycl/platform.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class __SYCL_EXPORT platform : public detail::OwnerLessBase<platform> {
/// Returns all unsupported (non-SYCL) platforms in the system.
///
/// \return a vector of all unsupported non-SYCL platforms.
static std::vector<platform> get_unsupported_platforms();
static std::vector<platform> ext_oneapi_get_unsupported_platforms();

/// Returns the backend associated with this platform.
///
Expand Down
6 changes: 4 additions & 2 deletions sycl/source/detail/platform_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,10 @@ static bool IsBannedPlatform(platform Platform) {
IsMatchingOpenCL(Platform, "AMD Accelerated Parallel Processing");
}

// Get the vector of platforms supported by a given UR plugin
// replace uses of this with a helper in plugin object, the plugin
// When `Supported` is `true` gets the vector of platforms supported by a given
// UR `Plugin`. Otherwise a vector of all unsupported (non-SYCL, or
// device-less) platforms.
// Replace uses of this with a helper in plugin object, the plugin
// objects will own the ur adapter handles and they'll need to pass them to
// urPlatformsGet - so urPlatformsGet will need to be wrapped with a helper
std::vector<platform> platform_impl::getPluginPlatforms(PluginPtr &Plugin,
Expand Down
5 changes: 2 additions & 3 deletions sycl/source/detail/platform_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ class platform_impl {

/// Returns all available SYCL platforms in the system.
///
/// By default the resulting vector always contains a single SYCL host
/// platform instance. There are means to override this behavior for testing
/// purposes. See environment variables guide for up-to-date instructions.
/// There are means to override this behavior for testing purposes. See
/// environment variables guide for up-to-date instructions.
///
/// \return a vector of all available SYCL platforms.
static std::vector<platform> get_platforms();
Expand Down
1 change: 1 addition & 0 deletions sycl/source/feature_test.hpp.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ inline namespace _V1 {
#define SYCL_EXT_ONEAPI_RAW_KERNEL_ARG 1
#define SYCL_EXT_ONEAPI_PROFILING_TAG 1
#define SYCL_EXT_ONEAPI_ENQUEUE_NATIVE_COMMAND 1
#define SYCL_EXT_ONEAPI_GET_UNSUPPORTED_PLATFORMS 1
// In progress yet
#define SYCL_EXT_ONEAPI_ATOMIC16 0

Expand Down
2 changes: 1 addition & 1 deletion sycl/source/platform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ std::vector<platform> platform::get_platforms() {
return detail::platform_impl::get_platforms();
}

std::vector<platform> platform::get_unsupported_platforms() {
std::vector<platform> platform::ext_oneapi_get_unsupported_platforms() {
return detail::platform_impl::get_unsupported_platforms();
}

Expand Down
2 changes: 1 addition & 1 deletion sycl/test/abi/sycl_symbols_linux.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3545,7 +3545,7 @@ _ZN4sycl3_V17samplerC1EP11_cl_samplerRKNS0_7contextE
_ZN4sycl3_V17samplerC2ENS0_29coordinate_normalization_modeENS0_15addressing_modeENS0_14filtering_modeERKNS0_13property_listE
_ZN4sycl3_V17samplerC2EP11_cl_samplerRKNS0_7contextE
_ZN4sycl3_V18platform13get_platformsEv
_ZN4sycl3_V18platform25get_unsupported_platformsEv
_ZN4sycl3_V18platform36ext_oneapi_get_unsupported_platformsEv
_ZN4sycl3_V18platformC1EP15_cl_platform_id
_ZN4sycl3_V18platformC1ERKNS0_15device_selectorE
_ZN4sycl3_V18platformC1ERKNS0_6deviceE
Expand Down
2 changes: 1 addition & 1 deletion sycl/test/abi/sycl_symbols_windows.dump
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4035,7 +4035,7 @@
?get_platform@context@_V1@sycl@@QEBA?AVplatform@23@XZ
?get_platform@device@_V1@sycl@@QEBA?AVplatform@23@XZ
?get_platforms@platform@_V1@sycl@@SA?AV?$vector@Vplatform@_V1@sycl@@V?$allocator@Vplatform@_V1@sycl@@@std@@@std@@XZ
?get_unsupported_platforms@platform@_V1@sycl@@SA?AV?$vector@Vplatform@_V1@sycl@@V?$allocator@Vplatform@_V1@sycl@@@std@@@std@@XZ
?ext_oneapi_get_unsupported_platforms@platform@_V1@sycl@@SA?AV?$vector@Vplatform@_V1@sycl@@V?$allocator@Vplatform@_V1@sycl@@@std@@@std@@XZ
?get_pointer_device@_V1@sycl@@YA?AVdevice@12@PEBXAEBVcontext@12@@Z
?get_pointer_type@_V1@sycl@@YA?AW4alloc@usm@12@PEBXAEBVcontext@12@@Z
?get_precision@stream@_V1@sycl@@QEBA_KXZ
Expand Down
3 changes: 2 additions & 1 deletion sycl/tools/sycl-ls/sycl-ls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ int main(int argc, char **argv) {
std::cout << "\nPlatforms: " << Platforms.size() << std::endl;
printVerbosePlatformInfo(Platforms, DeviceNums, SuppressNumberPrinting);

const auto &UnsupportedPlatforms = platform::get_unsupported_platforms();
const auto &UnsupportedPlatforms =
platform::ext_oneapi_get_unsupported_platforms();
std::cout << "\nUnsupported Platforms: " << UnsupportedPlatforms.size()
<< std::endl;
printVerbosePlatformInfo(UnsupportedPlatforms, DeviceNums,
Expand Down
Loading