[SYCL] Fix accessor subscript ambiguity and reference type (#8033)

Accessor subscript operations may cause ambiguity when the data type is
constant. This commit fixes this ambiguity and makes the reference
member alias of accessors adhere to the SYCL 2020 definition of them.

---------

Signed-off-by: Larsen, Steffen <[email protected]>
Co-authored-by: Alexey Sachkov <[email protected]>

Author: steffenlarsen

sycl/include/sycl/accessor.hpp

@@ -344,8 +344,9 @@ class accessor_common {
     }
 
     template <int CurDims = SubDims,
-              typename = detail::enable_if_t<CurDims == 1 && IsAccessAnyWrite>>
-    RefType operator[](size_t Index) const {
+              typename = detail::enable_if_t<
+                  CurDims == 1 && (IsAccessReadOnly || IsAccessAnyWrite)>>
+    typename AccType::reference operator[](size_t Index) const {
       MIDs[Dims - CurDims] = Index;
       return MAccessor[MIDs];
     }
@@ -357,13 +358,6 @@ class accessor_common {
       MIDs[Dims - CurDims] = Index;
       return MAccessor[MIDs];
     }
-
-    template <int CurDims = SubDims,
-              typename = detail::enable_if_t<CurDims == 1 && IsAccessReadOnly>>
-    ConstRefType operator[](size_t Index) const {
-      MIDs[Dims - SubDims] = Index;
-      return MAccessor[MIDs];
-    }
   };
 };
 
@@ -1213,7 +1207,7 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
   // otherwise
   using value_type = typename std::conditional<AccessMode == access_mode::read,
                                                const DataT, DataT>::type;
-  using reference = DataT &;
+  using reference = value_type &;
   using const_reference = const DataT &;
 
   using iterator = typename detail::accessor_iterator<value_type, Dimensions>;
@@ -1973,30 +1967,17 @@ class __SYCL_EBO __SYCL_SPECIAL_CLASS __SYCL_TYPE(accessor) accessor :
   }
 
   template <int Dims = Dimensions, typename RefT = RefType,
