[SYCL] Don't expose vector of booleans as storage format (#1419)

bader · Victor Lomuller · web-flow · commit f43883b5ba89 · 2020-03-31T19:12:24.000+03:00
This reverts commit 1b8c008. Revert "[SYCL] Enable ext_vector_type of boolean type for SYCL language." Replaces vector of booleans with vector of integers to store results of logical/relational operations. Similar change is propagated to SPIR-V built-ins declarations. Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com> Signed-off-by: Alexey Bader <alexey.bader@intel.com> Co-authored-by: Victor Lomuller <victor@codeplay.com>
diff --git a/clang/lib/Sema/SPIRVBuiltins.td b/clang/lib/Sema/SPIRVBuiltins.td
@@ -367,7 +367,7 @@ def IntLongFloatGenType1   : GenericType<"IntLongFloatGenType1", TLIntLongFloats
 
 // GenType definitions for every single base type (e.g. fp32 only).
 // Names are like: GenTypeFloatVecAndScalar.
-foreach Type = [Bool, Char, UChar, Short, UShort,
+foreach Type = [Char, UChar, SChar, Short, UShort,
                 Int, UInt, Long, ULong,
                 Float, Double, Half] in {
   foreach VecSizes = [VecAndScalar, VecNoScalar] in {
@@ -816,13 +816,17 @@ foreach name = ["Dot"] in {
 }
 
 foreach name = ["Any", "All"] in {
-  def : SPVBuiltin<name, [Bool, GenTypeBoolVecAndScalar], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Bool], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, GenTypeSCharVecNoScalar], Attr.Const>;
 }
 
 foreach name = ["IsNan", "IsInf", "IsFinite", "IsNormal", "SignBitSet"] in {
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeFloatVecAndScalar], Attr.Const>;
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeDoubleVecAndScalar], Attr.Const>;
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeHalfVecAndScalar], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Float], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Double], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Half], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeFloatVecNoScalar], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeDoubleVecNoScalar], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeHalfVecNoScalar], Attr.Const>;
 }
 
 foreach name = ["LessOrGreater",
@@ -833,9 +837,12 @@ foreach name = ["LessOrGreater",
                 "FOrdGreaterThan", "FUnordGreaterThan",
                 "FOrdLessThanEqual", "FUnordLessThanEqual",
                 "FOrdGreaterThanEqual", "FUnordGreaterThanEqual"] in {
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeFloatVecAndScalar, GenTypeFloatVecAndScalar], Attr.Const>;
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeDoubleVecAndScalar, GenTypeDoubleVecAndScalar], Attr.Const>;
-  def : SPVBuiltin<name, [GenTypeBoolVecAndScalar, GenTypeHalfVecAndScalar, GenTypeHalfVecAndScalar], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Float, Float], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Double, Double], Attr.Const>;
+  def : SPVBuiltin<name, [Bool, Half, Half], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeFloatVecNoScalar, GenTypeFloatVecNoScalar], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeDoubleVecNoScalar, GenTypeDoubleVecNoScalar], Attr.Const>;
+  def : SPVBuiltin<name, [GenTypeSCharVecNoScalar, GenTypeHalfVecNoScalar, GenTypeHalfVecNoScalar], Attr.Const>;
 }
 
 foreach name = ["BitCount"] in {
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
@@ -2491,7 +2491,7 @@ QualType Sema::BuildExtVectorType(QualType T, Expr *ArraySize,
   // of bool aren't allowed.
   if ((!T->isDependentType() && !T->isIntegerType() &&
        !T->isRealFloatingType()) ||
-      (!Context.getLangOpts().SYCLIsDevice && T->isBooleanType())) {
+      T->isBooleanType()) {
     Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T;
     return QualType();
   }
diff --git a/clang/test/CodeGenSYCL/bool-vectors.cpp b/clang/test/CodeGenSYCL/bool-vectors.cpp
diff --git a/clang/test/SemaSYCL/bool-vectors.cpp b/clang/test/SemaSYCL/bool-vectors.cpp
diff --git a/sycl/include/CL/sycl/detail/boolean.hpp b/sycl/include/CL/sycl/detail/boolean.hpp
@@ -28,7 +28,7 @@ template <int Num> struct Assigner {
   static void init(R &r, const T x) {
     Assigner<Num - 1>::template init<R, T, ET>(r, x);
     ET v = x.template swizzle<Num>();
-    r.value[Num] = msbIsSet(v);
+    r.value[Num] = msbIsSet(v) * (-1);
   }
 };
 
@@ -39,15 +39,15 @@ template <> struct Assigner<0> {
   template <typename R, typename T, typename ET>
   static void init(R &r, const T x) {
     ET v = x.template swizzle<0>();
-    r.value[0] = msbIsSet(v);
+    r.value[0] = msbIsSet(v) * (-1);
   }
 };
 
 template <int N> struct Boolean {
   static_assert(((N == 2) || (N == 3) || (N == 4) || (N == 8) || (N == 16)),
                 "Invalid size");
 
-  using element_type = bool;
+  using element_type = int8_t;
 
 #ifdef __SYCL_DEVICE_ONLY__
   using DataType = element_type __attribute__((ext_vector_type(N)));
@@ -56,11 +56,11 @@ template <int N> struct Boolean {
   using DataType = element_type[N];
 #endif
 
-  Boolean() : value{false} {}
+  Boolean() : value{0} {}
 
   Boolean(std::initializer_list<element_type> l) {
     for (size_t I = 0; I < N; ++I) {
-      value[I] = *(l.begin() + I);
+      value[I] = *(l.begin() + I) ? -1 : 0;
     }
   }
 
@@ -70,6 +70,12 @@ template <int N> struct Boolean {
     }
   }
 
+  template <typename T> Boolean(const T rhs) {
+    static_assert(is_vgeninteger<T>::value, "Invalid constructor");
+    Assigner<N - 1>::template init<Boolean<N>, T, typename T::element_type>(
+        *this, rhs);
+  }
+
 #ifdef __SYCL_DEVICE_ONLY__
   // TODO change this to the vectors assignment when the assignment will be
   // fixed on Intel GPU NEO OpenCL runtime
@@ -78,64 +84,45 @@ template <int N> struct Boolean {
       value[I] = rhs[I];
     }
   }
-#endif
-
-  template <typename T> Boolean(const T rhs) {
-    static_assert(is_vgeninteger<T>::value, "Invalid constructor");
-    Assigner<N - 1>::template init<Boolean<N>, T, typename T::element_type>(
-        *this, rhs);
-  }
 
-#ifdef __SYCL_DEVICE_ONLY__
   operator vector_t() const { return value; }
 #endif
 
   template <typename T> operator T() const {
     static_assert(is_vgeninteger<T>::value, "Invalid conversion");
     T r;
     Assigner<N - 1>::assign(r, *this);
-    return r * -1;
+    return r;
   }
 
 private:
   template <int Num> friend struct Assigner;
-  alignas(detail::vector_alignment<bool, N>::value) DataType value;
+  alignas(detail::vector_alignment<element_type, N>::value) DataType value;
 };
 
 template <> struct Boolean<1> {
+  Boolean() = default;
 
-  using element_type = bool;
-
-#ifdef __SYCL_DEVICE_ONLY__
-  using DataType = element_type;
-  using vector_t = DataType;
-#else
-  using DataType = element_type;
-#endif
-
-  Boolean() : value(false) {}
-
-  Boolean(const Boolean &rhs) : value(rhs.value) {}
-
-#ifdef __SYCL_DEVICE_ONLY__
-  Boolean(const vector_t rhs) : value(rhs) {}
-#endif
-
+  // Build from a signed interger type
   template <typename T> Boolean(T val) : value(val) {
     static_assert(is_sgeninteger<T>::value, "Invalid constructor");
   }
 
-#ifdef __SYCL_DEVICE_ONLY__
-  operator vector_t() const { return value; }
-#endif
-
+  // Cast to a signed interger type
   template <typename T> operator T() const {
     static_assert(is_sgeninteger<T>::value, "Invalid conversion");
     return value;
   }
 
+#ifdef __SYCL_DEVICE_ONLY__
+  // Build from a boolean type
+  Boolean(bool f) : value(f) {}
+  // Cast to a boolean type
+  operator bool() const { return value; }
+#endif
+
 private:
-  alignas(1) DataType value;
+  alignas(1) bool value = false;
 };
 
 } // namespace detail
