[SYCL] Augment known_identity for std::complex and std::plus.

[uditagarwal97]

This PR proposes to augment sycl::has_known_identity to return true for std::complex and std::plus operator. This will have two benefits:

1. It enables support for complex numbers in sycl::reduction without the user having to explicitly pass identity.
2. sycl::known_identity can now be used to simplify the implementation of group algorithms (See PR [SYCL] Add complex support to group algorithms #5394).

Also, this PR addresses the Github issue #5477.
Test Case PR: intel/llvm-test-suite#1609

[Pennycook]

Why do we have these two instantiations of std::complex<> for integral T?

I've only ever seen std::complex<float>, std::complex<double> and std::complex<long double>. Both sycl_ext_oneapi_complex and sycl_ext_oneapi_complex_algorithms only support float, double and sycl::half.

[uditagarwal97]

Thanks for bringing this up. I wasn't aware that for std::complex, we only support float and double types. I've accordingly made the changes.

[aelovikov-intel]

Do we need to update any extensions with new wording?

[uditagarwal97]

Do we need to update any extensions with new wording?

I'm not sure but I think update to sycl_ext_oneapi_complex_algorithms.asciidoc is not required as it already introduces std::complex and std::complex to reduce and reduce_over_group algorithms.
@Pennycook What's your opinion?

[Pennycook]

I'm not sure but I think update to sycl_ext_oneapi_complex_algorithms.asciidoc is not required as it already introduces std::complex and std::complex to reduce and reduce_over_group algorithms. @Pennycook What's your opinion?

SYCL 2020 says:

If an implementation can identify an identity value for a given combination of accumulator type and function object type, the value is defined as a member of the known_identity trait class. Whether this member value exists can be tested using the has_known_identity trait class.

Since the intent of this trait is to let people test which types an implementation can reason about, it seems redundant to also put that information into an extension specification. But I might just be feeling lazy, so I'd like @gmlueck's opinion on this.

[gmlueck]

Since the intent of this trait is to let people test which types an implementation can reason about, it seems redundant to also put that information into an extension specification. But I might just be feeling lazy, so I'd like @gmlueck's opinion on this.

I think it's worthwhile to add this to the extension specification. The core SYCL spec has Table 123 "Known identities", which lists the operators that have known identifies for certain types. It would make sense for our extension spec to list extended cases that have known identities.

BTW, sycl_ext_oneapi_complex_algorithms is listed as "proposed". Is this implemented now? If so, it should be moved to either "experimental" or "supported" as appropriate (and it should be updated to note its current status).

[uditagarwal97]

Since the intent of this trait is to let people test which types an implementation can reason about, it seems redundant to also put that information into an extension specification. But I might just be feeling lazy, so I'd like @gmlueck's opinion on this.

I think it's worthwhile to add this to the extension specification. The core SYCL spec has Table 123 "Known identities", which lists the operators that have known identifies for certain types. It would make sense for our extension spec to list extended cases that have known identities.

Sure, thanks for the feedback. I've updated the extension specification as well. @Pennycook please review the changes.

BTW, sycl_ext_oneapi_complex_algorithms is listed as "proposed". Is this implemented now? If so, it should be moved to either "experimental" or "supported" as appropriate (and it should be updated to note its current status).

I don't know if sycl_ext_oneapi_complex_algorithms is completely implemented.

[gmlueck]

I don't know if sycl_ext_oneapi_complex_algorithms is completely implemented.

Do either of you know? I'm wondering if we just forgot to move the extension specification.

[Pennycook]

Do either of you know? I'm wondering if we just forgot to move the extension specification.

sycl_ext_oneapi_complex_algorithms says that it depends on two other extensions. The first is sycl_ext_oneapi_complex, which is still showing as "proposed". The second is sycl_ext_oneapi_marray, which hasn't actually been merged yet: #6550

[gmlueck]

spec changes LGTM

[aelovikov-intel]

SYCL RT changes LGTM.

[uditagarwal97]

@intel/llvm-gatekeepers The PR is ready!

[steffenlarsen]

CUDA Failed Tests (1):
SYCL :: SubGroup/shuffle_fp64.cpp
Unrelated and reported in #8516.