-            typename = detail::enable_if_t<Dims == 0 && IsAccessAnyWrite &&
-                                           !std::is_const<RefT>::value>>
-  operator RefType() const {
+            typename = detail::enable_if_t<Dims == 0 && (IsAccessAnyWrite ||
+                                                         IsAccessReadOnly)>>
+  operator reference() const {
     const size_t LinearIndex = getLinearIndex(id<AdjustedDim>());
     return *(getQualifiedPtr() + LinearIndex);
   }
 
   template <int Dims = Dimensions,
-            typename = detail::enable_if_t<Dims == 0 && IsAccessReadOnly>>
-  operator ConstRefType() const {
-    const size_t LinearIndex = getLinearIndex(id<AdjustedDim>());
-    return *(getQualifiedPtr() + LinearIndex);
-  }
-
-  template <int Dims = Dimensions,
-            typename = detail::enable_if_t<(Dims > 0) && IsAccessAnyWrite>>
-  RefType operator[](id<Dimensions> Index) const {
-    const size_t LinearIndex = getLinearIndex(Index);
-    return getQualifiedPtr()[LinearIndex];
-  }
-
-  template <int Dims = Dimensions>
-  typename detail::enable_if_t<(Dims > 0) && IsAccessReadOnly, ConstRefType>
-  operator[](id<Dimensions> Index) const {
+            typename = detail::enable_if_t<(Dims > 0) && (IsAccessAnyWrite ||
+                                                          IsAccessReadOnly)>>
+  reference operator[](id<Dimensions> Index) const {
     const size_t LinearIndex = getLinearIndex(Index);
     return getQualifiedPtr()[LinearIndex];
   }

sycl/test/regression/accessor_subscript_and_ref_type.cpp

@@ -0,0 +1,97 @@
+// RUN: %clangxx -fsycl -fsyntax-only %s
+
+// Test checks that the subscript operator and reference alias on accessors
+// evaluate to the right types.
+#include <sycl/sycl.hpp>
+
+using namespace sycl;
+
+// Trait for getting the return type of a full subscript operation.
+template <int Dims, typename AccT> struct FullSubscriptType;
+template <typename AccT> struct FullSubscriptType<1, AccT> {
+  using type = decltype(std::declval<AccT>()[0]);
+};
+template <typename AccT> struct FullSubscriptType<2, AccT> {
+  using type = decltype(std::declval<AccT>()[0][0]);
+};
+template <typename AccT> struct FullSubscriptType<3, AccT> {
+  using type = decltype(std::declval<AccT>()[0][0][0]);
+};
+template <int Dims, typename AccT>
+using FullSubscriptTypeT = typename FullSubscriptType<Dims, AccT>::type;
+
+// Expected reference type of an accessor given an access mode.
+template <access::mode AccessMode, typename DataT>
+using ExpectedRefTypeT = std::conditional_t<AccessMode == access::mode::read,
+                                            std::add_const_t<DataT> &, DataT &>;
+
+// Trait for getting the expected return type of a full subscript operation.
+template <access::mode AccessMode, access::target AccessTarget, typename DataT>
+struct ExpectedSubscriptType {
+  using type = ExpectedRefTypeT<AccessMode, DataT>;
+};
+template <typename DataT, access::target AccessTarget>
+struct ExpectedSubscriptType<access::mode::atomic, AccessTarget, DataT> {
+  using type = atomic<DataT, access::address_space::global_space>;
+};
+template <typename DataT>
+struct ExpectedSubscriptType<access::mode::atomic, access::target::local,
+                             DataT> {
+  using type = atomic<DataT, access::address_space::local_space>;
+};
+template <access::mode AccessMode, access::target AccessTarget, typename DataT>
+using ExpectedSubscriptTypeT =
+    typename ExpectedSubscriptType<AccessMode, AccessTarget, DataT>::type;
+
+template <typename DataT, int Dims, access::mode AccessMode,
+          access::target AccessTarget, typename AccT>
+void CheckAccRefAndSubscript() {
+  static_assert(std::is_same_v<typename AccT::reference,
+                               ExpectedRefTypeT<AccessMode, DataT>>);
+  static_assert(
+      std::is_same_v<FullSubscriptTypeT<Dims, AccT>,
+                     ExpectedSubscriptTypeT<AccessMode, AccessTarget, DataT>>);
+}
+
+template <typename DataT, int Dims, access::mode AccessMode> void CheckAcc() {
+  CheckAccRefAndSubscript<DataT, Dims, AccessMode, access::target::host_buffer,
+                          host_accessor<DataT, Dims, AccessMode>>();
+  CheckAccRefAndSubscript<
+      DataT, Dims, AccessMode, access::target::device,
+      accessor<DataT, Dims, AccessMode, access::target::device>>();
+  CheckAccRefAndSubscript<
+      DataT, Dims, AccessMode, access::target::host_buffer,
+      accessor<DataT, Dims, AccessMode, access::target::host_buffer>>();
+  if constexpr (AccessMode == access::mode::read_write) {
+    CheckAccRefAndSubscript<DataT, Dims, AccessMode, access::target::local,
+                            local_accessor<DataT, Dims>>();
+  }
+  if constexpr (AccessMode == access::mode::read_write ||
+                AccessMode == access::mode::atomic) {
+    CheckAccRefAndSubscript<
+        DataT, Dims, AccessMode, access::target::local,
+        accessor<DataT, Dims, AccessMode, access::target::local>>();
+  }
+}
+
+template <typename DataT, access::mode AccessMode> void CheckAccAllDims() {
+  CheckAcc<DataT, 1, AccessMode>();
+  CheckAcc<DataT, 2, AccessMode>();
+  CheckAcc<DataT, 3, AccessMode>();
+}
+
+template <typename DataT> void CheckAccAllAccessModesAndDims() {
+  CheckAccAllDims<DataT, access::mode::write>();
+  CheckAccAllDims<DataT, access::mode::read_write>();
+  CheckAccAllDims<DataT, access::mode::discard_write>();
+  CheckAccAllDims<DataT, access::mode::discard_read_write>();
+  CheckAccAllDims<DataT, access::mode::read>();
+  if constexpr (!std::is_const_v<DataT>)
+    CheckAccAllDims<DataT, access::mode::atomic>();
+}
+
+int main() {
+  CheckAccAllAccessModesAndDims<int>();
+  CheckAccAllAccessModesAndDims<const int>();
+  return 0;
+}