diff --git a/sycl/include/CL/sycl/detail/generic_type_traits.hpp b/sycl/include/CL/sycl/detail/generic_type_traits.hpp
@@ -480,7 +480,7 @@ convertDataToType(FROM t) {
   return TryToGetPointer(t);
 }
 
-// Used for all,any and select relational built-in functions
+// Used for all, any and select relational built-in functions
 template <typename T> inline constexpr T msbMask(T) {
   using UT = make_unsigned_t<T>;
   return T(UT(1) << (sizeof(T) * 8 - 1));
@@ -559,7 +559,7 @@ struct RelConverter<
 #ifdef __SYCL_DEVICE_ONLY__
     typename ret_t::vector_t result(0);
     for (size_t I = 0; I < N; ++I) {
-      result[I] = 0 - value[I];
+      result[I] = value[I];
     }
     return result;
 #else

Original file line number	Diff line number	Diff line change
`@@ -2491,7 +2491,7 @@ QualType Sema::BuildExtVectorType(QualType T, Expr *ArraySize,`
`2491`	`2491`	`// of bool aren't allowed.`
`2492`	`2492`	`if ((!T->isDependentType() && !T->isIntegerType() &&`
`2493`	`2493`	`!T->isRealFloatingType()) \|\|`
`2494`		`- (!Context.getLangOpts().SYCLIsDevice && T->isBooleanType())) {`
	`2494`	`+ T->isBooleanType()) {`
`2495`	`2495`	`Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T;`
`2496`	`2496`	`return QualType();`
`2497`	`2497`	`}`