From 4313828e7fc87f1cb66147ae357913d893e7b493 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Sat, 13 Apr 2024 08:11:54 -0500 Subject: [PATCH 1/7] Proposal to remove sycl chapter --- README.rst | 1 + proposals/001-removing-sycl.rst | 84 +++++++++++++++++++++++++++++++++ proposals/index.rst | 10 ++++ 3 files changed, 95 insertions(+) create mode 100644 proposals/001-removing-sycl.rst create mode 100644 proposals/index.rst diff --git a/README.rst b/README.rst index 0728cba..d199e45 100644 --- a/README.rst +++ b/README.rst @@ -7,3 +7,4 @@ .. _`Meeting Notes`: meetings/README.rst .. _`RFCs`: RFC/README.rst + diff --git a/proposals/001-removing-sycl.rst b/proposals/001-removing-sycl.rst new file mode 100644 index 0000000..b527eb8 --- /dev/null +++ b/proposals/001-removing-sycl.rst @@ -0,0 +1,84 @@ +========================== +001: Removing SYCL Chapter +========================== + +Remove the SYCL chapter from the oneAPI specification. + +See also: + +* `WG discussion`_ + +.. _`WG discussion`: https://github.com/uxlfoundation/spec-working-group/blob/main/meetings/notes.rst#notes + +Background +========== + +The SYCL chapter no longer serves a useful purpose and is causing some confusion +about UXL's role in SYCL. The original motivation for the chapter was that DPC++ +was considered part of oneapi specification, and the chapter specifies the API +that DPC++ supports, which is SYCL 2020 + extensions. With the adoption of SYCL +2020, the need for extensions is reduced. DPC++ is not part of UXL. The hope is +that the SYCL support in DPC++ will be upstreamed in LLVM, and that would be the +home for specification of extensions, not the oneAPI specification. + +After the initial WG discussion, I followed up with Alexey & Greg. + +There are several issues: + +1. Policy/Documentation about use of SYCL vendor extensions in interfaces that + are part of oneapi spec +2. Policy/Documentation about use of SYCL vendor extensions in UXL open source + projects. +3. Documentation of use of ``std::`` from SYCL kernels + +Use of SYCL vendor extensions in interfaces +=========================================== + +I searched the header files by cloning all the UXL projects and the sources of +the spec:: + + grep -n -r --include=\*.{h,hpp,rst} -E 'sycl::ext|ext_intel|ext_oneapi' . + +After review, I don't believe that extensions are currently part of the API. The +closest to that is when a type defined in sycl::ext is made available in a +library namespace:: + + namespace oneapi { + namespace mkl { + + #ifndef __HIPSYCL__ + using bfloat16 = sycl::ext::oneapi::bfloat16; + #endif + + +I believe this was done to make ``oneapi::mkl::bfloat16`` available. + +Recommendation + Oneapi spec components should not use vendor extensions in interfaces. Either + wrap the type (example above) or work with SYCL WG to make it part of SYCL + spec or as a KHR extension. If there is a compelling reason to use a vendor + extension, then review it with the spec WG as a candiate for a KHR extension. + +Use of SYCL vendor extensions in implementations +================================================ + +Implementations need to use vendor extensions for best performance and +functionality. It would be useful if a project had a reference implementation +designed for maximum portability. This would be a good topic for the open source +WG and individual projects. + +Recommendation + Discuss reference implementations in the open source WG. + +Documentation of use of ``std::`` from SYCL kernels +=================================================== + +SYCL specification does not support the use of ``std::`` in kernels and does not +provide replacements. oneDPL wants to support standard C++ where ``std::`` would +be used in code called from a kernel. DPL documents a list of ``std::`` +functions that are supported. It would be better if the SYCL specification or +SYCL implementation documentation defined what can be used. This documentation +would be useful for other projects. It has been discussed in the SYCL WG. + +Recommendation + Raise it as a topic in the SYCL WG and DPC++ project. diff --git a/proposals/index.rst b/proposals/index.rst new file mode 100644 index 0000000..dca32bc --- /dev/null +++ b/proposals/index.rst @@ -0,0 +1,10 @@ +========= +Proposals +========= + +`001 Removing SYCL Chapter`_ + +.. _`001 Removing SYCL Chapter`: 001-removing-sycl-chapter.rst + + + From 1511ac26cf9dc9f6892b6934e247628c627b14ee Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Sun, 14 Apr 2024 08:09:20 -0500 Subject: [PATCH 2/7] Clarify recommendation on use of extensions --- proposals/001-removing-sycl.rst | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/proposals/001-removing-sycl.rst b/proposals/001-removing-sycl.rst index b527eb8..221dd93 100644 --- a/proposals/001-removing-sycl.rst +++ b/proposals/001-removing-sycl.rst @@ -54,10 +54,17 @@ library namespace:: I believe this was done to make ``oneapi::mkl::bfloat16`` available. Recommendation - Oneapi spec components should not use vendor extensions in interfaces. Either - wrap the type (example above) or work with SYCL WG to make it part of SYCL - spec or as a KHR extension. If there is a compelling reason to use a vendor - extension, then review it with the spec WG as a candiate for a KHR extension. + Oneapi spec components should not use vendor extensions in interfaces. Some + alternatives are: + + * Wrap the type (example above) + * Work with SYCL WG to make it part of the specification or a KHR extension. + KHR extensions are Khronos extensions which have been approved in the SYCL + WG and are more likely to be provided in more than one SYCL implementation. + + If using a vendor extension is the only way to achieve the desired behavior, + then it should be documented in the specification. + Use of SYCL vendor extensions in implementations ================================================ From 2fee58723eeff58d29da20d8fa5b11ad94feb915 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Sun, 14 Apr 2024 08:32:34 -0500 Subject: [PATCH 3/7] More rewrites --- proposals/001-removing-sycl.rst | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/proposals/001-removing-sycl.rst b/proposals/001-removing-sycl.rst index 221dd93..f31d874 100644 --- a/proposals/001-removing-sycl.rst +++ b/proposals/001-removing-sycl.rst @@ -69,23 +69,26 @@ Recommendation Use of SYCL vendor extensions in implementations ================================================ -Implementations need to use vendor extensions for best performance and -functionality. It would be useful if a project had a reference implementation -designed for maximum portability. This would be a good topic for the open source -WG and individual projects. +We do not have reference implementations for oneAPI spec components, so this is +relevant for the open source projects. Vendor extensions limit portability, but +they are needed for best performance and functionality. The effort and value of +eliminating vendor extensions depends on the project. This would be a good topic +for the open source WG and individual projects. Recommendation - Discuss reference implementations in the open source WG. + Discuss in the open source WG. Documentation of use of ``std::`` from SYCL kernels =================================================== -SYCL specification does not support the use of ``std::`` in kernels and does not -provide replacements. oneDPL wants to support standard C++ where ``std::`` would -be used in code called from a kernel. DPL documents a list of ``std::`` -functions that are supported. It would be better if the SYCL specification or -SYCL implementation documentation defined what can be used. This documentation -would be useful for other projects. It has been discussed in the SYCL WG. +The SYCL specification does not support the use of ``std::`` in kernels and does +not provide replacements. oneDPL wants to support standard C++ where ``std::`` +would be used in code called from a kernel. DPL documents a list of ``std::`` +functions that will work with DPC++. Being able to use some ``std::`` +functionality in SYCL kernels would make it easier for all SYCL programmers. +This is something that could be potentially moved from oneDPL to a common part +of the oneAPI specification if more than one group felt they needed it. Recommendation - Raise it as a topic in the SYCL WG and DPC++ project. + The chapter we are removing does not have the information. This issue can be + raised as a separate proposal. From 11798f16093f068ccabc023745f7a24e35f32823 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Mon, 15 Apr 2024 07:46:12 -0500 Subject: [PATCH 4/7] rename to RFC --- {proposals => RFCs}/001-removing-sycl.rst | 0 {proposals => RFCs}/index.rst | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {proposals => RFCs}/001-removing-sycl.rst (100%) rename {proposals => RFCs}/index.rst (100%) diff --git a/proposals/001-removing-sycl.rst b/RFCs/001-removing-sycl.rst similarity index 100% rename from proposals/001-removing-sycl.rst rename to RFCs/001-removing-sycl.rst diff --git a/proposals/index.rst b/RFCs/index.rst similarity index 100% rename from proposals/index.rst rename to RFCs/index.rst From 9bbf4efb9c27d4d91ec46c8325439da5b3c693b2 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Mon, 15 Apr 2024 07:46:36 -0500 Subject: [PATCH 5/7] Update RFCs index page --- RFCs/index.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/RFCs/index.rst b/RFCs/index.rst index dca32bc..0b33cdd 100644 --- a/RFCs/index.rst +++ b/RFCs/index.rst @@ -1,6 +1,6 @@ -========= -Proposals -========= +==== +RFCs +==== `001 Removing SYCL Chapter`_ From 0d4695db498c7ed2e741cc92ed864b9dc7a38270 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Tue, 16 Apr 2024 11:41:04 -0500 Subject: [PATCH 6/7] cleanup --- {RFCs => RFC}/001-removing-sycl.rst | 0 RFCs/index.rst | 10 ---------- 2 files changed, 10 deletions(-) rename {RFCs => RFC}/001-removing-sycl.rst (100%) delete mode 100644 RFCs/index.rst diff --git a/RFCs/001-removing-sycl.rst b/RFC/001-removing-sycl.rst similarity index 100% rename from RFCs/001-removing-sycl.rst rename to RFC/001-removing-sycl.rst diff --git a/RFCs/index.rst b/RFCs/index.rst deleted file mode 100644 index 0b33cdd..0000000 --- a/RFCs/index.rst +++ /dev/null @@ -1,10 +0,0 @@ -==== -RFCs -==== - -`001 Removing SYCL Chapter`_ - -.. _`001 Removing SYCL Chapter`: 001-removing-sycl-chapter.rst - - - From 82f9339691752bd5b8cf352fafc88c3ebc9e8be9 Mon Sep 17 00:00:00 2001 From: Robert Cohn Date: Tue, 16 Apr 2024 11:41:40 -0500 Subject: [PATCH 7/7] update --- README.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/README.rst b/README.rst index d199e45..0728cba 100644 --- a/README.rst +++ b/README.rst @@ -7,4 +7,3 @@ .. _`Meeting Notes`: meetings/README.rst .. _`RFCs`: RFC/README.rst -