[ESIMD] Fix 'ambiguous operator' error with length 1 simd operands #4149
Conversation
Signed-off-by: kbobrovs <Konstantin.S.Bobrovsky@intel.com>
| @@ -193,6 +193,8 @@ template <typename BaseTy, typename RegionTy> class simd_view_impl { | |||
| } | |||
|
|
|||
| #define DEF_BINOP(BINOP, OPASSIGN) \ | |||
| template <class T1 = simd_view_impl, \ | |||
There was a problem hiding this comment.
As I understood a[0] + x was ambiguous when the underlying types were different, right? E.g., int and uint32_t.
You resolve it by disabling one of the candidates and rely on implicit conversion to underlying type and built-in operator+ for primitive types.
Can we sink those two BINOP overloads into the derived class of generic simd_view implementation? That way I think we don't need this SFINAE trick.
There was a problem hiding this comment.
As I understood a[0] + x was ambiguous when the underlying types were different, right? E.g., int and uint32_t.
not really. int - int causes the failure too. In the test I just modelled the user test case where the types were different.
You resolve it by disabling one of the candidates and rely on implicit conversion to underlying type and built-in operator+ for primitive types.
yes, with length == 1, simd_view_impl and value_type are implicitly convertible to the element type, thus compiler will always be able to resolve BINOP(const simd_view_impl &, const value_type &) to the built-in operation over primitive types after implicitly converting both sides.
Can we sink those two BINOP overloads into the derived class of generic simd_view implementation?
not sure I understand. Do you mean moving BINOP from simd_view_impl to simd_view? That won't affect resolution in any way, since simd_view already binds to simd_view_impl w/o any conversions as simd_view extends simd_view_impl.
There was a problem hiding this comment.
Per discussion with @DenisBakhvalov: his suggestion was to move BINOP to simd_view to make them unavailable for the length=1 simd_view specialization. That might make sense, but (1) would be pretty intrusive change at this point (2) my upcoming simd_mask introduction patch refactors operators anyway (none will be friends), so this can be re-visited when reviewing that patch. So we decided to go with the current fix at this point.
…ackend_plugin * upstream/sycl: (26 commits) [SPIR-V][NFC] Move non-upstreamed FuncParam decorations into internal:: (intel#4138) [SYCL] Move free function queries to experimental namespace (intel#4090) [SYCL][XPTI] Enable PI calls notifications with arguments (intel#4148) [SYCL] Revert queue::wait() to its old behaviour with Level Zero (intel#4153) [SYCL] Add missing <cstring> header to spirv.hpp (intel#4157) [SYCL] Adds info query for atomic_memory_order_capabilities on device and context (intel#4105) [SYCL] Improve performance of generic shuffles (intel#3815) [SYCL] Fix the error with namespaces caused during rebase of intel#4014 (intel#4151) [ESIMD] Fix 'ambiguous operator' error with length 1 simd operands (intel#4149) [libdevice][NFC] Fix libdevice dependencies list (intel#4130) [SPIR-V] Reland Enco 829F de debug info producer in SPIR-V (intel#4082) [SYCL][ROCm] Add ROCm support to get_device_count_by_type (intel#4113) [SYCL] Fix sRGB device info (intel#4145) [SYCL][ROCm] Fix kernel launch with multiple dimensions (intel#4063) [SYCL][ROCm] Fix compilation for AMD GPU with -fsycl-dead-args-optimization (intel#4126) [SYCL][Level Zero] Enable multi-CCS support. (intel#4038) [SYCL] Pass bound arch to unbundler (intel#4112) [ESIMD][doc] Added documentation for some ESIMD math APIs (intel#3995) [ESIMD] rename gather4/scatter4 to gather_rgba/scatter_rgba (intel#4120) [SYCL][NFC] Remove unused variable. (intel#4131) ...
Signed-off-by: kbobrovs Konstantin.S.Bobrovsky@intel.com