Skip to content

Conversation

@aelovikov-intel
Copy link
Contributor

No description provided.

Copy link
Contributor

@dm-vodopyanov dm-vodopyanov left a comment

Choose a reason for hiding this comment

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

Strange that we didn't have tests for that...

@gmlueck
Copy link
Contributor

gmlueck commented Apr 3, 2024

I think the following features are still implemented, even though they are part of the extension that you removed:

  • info::device::sub_group_independent_forward_progress
  • [[intel::named_sub_group_size]]
  • [[intel::intel::sub_group_size]]

For the attribute [[intel::intel::sub_group_size]], we should add a deprecation notice and tell people to use [[sycl::reqd_sub_group_size]] instead. Do we already have that deprecation notice? If so, we should remove support for that old attribute now.

We eventually want to replace [[intel::named_sub_group_size]] with the named_sub_group_size property from sycl_ext_oneapi_named_sub_group_sizes, but that extension isn't implemented yet. If we already have support for named sub-group sizes, is it easy to add the properties from this extension? Then, we could mark the attribute as deprecated, and eventually remove it.

I'm not sure what our plan is for info::device::sub_group_independent_forward_progress. Maybe @Pennycook knows?

I also noticed that this option:

  • -fsycl-default-sub-group-size

is implemented, and it was also documented in the extension you removed. It's also documented in the proposed sycl_ext_oneapi_named_sub_group_sizes extension. You can see a brief help message for this option via clang++ --help. Should we add it to the UsersManual? I'm not sure how we decide which options to document there.

@KornevNikita
Copy link
Contributor

There are also some redundant traits:

include/sycl/detail/spirv.hpp:template <> struct group_scope<::sycl::ext::oneapi::sub_group> {
include/sycl/detail/spirv.hpp:template <> struct GroupId<::sycl::ext::oneapi::sub_group> {
include/sycl/detail/type_traits.hpp:struct is_fixed_topology_group<sycl::ext::oneapi::sub_group> : std::true_type {
include/sycl/detail/type_traits.hpp:template <> struct is_sub_group<ext::oneapi::sub_group> : std::true_type {};

And forward declarations:

include/sycl/detail/spirv.hpp:struct sub_group;
include/sycl/detail/type_traits.hpp:struct sub_group;
include/sycl/ext/oneapi/sub_group_mask.hpp:struct sub_group;

@Pennycook
Copy link
Contributor

We eventually want to replace [[intel::named_sub_group_size]] with the named_sub_group_size property from sycl_ext_oneapi_named_sub_group_sizes, but that extension isn't implemented yet. If we already have support for named sub-group sizes, is it easy to add the properties from this extension? Then, we could mark the attribute as deprecated, and eventually remove it.

I think we might want to leave the attribute alone for now. I'd like to propose named sub-group sizes to Khronos, and it's not clear to me yet whether the attribute form or properties form will be preferred there (in the short term).

I'm not sure what our plan is for info::device::sub_group_independent_forward_progress. Maybe @Pennycook knows?

This corresponds to an OpenCL query that isn't very useful for SYCL, and we deliberately didn't propose it for inclusion in SYCL 2020. We should remove it and point to the queries from sycl_ext_oneapi_forward_progress instead (when they're implemented).

I also noticed that this option:

  • -fsycl-default-sub-group-size

is implemented, and it was also documented in the extension you removed. It's also documented in the proposed sycl_ext_oneapi_named_sub_group_sizes extension. You can see a brief help message for this option via clang++ --help. Should we add it to the UsersManual? I'm not sure how we decide which options to document there.

Somebody should check that this (and named sub-group sizes, actually) do what we'd expect. The last time I checked, the compiler recognized this flag and attribute but didn't actually connect it up to generating the correct SPIR-V.

@gmlueck
Copy link
Contributor

gmlueck commented Apr 3, 2024

I'm not sure what our plan is for info::device::sub_group_independent_forward_progress. Maybe @Pennycook knows?

This corresponds to an OpenCL query that isn't very useful for SYCL, and we deliberately didn't propose it for inclusion in SYCL 2020. We should remove it and point to the queries from sycl_ext_oneapi_forward_progress instead (when they're implemented).

If the current query isn't useful, can we just deprecate it now with no suggested alternative?

@Pennycook
Copy link
Contributor

If the current query isn't useful, can we just deprecate it now with no suggested alternative?

I think that's a good idea.

@aelovikov-intel aelovikov-intel added the abi-break change that's breaking abi and waiting for the next window to be able to merge label Apr 3, 2024
againull pushed a commit that referenced this pull request Aug 14, 2025
As discussed in #13258, not all of
sycl_ext_oneapi_named_sub_group_sizes has replacements ready. As such,
we cannot remove the sub_group class yet.
@aelovikov-intel aelovikov-intel deleted the remove-oneapi-sg branch November 6, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

abi-break change that's breaking abi and waiting for the next window to be able to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants