Signed-off-by: kbobrovs <[email protected]> (#3557)

[ESIMD] Make all simd_view constructors non-public.

This addresses long due ESIMD library review comments:

// TODO @rolandschulz
// {quote}
// Why is this and the next constructor public ? Those should only be called
// internally by e.g.select, correct ?
// {/quote}

// TODO @rolandschulz
// {quote}
// Is this intentional not a correct copy constructor (would need to be const
// for that)? I believe we agreed that simd_view would have a deleted copy and
// move constructor.Why are they suddenly back ?
// {/quote}

Author: kbobrovs

sycl/include/CL/sycl/INTEL/esimd/esimd.hpp

@@ -29,6 +29,8 @@ using namespace sycl::INTEL::gpu::detail;
 ///
 /// \ingroup sycl_esimd
 template <typename Ty, int N> class simd {
+  template <typename, typename> friend class simd_view;
+
 public:
   /// The underlying builtin data type.
   using vector_type = detail::vector_type_t<Ty, N>;

sycl/include/CL/sycl/INTEL/esimd/esimd_view.hpp

@@ -24,6 +24,9 @@ namespace gpu {
 ///
 /// \ingroup sycl_esimd
 template <typename BaseTy, typename RegionTy> class simd_view {
+  template <typename, int> friend class simd;
+  template <typename, typename> friend class simd_view;
+
 public:
   static_assert(!is_simd_view_v<BaseTy>::value);
   // Deduce the corresponding value type from its region type.
@@ -44,31 +47,17 @@ template <typename BaseTy, typename RegionTy> class simd_view {
   /// type of the base object type.
   using element_type = typename ShapeTy::element_type;
 
-  // TODO @rolandschulz
-  // {quote}
-  // Why is this and the next constructor public ? Those should only be called
-  // internally by e.g.select, correct ?
-  // {/quote}
-  //
   /// @{
   /// Constructors.
+
+private:
   simd_view(BaseTy &Base, RegionTy Region) : M_base(Base), M_region(Region) {}
   simd_view(BaseTy &&Base, RegionTy Region) : M_base(Base), M_region(Region) {}
 
-  // TODO @rolandschulz
-  // {quote}
-  // Is this intentional not a correct copy constructor (would need to be const
-  // for that)? I believe we agreed that simd_view would have a deleted copy and
-  // move constructor.Why are they suddenly back ?
-  // {/quote}
-  // TODO @kbobrovs
-  // copy constructor is still incorrect (no 'const'), move constructor is still
-  // present.
-  //
-  // Disallow copy constructor for simd_view.
-  simd_view(simd_view &Other) = delete;
-  simd_view(simd_view &&Other)
-      : M_base(Other.M_base), M_region(Other.M_region) {}
+public:
+  // Disallow copy and move constructors for simd_view.
+  simd_view(const simd_view &Other) = delete;
+  simd_view(simd_view &&Other) = delete;
   /// @}
 
   /// Conversion to simd type.

sycl/test/esimd/simd_view.cpp

@@ -7,18 +7,6 @@
 
 using namespace sycl::INTEL::gpu;
 
-bool test_simd_view_ctors() __attribute__((sycl_device)) {
-  simd<int, 16> v0(0, 1);
-
-  region1d_t<int, 4, 1> r0(4);
-  simd_view<simd<int, 16>, region1d_t<int, 4, 1>> ref0(v0, r0);
-  simd_view<simd<int, 16>, region1d_t<int, 4, 1>> ref1(std::move(ref0));
-  simd_view<simd<int, 16>, region1d_t<int, 4, 1>> ref2(
-      std::move(v0.select<4, 1>(8)));
-
-  return (ref0[0] == 4) && (ref1[1] == 5) && (ref2[0] == 8);
-}
-
 bool test_simd_view_bin_ops() __attribute__((sycl_device)) {
   simd<int, 16> v0 = 1;
   simd<int, 16> v1 = 2;