From 4853e7b92c179251b7eb13b0ccfeb0ff3d51484c Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Mon, 16 Mar 2020 13:39:39 +0300 Subject: [PATCH 1/7] [SYCL] Don't expose vector of booleans as storage format This reverts commit 1b8c0089ad79ee4765051ddfc722abc31d3ddac9. 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 Signed-off-by: Alexey Bader --- clang/lib/Sema/SPIRVBuiltins.td | 39 ++++++++-- clang/lib/Sema/SemaType.cpp | 2 +- clang/test/CodeGenSYCL/bool-vectors.cpp | 75 ------------------- clang/test/SemaSYCL/bool-vectors.cpp | 27 ------- sycl/include/CL/sycl/detail/boolean.hpp | 45 +++++------ .../CL/sycl/detail/generic_type_traits.hpp | 4 +- 6 files changed, 51 insertions(+), 141 deletions(-) delete mode 100644 clang/test/CodeGenSYCL/bool-vectors.cpp delete mode 100644 clang/test/SemaSYCL/bool-vectors.cpp diff --git a/clang/lib/Sema/SPIRVBuiltins.td b/clang/lib/Sema/SPIRVBuiltins.td index 69c8f7b82c54e..ec0701081e20f 100644 --- 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 = [Bool, Char, UChar, SChar, Short, UShort, Int, UInt, Long, ULong, Float, Double, Half] in { foreach VecSizes = [VecAndScalar, VecNoScalar] in { @@ -816,13 +816,36 @@ foreach name = ["Dot"] in { } foreach name = ["Any", "All"] in { - def : SPVBuiltin; + def : SPVBuiltin; +} + +foreach name = ["Any", "All"] in { + def : SPVBuiltin; +} + +foreach name = ["IsNan", "IsInf", "IsFinite", "IsNormal", "SignBitSet"] in { + def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; } foreach name = ["IsNan", "IsInf", "IsFinite", "IsNormal", "SignBitSet"] in { - def : SPVBuiltin; - def : SPVBuiltin; - def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; +} + +foreach name = ["LessOrGreater", + "Ordered", "Unordered", + "FOrdEqual", "FUnordEqual", + "FOrdNotEqual", "FUnordNotEqual", + "FOrdLessThan", "FUnordLessThan", + "FOrdGreaterThan", "FUnordGreaterThan", + "FOrdLessThanEqual", "FUnordLessThanEqual", + "FOrdGreaterThanEqual", "FUnordGreaterThanEqual"] in { + def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; } foreach name = ["LessOrGreater", @@ -833,9 +856,9 @@ foreach name = ["LessOrGreater", "FOrdGreaterThan", "FUnordGreaterThan", "FOrdLessThanEqual", "FUnordLessThanEqual", "FOrdGreaterThanEqual", "FUnordGreaterThanEqual"] in { - def : SPVBuiltin; - def : SPVBuiltin; - def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; + def : SPVBuiltin; } foreach name = ["BitCount"] in { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 0ac822a0ee227..a4fd9c33e29ba 100644 --- 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 deleted file mode 100644 index bdec9839218bd..0000000000000 --- a/clang/test/CodeGenSYCL/bool-vectors.cpp +++ /dev/null @@ -1,75 +0,0 @@ -// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64-unknown-unknown-sycldevice -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s - -template -__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { - kernelFunc(); -} -using bool1 = bool; -using bool2 = bool __attribute__((ext_vector_type(2))); -using bool3 = bool __attribute__((ext_vector_type(3))); -using bool4 = bool __attribute__((ext_vector_type(4))); -using bool8 = bool __attribute__((ext_vector_type(8))); -using bool16 = bool __attribute__((ext_vector_type(16))); - -extern SYCL_EXTERNAL bool1 foo1(); -// CHECK-DAG: declare spir_func zeroext i1 @[[FOO1:[a-zA-Z0-9_]+]]() -extern SYCL_EXTERNAL bool2 foo2(); -// CHECK-DAG: declare spir_func <2 x i1> @[[FOO2:[a-zA-Z0-9_]+]]() -extern SYCL_EXTERNAL bool3 foo3(); -// CHECK-DAG: declare spir_func <3 x i1> @[[FOO3:[a-zA-Z0-9_]+]]() -extern SYCL_EXTERNAL bool4 foo4(); -// CHECK-DAG: declare spir_func <4 x i1> @[[FOO4:[a-zA-Z0-9_]+]]() -extern SYCL_EXTERNAL bool8 foo8(); -// CHECK-DAG: declare spir_func <8 x i1> @[[FOO8:[a-zA-Z0-9_]+]]() -extern SYCL_EXTERNAL bool16 foo16(); -// CHECK-DAG: declare spir_func <16 x i1> @[[FOO16:[a-zA-Z0-9_]+]]() - -void bar (bool1 b) {}; -// CHECK-DAG: define spir_func void @[[BAR1:[a-zA-Z0-9_]+]](i1 zeroext % -void bar (bool2 b) {}; -// CHECK-DAG: define spir_func void @[[BAR2:[a-zA-Z0-9_]+]](<2 x i1> % -void bar (bool3 b) {}; -// CHECK-DAG: define spir_func void @[[BAR3:[a-zA-Z0-9_]+]](<3 x i1> % -void bar (bool4 b) {}; -// CHECK-DAG: define spir_func void @[[BAR4:[a-zA-Z0-9_]+]](<4 x i1> % -void bar (bool8 b) {}; -// CHECK-DAG: define spir_func void @[[BAR8:[a-zA-Z0-9_]+]](<8 x i1> % -void bar (bool16 b) {}; -// CHECK-DAG: define spir_func void @[[BAR16:[a-zA-Z0-9_]+]](<16 x i1> % - -int main() { - kernel( - [=]() { - bool1 b1 = foo1(); - // CHECK-DAG: [[B1:%[a-zA-Z0-9_]+]] = alloca i8, align 1 - // CHECK-DAG: [[CALL1:%[a-zA-Z0-9_]+]] = call spir_func zeroext i1 @[[FOO1]] - bar(b1); - // CHECK-DAG: call spir_func void @[[BAR1]](i1 zeroext [[B1_ARG:%[a-zA-Z0-9_]+]] - bool2 b2 = foo2(); - // CHECK-DAG: [[B2:%[a-zA-Z0-9_]+]] = alloca <2 x i1>, align 2 - // CHECK-DAG: [[CALL2:%[a-zA-Z0-9_]+]] = call spir_func <2 x i1> @[[FOO2]] - bar(b2); - // CHECK-DAG: call spir_func void @[[BAR2]](<2 x i1> [[B2_ARG:%[a-zA-Z0-9_]+]] - bool3 b3 = foo3(); - // CHECK-DAG: [[B3:%[a-zA-Z0-9_]+]] = alloca <3 x i1>, align 4 - // CHECK-DAG: [[CALL3:%[a-zA-Z0-9_]+]] = call spir_func <3 x i1> @[[FOO3]] - bar(b3); - // CHECK-DAG: call spir_func void @[[BAR3]](<3 x i1> [[B3_ARG:%[a-zA-Z0-9_]+]] - bool4 b4 = foo4(); - // CHECK-DAG: [[B4:%[a-zA-Z0-9_]+]] = alloca <4 x i1>, align 4 - // CHECK-DAG: [[CALL4:%[a-zA-Z0-9_]+]] = call spir_func <4 x i1> @[[FOO4]] - bar(b4); - // CHECK-DAG: call spir_func void @[[BAR4]](<4 x i1> [[B4_ARG:%[a-zA-Z0-9_]+]] - bool8 b8 = foo8(); - // CHECK-DAG: [[B8:%[a-zA-Z0-9_]+]] = alloca <8 x i1>, align 8 - // CHECK-DAG: [[CALL8:%[a-zA-Z0-9_]+]] = call spir_func <8 x i1> @[[FOO8]] - bar(b8); - // CHECK-DAG: call spir_func void @[[BAR8]](<8 x i1> [[B8_ARG:%[a-zA-Z0-9_]+]] - bool16 b16 = foo16(); - // CHECK-DAG: [[B16:%[a-zA-Z0-9_]+]] = alloca <16 x i1>, align 16 - // CHECK-DAG: [[CALL16:%[a-zA-Z0-9_]+]] = call spir_func <16 x i1> @[[FOO16]] - bar(b16); - // CHECK-DAG: call spir_func void @[[BAR16]](<16 x i1> [[B16_ARG:%[a-zA-Z0-9_]+]] - }); - return 0; -} diff --git a/clang/test/SemaSYCL/bool-vectors.cpp b/clang/test/SemaSYCL/bool-vectors.cpp deleted file mode 100644 index 0ffd77b5f9dfd..0000000000000 --- a/clang/test/SemaSYCL/bool-vectors.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsyntax-only -verify %s -// expected-no-diagnostics - -template -__attribute__((sycl_kernel)) void kernel(Func kernelFunc) { - kernelFunc(); -} -using bool1 = bool; -using bool2 = bool __attribute__((ext_vector_type(2))); -using bool3 = bool __attribute__((ext_vector_type(3))); -using bool4 = bool __attribute__((ext_vector_type(4))); -using bool8 = bool __attribute__((ext_vector_type(8))); -using bool16 = bool __attribute__((ext_vector_type(16))); - -int main() { - kernel( - [=]() { - bool1 b1; - bool2 b2; - bool3 b3; - bool4 b4; - bool8 b8; - bool16 b16; - }); - return 0; -} - diff --git a/sycl/include/CL/sycl/detail/boolean.hpp b/sycl/include/CL/sycl/detail/boolean.hpp index c7e76f1df33bc..849114ffcb440 100644 --- a/sycl/include/CL/sycl/detail/boolean.hpp +++ b/sycl/include/CL/sycl/detail/boolean.hpp @@ -28,7 +28,7 @@ template struct Assigner { static void init(R &r, const T x) { Assigner::template init(r, x); ET v = x.template swizzle(); - r.value[Num] = msbIsSet(v); + r.value[Num] = msbIsSet(v)*(-1); } }; @@ -39,7 +39,7 @@ template <> struct Assigner<0> { template 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); } }; @@ -47,7 +47,7 @@ template 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 struct Boolean { using DataType = element_type[N]; #endif - Boolean() : value{false} {} + Boolean() : value{0} {} Boolean(std::initializer_list l) { for (size_t I = 0; I < N; ++I) { - value[I] = *(l.begin() + I); + value[I] = (!!*(l.begin() + I))*(-1); } } @@ -70,6 +70,12 @@ template struct Boolean { } } + template Boolean(const T rhs) { + static_assert(is_vgeninteger::value, "Invalid constructor"); + Assigner::template init, 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,15 +84,7 @@ template struct Boolean { value[I] = rhs[I]; } } -#endif - - template Boolean(const T rhs) { - static_assert(is_vgeninteger::value, "Invalid constructor"); - Assigner::template init, T, typename T::element_type>( - *this, rhs); - } -#ifdef __SYCL_DEVICE_ONLY__ operator vector_t() const { return value; } #endif @@ -94,38 +92,29 @@ template struct Boolean { static_assert(is_vgeninteger::value, "Invalid conversion"); T r; Assigner::assign(r, *this); - return r * -1; + return r; } private: template friend struct Assigner; - alignas(detail::vector_alignment::value) DataType value; + alignas(detail::vector_alignment::value) DataType value; }; template <> struct Boolean<1> { - - using element_type = bool; - -#ifdef __SYCL_DEVICE_ONLY__ - using DataType = element_type; - using vector_t = DataType; -#else - using DataType = element_type; -#endif + using DataType = bool; Boolean() : value(false) {} Boolean(const Boolean &rhs) : value(rhs.value) {} -#ifdef __SYCL_DEVICE_ONLY__ - Boolean(const vector_t rhs) : value(rhs) {} -#endif - template Boolean(T val) : value(val) { static_assert(is_sgeninteger::value, "Invalid constructor"); } #ifdef __SYCL_DEVICE_ONLY__ + using vector_t = DataType; + Boolean(const vector_t rhs) : value(rhs) {} + operator vector_t() const { return value; } #endif diff --git a/sycl/include/CL/sycl/detail/generic_type_traits.hpp b/sycl/include/CL/sycl/detail/generic_type_traits.hpp index 522610ecb288a..f5e37295c8321 100644 --- 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 inline constexpr T msbMask(T) { using UT = make_unsigned_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 From f36747859d9baf488bff36ae6ab316f067a31a11 Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Sat, 28 Mar 2020 10:12:15 +0300 Subject: [PATCH 2/7] Fix formatting issues Signed-off-by: Alexey Bader --- sycl/include/CL/sycl/detail/boolean.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sycl/include/CL/sycl/detail/boolean.hpp b/sycl/include/CL/sycl/detail/boolean.hpp index 849114ffcb440..ccd3785ddfd03 100644 --- a/sycl/include/CL/sycl/detail/boolean.hpp +++ b/sycl/include/CL/sycl/detail/boolean.hpp @@ -28,7 +28,7 @@ template struct Assigner { static void init(R &r, const T x) { Assigner::template init(r, x); ET v = x.template swizzle(); - r.value[Num] = msbIsSet(v)*(-1); + r.value[Num] = msbIsSet(v) * (-1); } }; @@ -39,7 +39,7 @@ template <> struct Assigner<0> { template static void init(R &r, const T x) { ET v = x.template swizzle<0>(); - r.value[0] = msbIsSet(v)*(-1); + r.value[0] = msbIsSet(v) * (-1); } }; @@ -60,7 +60,7 @@ template struct Boolean { Boolean(std::initializer_list l) { for (size_t I = 0; I < N; ++I) { - value[I] = (!!*(l.begin() + I))*(-1); + value[I] = (!!*(l.begin() + I)) * (-1); } } From 1c0ccb93807dec6d74e3b04d39947e5a583e91ab Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Sat, 28 Mar 2020 10:13:10 +0300 Subject: [PATCH 3/7] Refactor Boolean<1> specialization Signed-off-by: Alexey Bader --- sycl/include/CL/sycl/detail/boolean.hpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/sycl/include/CL/sycl/detail/boolean.hpp b/sycl/include/CL/sycl/detail/boolean.hpp index ccd3785ddfd03..a8be040229f1c 100644 --- a/sycl/include/CL/sycl/detail/boolean.hpp +++ b/sycl/include/CL/sycl/detail/boolean.hpp @@ -101,30 +101,28 @@ template struct Boolean { }; template <> struct Boolean<1> { - using DataType = bool; - - Boolean() : value(false) {} - - Boolean(const Boolean &rhs) : value(rhs.value) {} + Boolean() = default; + // Build from a signed interger type template Boolean(T val) : value(val) { static_assert(is_sgeninteger::value, "Invalid constructor"); } -#ifdef __SYCL_DEVICE_ONLY__ - using vector_t = DataType; - Boolean(const vector_t rhs) : value(rhs) {} - - operator vector_t() const { return value; } -#endif - + // Cast to a signed interger type template operator T() const { static_assert(is_sgeninteger::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 From cb98ea02a62b31cfa135ac3b9cf60f5354cf262d Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Tue, 31 Mar 2020 13:26:51 +0300 Subject: [PATCH 4/7] Apply code review comment Signed-off-by: Alexey Bader --- sycl/include/CL/sycl/detail/boolean.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sycl/include/CL/sycl/detail/boolean.hpp b/sycl/include/CL/sycl/detail/boolean.hpp index a8be040229f1c..5f2c8776eca6d 100644 --- a/sycl/include/CL/sycl/detail/boolean.hpp +++ b/sycl/include/CL/sycl/detail/boolean.hpp @@ -60,7 +60,7 @@ template struct Boolean { Boolean(std::initializer_list l) { for (size_t I = 0; I < N; ++I) { - value[I] = (!!*(l.begin() + I)) * (-1); + value[I] = *(l.begin() + I) ? -1 : 0; } } From b244bbb4d89920ce1fd990e5efdf3ebfad3088ec Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Tue, 31 Mar 2020 13:50:10 +0300 Subject: [PATCH 5/7] Applied code review comments Signed-off-by: Alexey Bader --- clang/lib/Sema/SPIRVBuiltins.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SPIRVBuiltins.td b/clang/lib/Sema/SPIRVBuiltins.td index ec0701081e20f..20bbdc51a10d9 100644 --- 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, SChar, Short, UShort, +foreach Type = [Char, UChar, SChar, Short, UShort, Int, UInt, Long, ULong, Float, Double, Half] in { foreach VecSizes = [VecAndScalar, VecNoScalar] in { From bf2bdfa6f565ca63accf79d9f4ba8b45fcf4bea8 Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Tue, 31 Mar 2020 17:07:53 +0300 Subject: [PATCH 6/7] Update clang/lib/Sema/SPIRVBuiltins.td Signed-off-by: Alexey Bader alexey.bader@intel.com Co-Authored-By: Victor Lomuller --- clang/lib/Sema/SPIRVBuiltins.td | 3 --- 1 file changed, 3 deletions(-) diff --git a/clang/lib/Sema/SPIRVBuiltins.td b/clang/lib/Sema/SPIRVBuiltins.td index 20bbdc51a10d9..8b45c6c0f3e2d 100644 --- a/clang/lib/Sema/SPIRVBuiltins.td +++ b/clang/lib/Sema/SPIRVBuiltins.td @@ -817,9 +817,6 @@ foreach name = ["Dot"] in { foreach name = ["Any", "All"] in { def : SPVBuiltin; -} - -foreach name = ["Any", "All"] in { def : SPVBuiltin; } From 326379ca828c29e2ad302a097c7f7d89df20a929 Mon Sep 17 00:00:00 2001 From: Alexey Bader Date: Tue, 31 Mar 2020 13:56:17 +0300 Subject: [PATCH 7/7] Merge loops defining built-ins with bool argument. Signed-off-by: Alexey Bader --- clang/lib/Sema/SPIRVBuiltins.td | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/clang/lib/Sema/SPIRVBuiltins.td b/clang/lib/Sema/SPIRVBuiltins.td index 8b45c6c0f3e2d..4afdfd2716860 100644 --- a/clang/lib/Sema/SPIRVBuiltins.td +++ b/clang/lib/Sema/SPIRVBuiltins.td @@ -824,9 +824,6 @@ foreach name = ["IsNan", "IsInf", "IsFinite", "IsNormal", "SignBitSet"] in { def : SPVBuiltin; def : SPVBuiltin; def : SPVBuiltin; -} - -foreach name = ["IsNan", "IsInf", "IsFinite", "IsNormal", "SignBitSet"] in { def : SPVBuiltin; def : SPVBuiltin; def : SPVBuiltin; @@ -843,16 +840,6 @@ foreach name = ["LessOrGreater", def : SPVBuiltin; def : SPVBuiltin; def : SPVBuiltin; -} - -foreach name = ["LessOrGreater", - "Ordered", "Unordered", - "FOrdEqual", "FUnordEqual", - "FOrdNotEqual", "FUnordNotEqual", - "FOrdLessThan", "FUnordLessThan", - "FOrdGreaterThan", "FUnordGreaterThan", - "FOrdLessThanEqual", "FUnordLessThanEqual", - "FOrdGreaterThanEqual", "FUnordGreaterThanEqual"] in { def : SPVBuiltin; def : SPVBuiltin; def : SPVBuiltin;