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

[SYCL][DOC] Add table with DPC++ extensions status #1619

Merged
merged 6 commits into from
May 26, 2020
Merged

[SYCL][DOC] Add table with DPC++ extensions status #1619

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3c149e
[SYCL][DOC] Added table with DPC++ extensions status
romanovvlad Apr 30, 2020
934ba64
applying comments
romanovvlad May 12, 2020
77e41b9
applying comments
romanovvlad May 18, 2020
ea0d072
applying comments
romanovvlad May 19, 2020
f390659
applying comments
romanovvlad May 21, 2020
ad84252
conflict resolution
romanovvlad May 26, 2020
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
33 changes: 32 additions & 1 deletion sycl/doc/extensions/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
# Extensions

This is where documents can be found that propose extensions to the SYCL specification.
The directory contains documents that describe DPC++ extensions to SYCL
specification.

DPC++ extensions status:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a legend with possible "Status" values?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a selfish interest to capture information about the compiler options to turn on/off these extensions.
Do you think we can add another column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A column with compiler options which turn on/off specific extensions?
What do you think about having them in https://github.com/intel/llvm/blob/sycl/sycl/doc/UsersManual.md
and add a link from this doc to the user manual?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me.

AFAIK, some extensions are "library-only" and there is no dedicated clang driver option to manage library extensions - they are managed through macro definitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are managed through macro definitions.

Can we have such macro documented in the user manual as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should document them and align extensions API with this proposal: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/ExtensionMechanism/SYCL_INTEL_extension_api.asciidoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a link to user manual, please, let me know if something else should be done in scope of this comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we please maintain more info about the extensions and capture this:

  • where it is targeted, i.e. which devices/backends are going to support it
  • current availability through various targeted backends
  • what SW components are known to be involved in the implementation (e.g. SYCL headers, SPIR-V, SYCL RT, PI,...)

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 that if we add all this information the table will be too complex, but I can add a couple of columns that can be filled in future.


Copy link
Contributor

Choose a reason for hiding this comment

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

printf extension seems to be missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

