[SYCL][ESIMD] Support bool conversion to simd_mask in invoke_simd

[sarnex]

As per our recent discussions, bool passed into invoke_simd should convert to simd_mask with a user-defined type. Conversely, simd_mask will convert back to bool from inside the invoked function.

I also had to fix a couple of minor bugs in the implementation of the DPCPP-specific changes in std::experimental::simd.

I will make a follow-up PR to update the invoke_simd spec with the results of our recent discussion.

Test PR: intel/llvm-test-suite#1630

[sarnex]

@rolandschulz @Pennycook Hey guys, do you mind looking at this PR, particular at my std/experimental/simd.hpp change? It seems the original implementation has a couple of minor bugs fixed in this PR, but definitely would like a closer look. Thanks

[Pennycook]

I'm not sure about this change.

TS2 shows that this constructor is only available from fixed-sized masks: https://en.cppreference.com/w/cpp/experimental/simd/simd_mask/simd_mask

[sarnex]

Thanks for the review. The problem I'm trying to fix is the implicit conversion constructor doesn't work. Mind helping me find a better fix?

test.cpp:

#include <std/experimental/simd.hpp>

namespace simd_abi {
template <class T, int N>
using native_fixed_size = typename std::experimental::__simd_abi<
    std::experimental::_StorageKind::_VecExt, N>;
}

template <class T, int N>
using simd_mask =
    std::experimental::simd_mask<T, simd_abi::native_fixed_size<T, N>>;

int main() {

simd_mask<int, 8> x;
simd_mask<double, 8> y(x);
}

error:

test.cpp:16:22: error: no matching constructor for initialization of 'simd_mask<double, 8>' (aka 'simd_mask<double, __simd_abi<std::experimental::_StorageKind::_VecExt, 8>>')
simd_mask<double, 8> y(x);
                     ^ ~
../include/std/experimental/simd.hpp:1648:7: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'simd_mask<int, [...]>' to 'const simd_mask<double, [...]>' for 1st argument
class simd_mask {
      ^
../include/std/experimental/simd.hpp:1648:7: note: candidate constructor (the implicit move constructor) not viable: no known conversion from 'simd_mask<int, [...]>' to 'simd_mask<double, [...]>' for 1st argument
../include/std/experimental/simd.hpp:1674:12: note: candidate constructor not viable: no known conversion from 'simd_mask<int, 8>' (aka 'simd_mask<int, __simd_abi<std::experimental::_StorageKind::_VecExt, 8>>') to 'value_type' (aka 'bool') for 1st argument
  explicit simd_mask(value_type __v) noexcept {
           ^
../include/std/experimental/simd.hpp:1687:3: note: candidate template ignored: could not match '__simd_abi<1, [...]>' against '__simd_abi<2, [...]>'
  simd_mask(const simd_mask<_Up, simd_abi::fixed_size<size()>>& __v) noexcept {
  ^
../include/std/experimental/simd.hpp:1669:3: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
  simd_mask() = default;
  ^
../include/std/experimental/simd.hpp:1699:3: note: candidate constructor template not viable: requires 2 arguments, but 1 was provided
  simd_mask(const value_type*, _Flags);
  ^
1 error generated.

important part:

../include/std/experimental/simd.hpp:1687:3: note: candidate template ignored: could not match '__simd_abi<1, [...]>' against '__simd_abi<2, [...]>'
  simd_mask(const simd_mask<_Up, simd_abi::fixed_size<size()>>& __v) noexcept {

[Pennycook]

Ah, I see... We should wait to hear from @rolandschulz (because he's more familiar with the std::simd proposal), but I would be tempted to add a new constructor alongside the existing one, rather than replacing it. So, we'd have both:

simd_mask(const simd_mask<_Up, simd_abi::fixed_size<size()>>& __v) noexcept;
simd_mask(const simd_mask<_Up, abi_type>& __v) noexcept;

Does that work?

[sarnex]

ah yeah thats a better idea thanks, let me tentatively implement that

[v-klochkov]

@sarnex Jenkins/Precommit failed catastrophically. Can you please do 'git fetch' + 'git merge' to re-start testing?

[sarnex]

@v-klochkov I think you can do it by closing/reopening PR, let me try that

[v-klochkov]

@v-klochkov I think you can do it by closing/reopening PR, let me try that

It may work. git-merge though additionally may pick useful fixes for testing infra (if there were any)

[sarnex]

It may work. git-merge though additionally may pick useful fixes for testing infra (if there were any)

good point

[v-klochkov]

I am not merging yet because not sure if @rolandschulz had any comments.
Roland, if there are no comments or suggestions to changes something, then I'll merge it on Thursday evening.

[sarnex]

@v-klochkov Think we can merge this and the test PR and address any feedback from Roland in a follow up?