[SYCL][Doc] Add sycl complex to complex algorithms extension #6717
Conversation
| This extension builds on the `sycl::ext::oneapi::complex` and | ||
| `marray<sycl::ext::oneapi::complex>` classes. Therefore this extension is | ||
| dependent on link:sycl_ext_oneapi_complex.asciidoc[sycl_ext_oneapi_complex] | ||
| and | ||
| link:sycl_ext_oneapi_complex_marray.asciidoc[sycl_ext_oneapi_complex_marray]. |
There was a problem hiding this comment.
Prior to this change, developers could use the std::complex features without requiring those other two extensions. Does anything in our std::complex support now depend on these new extensions? If not, I think we should be explicit that only those parts of this extension which use sycl::complex or marray are dependent.
There was a problem hiding this comment.
Yes, nothing in std::complex is dependent on these new extensions. I have added a bit stating that all function overloads that use std::complex are not dependent upon these extensions.
| Usage of `gencomplex` and `mgencomplex` with double value type requires support | ||
| for double precision, and specifically the `sycl::aspect::fp64` device aspect. | ||
| Similarly, usage of `gencomplex` and `mgencomplex` with half value type | ||
| requires support for half-precision, with `sycl::aspect::fp16`. |
There was a problem hiding this comment.
Nit: This is a little confusing to me, because gencomplex and mgencomplex refer to all the value types. "gencomplex with half value type" seems less precise than "sycl::complex<sycl::half> and sycl::marray<sycl::complex<sycl::half>>>".
There was a problem hiding this comment.
Agreed ya this is confusing. I have updated this bit to, hopefully, be much clearer.
| The following group functions and algorithms accept `gencomplex` arguments | ||
| only when used with a `BinaryOperation` argument of `sycl::plus` and | ||
| `sycl::multiplies`. These operations do not support `mgencomplex`: |
There was a problem hiding this comment.
This change seems unrelated to the sycl::complex and sycl::marray parts of things. Can you say more about why you're lifting this restriction, and what SPIR-V instruction(s) you expect it to generate?
There was a problem hiding this comment.
Yes this change is somewhat unrelated to sycl::complex and sycl::marray<complex>, this restriction is being lifted to allow for a more fleshed out version of this extension. The multiplies operation is a valid binary operation for reductions and scans so we would like to support it. We currently do not have a valid SPIR-V instruction for the multiplies operation, so we would be adding a new one to support this op. I think we would name the instruction something along the lines of __spirv_GroupCMulKHR
This PR extends the complex algorithms extension to support
sycl::ext::oneapi::complexandmarray<sycl::ext::oneapi::complex>. Additionally it adds themultipliesoperator as a valid binary operation for complex values when reducing and scanning across work items. This PR has a dependency upon #6550.