These others seem to be missing too:

  1. Specialization constants
  2. Generic interoperability between DPC++ and native RT (https://github.com/KhronosGroup/SYCL-Shared/blob/master/proposals/sycl_generalization.md)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bader , do we have an extension doc for the printf extension?
@smaslov-intel I haven't added non-Intel extensions intentionally.
@pvchupin Should we track non-Intel extensions here? What if they are not implemented so far?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bader , do we have an extension doc for the printf extension?

I've requested it here, but there were updates yet. @AlexeySachkov any updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pvchupin Should we track non-Intel extensions here? What if they are not implemented so far?

Copy link
Contributor

@pvchupin pvchupin May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this table purpose is to clarify what we support or planning to prototype in the nearest future (Intel or non-Intel doesn't matter).
In particular we have specialization constants implementation, so we should add it. We should probably add generalization too. I don't think we should extend this list to all existing drafts, only to those we are considering or prototyped already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, are you OK if these extensions are added to the table in a separate PR(preferably by the authors)?

| Extension | Status | Comment |
|-------------|:------------|:------------|
| [SYCL_INTEL_bitcast](Bitcast/SYCL_INTEL_bitcast.asciidoc) | Proposal | |
| [C and C++ Standard libraries support](C-CXX-StandardLibrary/C-CXX-StandardLibrary.rst) | Partially supported | |
| [SYCL_INTEL_data_flow_pipes](DataFlowPipes/data_flow_pipes.asciidoc) | Supported | |
romanovvlad marked this conversation as resolved.
Show resolved Hide resolved
| [SYCL_INTEL_deduction_guides](deduction_guides/SYCL_INTEL_deduction_guides.asciidoc) | Supported | |
| [SYCL_INTEL_device_specific_kernel_queries](DeviceSpecificKernelQueries/SYCL_INTEL_device_specific_kernel_queries.asciidoc) | Proposal | |
| [SYCL_INTEL_enqueue_barrier](EnqueueBarrier/enqueue_barrier.asciidoc) | Proposal | |
| [SYCL_INTEL_extended_atomics](ExtendedAtomics/SYCL_INTEL_extended_atomics.asciidoc) | Proposal | |
| [SYCL_INTEL_group_algorithms](GroupAlgorithms/SYCL_INTEL_group_algorithms.asciidoc) | Proposal | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| [SYCL_INTEL_group_algorithms](GroupAlgorithms/SYCL_INTEL_group_algorithms.asciidoc) | Proposal | |
| [SYCL_INTEL_group_algorithms](GroupAlgorithms/SYCL_INTEL_group_algorithms.asciidoc) | Partially supported | Does not support host device |

Copy link
Contributor Author

@romanovvlad romanovvlad May 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Applied.

| [SYCL_INTEL_group_mask](./GroupMask/SYCL_INTEL_group_mask.asciidoc) | Proposal | |
| [FPGA selector](IntelFPGA/FPGASelector.md) | Supported | |
| [FPGA reg](IntelFPGA/FPGAReg.md) | Supported | |
| [SYCL_INTEL_kernel_restrict_all](KernelRestrictAll/SYCL_INTEL_kernel_restrict_all.asciidoc) | Supported | |
| [SYCL_INTEL_attribute_style](KernelRHSAttributes/SYCL_INTEL_attribute_style.asciidoc) | Proposal | |
| [Queue Order Properties](OrderedQueue/OrderedQueue_v2.adoc) | Supported | |
| [Queue Shortcuts](QueueShortcuts/QueueShortcuts.adoc) | Supported | |
| [Reductions for ND-Range Parallelism](Reduction/Reduction.md) | Partially supported | |
romanovvlad marked this conversation as resolved.
Show resolved Hide resolved
| [SYCL_INTEL_relax_standard_layout](RelaxStdLayout/SYCL_INTEL_relax_standard_layout.asciidoc) | Supported | |
| [SYCL_INTEL_reqd_work_group_size](ReqdWorkGroupSize/SYCL_INTEL_reqd_work_group_size.asciidoc) | Supported | |
| [SPV_INTEL_function_pointers](SPIRV/SPV_INTEL_function_pointers.asciidoc) | Supported | |
| [SPV_INTEL_inline_assembly](SPIRV/SPV_INTEL_inline_assembly.asciidoc) | Supported | |
| [SYCL_INTEL_static_local_memory_query](StaticLocalMemoryQuery/SYCL_INTEL_static_local_memory_query.asciidoc) | Proposal | |
| [SYCL_INTEL_sub_group_algorithms](SubGroupAlgorithms/SYCL_INTEL_sub_group_algorithms.asciidoc) | Proposal | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| [SYCL_INTEL_sub_group_algorithms](SubGroupAlgorithms/SYCL_INTEL_sub_group_algorithms.asciidoc) | Proposal | |
| [SYCL_INTEL_sub_group_algorithms](SubGroupAlgorithms/SYCL_INTEL_sub_group_algorithms.asciidoc) | Partially supported | Features from SYCL_INTEL_group_algorithms extended to sub-groups. Does not support host device. |

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied.

| [Sub-groups for NDRange Parallelism](SubGroupNDRange/SubGroupNDRange.md) | Proposal | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| [Sub-groups for NDRange Parallelism](SubGroupNDRange/SubGroupNDRange.md) | Proposal | |
| [Sub-groups for NDRange Parallelism](SubGroupNDRange/SubGroupNDRange.md) | Deprecated | Does not support host device. |

Copy link
Contributor

Choose a reason for hiding this comment

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

@Pennycook , @romanovvlad , do we need to add "SubGroups" to the list? https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/SubGroup/SYCL_INTEL_sub_group.asciidoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already mentioned, see line 31

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's not. There are 3 extensions in the directory: Subgroup, SubGroupNDRange and SubgroupAlgorithms and I think we want status per each.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pvchupin is right. I suggested what the Subgroup extension line could look like here: #1619 (comment)

SubGroupNDRange includes some queries and functions that were deprecated when equivalent functionality was introduced in GroupAlgorithms and SubgroupAlgorithms. We'll eventually drop support for it entirely (and when that happens we can probably remove its row from this table).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied original comment and added the line John suggested.

| [SYCL_INTEL_unnamed_kernel_lambda](UnnamedKernelLambda/SYCL_INTEL_unnamed_kernel_lambda.asciidoc) | Supported | |
| [Unified Shared Memory](USM/USM.adoc) | Supported | |