Skip to content

[ESIMD] Add writeable subsript operator for simd_view #4384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 25, 2021

Conversation

DenisBakhvalov
Copy link
Contributor

The primary goal for this patch is to allow writing through
simd_view subscript operator, e.g.:

  simd<int, 4> v = 1;
  auto vv = v.select<2, 1>(0);
  vv[0] = 42;

All the other changes are required to make that happen:

  1. One more specialization for nested simd_view class with len=1.

  2. Added explicit definition of BINOP for simd_view_impl with
    a scalar value because of the following code:

  simd<ushort, 64> s = 0;
  auto g = s.bit_cast_view<ushort, 4, 16>();
  auto x = g.row(1) - (g.row(1))[0];

Here g.row(1) return simd_view and (g.row(1))[0] returns
simd_view of size 1. To use any binops defined in simd_view_impl
before this patch we would need to implicitly convert RHS to
scalar and then convert scalar to vector type. Two implicit
conversions are not allowed according to C++ language rules,
hence new BINOP operator definition for simd_view_impl.

  1. I also added additional template parameter for simd_view
    specialization for lenght=1 to support the following code:
  simd<int, 1> s = 0;
  float f = s.bit_cast_view<float>();

This code is OK. The result of bit_cast_view should be mapped
to specialization of simd_view with length 1, so conversion to
scalar is allowed. To enable such mapping, I fixed the definition
of region_base_1. Previously we had zeros for StrideY and StrideX
but in fact, we don't care about their values, they can take any
values.

The primary goal for this patch is to allow writing through
simd_view subscript operator, e.g.:
```
  simd<int, 4> v = 1;
  auto vv = v.select<2, 1>(0);
  vv[0] = 42;
```

All the other changes are required to make that happen:

1. One more specialization for nested simd_view class with len=1.

2. Added explicit definition of BINOP for simd_view_impl with
a scalar value because of the following code:
```
  simd<ushort, 64> s = 0;
  auto g = s.bit_cast_view<ushort, 4, 16>();
  auto x = g.row(1) - (g.row(1))[0];
```
Here `g.row(1)` return simd_view and `(g.row(1))[0]` returns
simd_view of size 1. To use any binops defined in simd_view_impl
before this patch we would need to implicitly convert RHS to
scalar and then convert scalar to vector type. Two implicit
conversions are not allowed according to C++ language rules,
hence new BINOP operator definition for simd_view_impl.

3. I also added additional template parameter for simd_view
specialization for lenght=1 to support the following code:
```
  simd<int, 1> s = 0;
  float f = s.bit_cast_view<float>();
```
This code is OK. The result of bit_cast_view should be mapped
to specialization of simd_view with length 1, so conversion to
scalar is allowed. To enable such mapping, I fixed the definition
of region_base_1. Previously we had zeros for StrideY and StrideX
but in fact, we don't care about their values, they can take any
values.
@DenisBakhvalov
Copy link
Contributor Author

Corresponding E2E test is here: intel/llvm-test-suite#415

@bader bader merged commit b7654b9 into intel:sycl Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants