From 84eedc88552fa3cdf51fdab57f7574bc956c794a Mon Sep 17 00:00:00 2001 From: Allison Vacanti Date: Fri, 6 Dec 2019 12:52:26 -0500 Subject: [PATCH] Make BinaryOperators/Predicates more flexible. Allow the argument types to differ. This allows ArrayPortalValueReferences to be used. --- vtkm/BinaryOperators.h | 101 ++++++++++++++---- vtkm/BinaryPredicates.h | 59 +++++----- vtkm/cont/testing/TestingDeviceAdapter.h | 6 +- vtkm/testing/UnitTestBinaryOperators.cxx | 25 ++--- .../worklet/cosmotools/CosmoToolsHaloFinder.h | 2 +- 5 files changed, 125 insertions(+), 68 deletions(-) diff --git a/vtkm/BinaryOperators.h b/vtkm/BinaryOperators.h index 99303c966..98c20a30f 100644 --- a/vtkm/BinaryOperators.h +++ b/vtkm/BinaryOperators.h @@ -29,26 +29,48 @@ namespace vtkm /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns sum (addition) of the two values. -/// Note: Requires Type \p T implement the + operator. +/// @note Requires a suitable definition of `operator+(T, U)`. struct Sum { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT auto operator()(const T& x, const U& y) const -> decltype(x + y) { return x + y; } + + // If both types are the same integral type, explicitly cast the result to + // type T to avoid narrowing conversion warnings from operations that promote + // to int (e.g. `int operator+(char, char)`) + template + VTKM_EXEC_CONT + typename std::enable_if::value && sizeof(T) < sizeof(int), T>::type + operator()(const T& x, const T& y) const + { + return static_cast(x + y); + } }; /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns product (multiplication) of the two values. -/// Note: Requires Type \p T implement the * operator. +/// @note Requires a suitable definition of `operator*(T, U)`. struct Product { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT auto operator()(const T& x, const U& y) const -> decltype(x * y) { return x * y; } + + // If both types are the same integral type, explicitly cast the result to + // type T to avoid narrowing conversion warnings from operations that promote + // to int (e.g. `int operator+(char, char)`) + template + VTKM_EXEC_CONT + typename std::enable_if::value && sizeof(T) < sizeof(int), T>::type + operator()(const T& x, const T& y) const + { + return static_cast(x * y); + } }; #if (defined(VTKM_GCC) || defined(VTKM_CLANG)) @@ -57,12 +79,13 @@ struct Product /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns the \c x if x > y otherwise returns \c y. -/// Note: Requires Type \p T implement the < operator. +/// @note Requires a suitable definition of `bool operator<(T, U)` and that +/// `T` and `U` share a common type. //needs to be full length to not clash with vtkm::math function Max. struct Maximum { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT typename std::common_type::type operator()(const T& x, const U& y) const { return x < y ? y : x; } @@ -70,19 +93,20 @@ struct Maximum /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns the \c x if x < y otherwise returns \c y. -/// Note: Requires Type \p T implement the < operator. +/// @note Requires a suitable definition of `bool operator<(T, U)` and that +/// `T` and `U` share a common type. //needs to be full length to not clash with vtkm::math function Min. struct Minimum { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT typename std::common_type::type operator()(const T& x, const U& y) const { return x < y ? x : y; } }; /// Binary Predicate that takes two arguments argument \c x, and \c y and -/// returns a vtkm::Vec that represents the minimum and maximum values +/// returns a vtkm::Vec that represents the minimum and maximum values. /// Note: Requires Type \p T implement the vtkm::Min and vtkm::Max functions. template struct MinAndMax @@ -117,38 +141,71 @@ struct MinAndMax /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns the bitwise operation x&y -/// Note: Requires Type \p T implement the & operator. +/// @note Requires a suitable definition of `operator&(T, U)`. struct BitwiseAnd { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT auto operator()(const T& x, const U& y) const -> decltype(x & y) { return x & y; } + + // If both types are the same integral type, explicitly cast the result to + // type T to avoid narrowing conversion warnings from operations that promote + // to int (e.g. `int operator+(char, char)`) + template + VTKM_EXEC_CONT + typename std::enable_if::value && sizeof(T) < sizeof(int), T>::type + operator()(const T& x, const T& y) const + { + return static_cast(x & y); + } }; /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns the bitwise operation x|y -/// Note: Requires Type \p T implement the | operator. +/// @note Requires a suitable definition of `operator&(T, U)`. struct BitwiseOr { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT auto operator()(const T& x, const U& y) const -> decltype(x | y) { return x | y; } + + // If both types are the same integral type, explicitly cast the result to + // type T to avoid narrowing conversion warnings from operations that promote + // to int (e.g. `int operator+(char, char)`) + template + VTKM_EXEC_CONT + typename std::enable_if::value && sizeof(T) < sizeof(int), T>::type + operator()(const T& x, const T& y) const + { + return static_cast(x | y); + } }; /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns the bitwise operation x^y -/// Note: Requires Type \p T implement the ^ operator. +/// @note Requires a suitable definition of `operator&(T, U)`. struct BitwiseXor { - template - VTKM_EXEC_CONT T operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT auto operator()(const T& x, const U& y) const -> decltype(x ^ y) { return x ^ y; } + + // If both types are the same integral type, explicitly cast the result to + // type T to avoid narrowing conversion warnings from operations that promote + // to int (e.g. `int operator+(char, char)`) + template + VTKM_EXEC_CONT + typename std::enable_if::value && sizeof(T) < sizeof(int), T>::type + operator()(const T& x, const T& y) const + { + return static_cast(x ^ y); + } }; } // namespace vtkm diff --git a/vtkm/BinaryPredicates.h b/vtkm/BinaryPredicates.h index 2d9098b71..29b4e5353 100644 --- a/vtkm/BinaryPredicates.h +++ b/vtkm/BinaryPredicates.h @@ -15,37 +15,37 @@ namespace vtkm { -/// Binary Predicate that takes two arguments argument \c x, and \c y and -/// returns True if and only if \c x is equal to \c y. -/// Note: Requires Type \p T implement the == operator. -struct Equal -{ - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const - { - return x == y; - } -}; - /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns True if and only if \c x is not equal to \c y. -/// Note: Requires Type \p T implement the != operator. +/// @note: Requires that types T and U are comparable with !=. struct NotEqual { - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const { return x != y; } }; +/// Binary Predicate that takes two arguments argument \c x, and \c y and +/// returns True if and only if \c x is equal to \c y. +/// @note: Requires that types T and U are comparable with !=. +struct Equal +{ + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const + { + return x == y; + } +}; + /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns True if and only if \c x is less than \c y. -/// Note: Requires Type \p T implement the < operator. +/// @note: Requires that types T and U are comparable with <. struct SortLess { - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const { return x < y; } @@ -53,12 +53,12 @@ struct SortLess /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns True if and only if \c x is greater than \c y. -/// Note: Requires Type \p T implement the < operator, as we invert the -/// comparison +/// @note: Requires that types T and U are comparable via operator<(U, T). + struct SortGreater { - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const { return y < x; } @@ -66,12 +66,12 @@ struct SortGreater /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns True if and only if \c x and \c y are True. -/// Note: Requires Type \p T to be convertible to \c bool or implement the -/// && operator. +/// @note: Requires that types T and U are comparable with &&. + struct LogicalAnd { - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const { return x && y; } @@ -79,12 +79,11 @@ struct LogicalAnd /// Binary Predicate that takes two arguments argument \c x, and \c y and /// returns True if and only if \c x or \c y is True. -/// Note: Requires Type \p T to be convertible to \c bool or implement the -/// || operator. +/// @note: Requires that types T and U are comparable with ||. struct LogicalOr { - template - VTKM_EXEC_CONT bool operator()(const T& x, const T& y) const + template + VTKM_EXEC_CONT bool operator()(const T& x, const U& y) const { return x || y; } diff --git a/vtkm/cont/testing/TestingDeviceAdapter.h b/vtkm/cont/testing/TestingDeviceAdapter.h index 8a37101fc..71c7d2801 100644 --- a/vtkm/cont/testing/TestingDeviceAdapter.h +++ b/vtkm/cont/testing/TestingDeviceAdapter.h @@ -1293,7 +1293,7 @@ private: "Got bad value from Reduce with pair comparison object"); - std::cout << " Reduce bool array with vtkm::BitwiseAnd to see if all values are true." + std::cout << " Reduce bool array with vtkm::LogicalAnd to see if all values are true." << std::endl; //construct an array of bools and verify that they aren't all true constexpr vtkm::Id inputLength = 60; @@ -1304,8 +1304,8 @@ private: true, true, true, true, true, true, true, true, true, true, true, true, true, true, true }; auto barray = vtkm::cont::make_ArrayHandle(inputValues, inputLength); - bool all_true = Algorithm::Reduce(barray, true, vtkm::BitwiseAnd()); - VTKM_TEST_ASSERT(all_true == false, "reduction with vtkm::BitwiseAnd should return false"); + bool all_true = Algorithm::Reduce(barray, true, vtkm::LogicalAnd()); + VTKM_TEST_ASSERT(all_true == false, "reduction with vtkm::LogicalAnd should return false"); std::cout << " Reduce with custom value type and custom comparison operator." << std::endl; //test with a custom value type with the reduction value being a vtkm::Vec diff --git a/vtkm/testing/UnitTestBinaryOperators.cxx b/vtkm/testing/UnitTestBinaryOperators.cxx index 993f963d4..9537f2742 100644 --- a/vtkm/testing/UnitTestBinaryOperators.cxx +++ b/vtkm/testing/UnitTestBinaryOperators.cxx @@ -136,31 +136,32 @@ void TestBinaryOperators() { vtkm::testing::Testing::TryTypes(BinaryOperatorTestFunctor()); + vtkm::UInt32 v1 = 0xccccccccu; + vtkm::UInt32 v2 = 0xffffffffu; + vtkm::UInt32 v3 = 0x0u; + //test BitwiseAnd { vtkm::BitwiseAnd bitwise_and; - VTKM_TEST_ASSERT(bitwise_and(true, true) == true, "bitwise_and true wrong."); - VTKM_TEST_ASSERT(bitwise_and(true, false) == false, "bitwise_and true wrong."); - VTKM_TEST_ASSERT(bitwise_and(false, true) == false, "bitwise_and true wrong."); - VTKM_TEST_ASSERT(bitwise_and(false, false) == false, "bitwise_and true wrong."); + VTKM_TEST_ASSERT(bitwise_and(v1, v2) == (v1 & v2), "bitwise_and wrong."); + VTKM_TEST_ASSERT(bitwise_and(v1, v3) == (v1 & v3), "bitwise_and wrong."); + VTKM_TEST_ASSERT(bitwise_and(v2, v3) == (v2 & v3), "bitwise_and wrong."); } //test BitwiseOr { vtkm::BitwiseOr bitwise_or; - VTKM_TEST_ASSERT(bitwise_or(true, true) == true, "bitwise_or true wrong."); - VTKM_TEST_ASSERT(bitwise_or(true, false) == true, "bitwise_or true wrong."); - VTKM_TEST_ASSERT(bitwise_or(false, true) == true, "bitwise_or true wrong."); - VTKM_TEST_ASSERT(bitwise_or(false, false) == false, "bitwise_or true wrong."); + VTKM_TEST_ASSERT(bitwise_or(v1, v2) == (v1 | v2), "bitwise_or wrong."); + VTKM_TEST_ASSERT(bitwise_or(v1, v3) == (v1 | v3), "bitwise_or wrong."); + VTKM_TEST_ASSERT(bitwise_or(v2, v3) == (v2 | v3), "bitwise_or wrong."); } //test BitwiseXor { vtkm::BitwiseXor bitwise_xor; - VTKM_TEST_ASSERT(bitwise_xor(true, true) == false, "bitwise_xor true wrong."); - VTKM_TEST_ASSERT(bitwise_xor(true, false) == true, "bitwise_xor true wrong."); - VTKM_TEST_ASSERT(bitwise_xor(false, true) == true, "bitwise_xor true wrong."); - VTKM_TEST_ASSERT(bitwise_xor(false, false) == false, "bitwise_xor true wrong."); + VTKM_TEST_ASSERT(bitwise_xor(v1, v2) == (v1 ^ v2), "bitwise_xor wrong."); + VTKM_TEST_ASSERT(bitwise_xor(v1, v3) == (v1 ^ v3), "bitwise_xor wrong."); + VTKM_TEST_ASSERT(bitwise_xor(v2, v3) == (v2 ^ v3), "bitwise_xor wrong."); } } diff --git a/vtkm/worklet/cosmotools/CosmoToolsHaloFinder.h b/vtkm/worklet/cosmotools/CosmoToolsHaloFinder.h index 9f852cb53..819fe981d 100644 --- a/vtkm/worklet/cosmotools/CosmoToolsHaloFinder.h +++ b/vtkm/worklet/cosmotools/CosmoToolsHaloFinder.h @@ -152,7 +152,7 @@ void CosmoTools::HaloFinder(vtkm::cont::ArrayHandle& r rootedStar); // output (whole array) // If all vertices are in rooted stars, algorithm is complete - bool allStars = DeviceAlgorithm::Reduce(rootedStar, true, vtkm::BitwiseAnd()); + bool allStars = DeviceAlgorithm::Reduce(rootedStar, true, vtkm::LogicalAnd()); if (allStars) { break;