Skip to content

Commit

Permalink
Fix Vc::iif for builtin types and non-default SimdArray
Browse files Browse the repository at this point in the history
* The static assertion broke builtin types and SFINAE. Remove the
static_assert and use a deleted function instead (to express that this
is intended).
* Add two template parameters to conditional_assign for SimdArray to
enable non-default SimdArray types.
* Add a unit test for the builtin types use if Vc::iif
* Add tests that SFINAE behaves.

Fixes: gh-90
Signed-off-by: Matthias Kretz <kretz@kde.org>
  • Loading branch information
mattkretz committed Dec 1, 2015
1 parent c37b4d2 commit 2e2d4c8
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 34 deletions.
37 changes: 9 additions & 28 deletions common/iif.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

namespace Vc_VERSIONED_NAMESPACE
{

namespace
{
template<typename T> struct assert_for_iif
{
typedef T type;
static_assert(Vc::is_simd_vector<T>::value, "Incorrect use of Vc::iif. If you use a mask as first parameter, the second and third parameters must be of vector type.");
};
} // anonymous namespace

/**
* \ingroup Utilities
*
Expand All @@ -67,31 +57,22 @@ namespace
* Assuming \c a has the values [0, 3, 5, 1], \c b is [1, 1, 1, 1], and \c c is [1, 2, 3, 4], then x
* will be [2, 2, 3, 5].
*/
template<typename Mask, typename T> Vc_ALWAYS_INLINE
typename std::enable_if<Vc::is_simd_mask<Mask>::value, typename assert_for_iif<T>::type>::type
iif(const Mask &condition, const T &trueValue, const T &falseValue)
template <typename Mask, typename T>
Vc_ALWAYS_INLINE enable_if<is_simd_mask<Mask>::value && is_simd_vector<T>::value, T> iif(
const Mask &condition, const T &trueValue, const T &falseValue)
{
T result(falseValue);
Vc::where(condition) | result = trueValue;
return result;
}

/* the following might be a nice shortcut in some cases, but:
* 1. it fails if there are different vector classes for the same T
* 2. the semantics are a bit fishy: basically the mask determines how to blow up the scalar values
template<typename Mask, typename T> Vc_ALWAYS_INLINE
typename std::enable_if<Vc::is_simd_mask<Mask>::value && !Vc::is_simd_vector<T>::value, void>::type
#ifndef Vc_MSVC
iif(Mask condition, T trueValue, T falseValue)
#else
iif(const Mask &condition, T trueValue, T falseValue)
#endif
{
Vc::Vector<T> f = falseValue;
Vc::where(condition) | f = trueValue;
return f;
}
/**\internal
* The following declaration makes it explicit that `iif (Mask, non-vector, non-vector)`
* is not supposed to work. Doing the same thing with \c static_assert would break SFINAE.
*/
template <typename Mask, typename T>
enable_if<is_simd_mask<Mask>::value && !is_simd_vector<T>::value, T> iif(
const Mask &, const T &, const T &) = delete;

/**
* \ingroup Utilities
Expand Down
11 changes: 6 additions & 5 deletions common/simdarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -1911,9 +1911,10 @@ Vc_BINARY_FUNCTION__(max)
} // namespace internal
// conditional_assign {{{1
#define Vc_CONDITIONAL_ASSIGN(name__, op__) \
template <Operator O, typename T, std::size_t N, typename M, typename U> \
template <Operator O, typename T, std::size_t N, typename V, size_t VN, typename M, \
typename U> \
Vc_INTRINSIC enable_if<O == Operator::name__, void> conditional_assign( \
SimdArray<T, N> &lhs, M &&mask, U &&rhs) \
SimdArray<T, N, V, VN> &lhs, M &&mask, U &&rhs) \
{ \
lhs(mask) op__ rhs; \
}
Expand All @@ -1931,9 +1932,9 @@ Vc_CONDITIONAL_ASSIGN(RightShiftAssign,>>=)
#undef Vc_CONDITIONAL_ASSIGN

#define Vc_CONDITIONAL_ASSIGN(name__, expr__) \
template <Operator O, typename T, std::size_t N, typename M> \
Vc_INTRINSIC enable_if<O == Operator::name__, SimdArray<T, N>> conditional_assign( \
SimdArray<T, N> &lhs, M &&mask) \
template <Operator O, typename T, std::size_t N, typename V, size_t VN, typename M> \
Vc_INTRINSIC enable_if<O == Operator::name__, SimdArray<T, N, V, VN>> \
conditional_assign(SimdArray<T, N, V, VN> &lhs, M &&mask) \
{ \
return expr__; \
}
Expand Down
31 changes: 30 additions & 1 deletion tests/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,23 @@ TEST(testMallocAlignment)
COMPARE((reinterpret_cast<std::uintptr_t>(&a[0]) & mask), 0ul);
}

TEST_TYPES(V, testIif, (ALL_VECTORS))
template <typename A, typename B, typename C,
typename = decltype(Vc::iif(std::declval<A>(), std::declval<B>(),
std::declval<C>()))>
inline void sfinaeIifIsNotCallable(A &&, B &&, C &&, int)
{
FAIL() << "iif(" << UnitTest::typeToString<A>() << UnitTest::typeToString<B>()
<< UnitTest::typeToString<C>() << ") appears to be callable.";
}

template <typename A, typename B, typename C>
inline void sfinaeIifIsNotCallable(A &&, B &&, C &&, ...)
{
// passes
}

TEST_TYPES(V, testIif,
(ALL_VECTORS, SIMD_ARRAYS(31), Vc::SimdArray<float, 8, Vc::Scalar::float_v>))
{
typedef typename V::EntryType T;
const T one = T(1);
Expand All @@ -338,9 +354,22 @@ TEST_TYPES(V, testIif, (ALL_VECTORS))
}
COMPARE(iif (x > y, x, y), reference);
COMPARE(iif (x > y, V(one), V(two)), reference2);
sfinaeIifIsNotCallable(
x > y, int(), float(),
int()); // mismatching true/false value types should never work
sfinaeIifIsNotCallable(
x > y, int(), int(),
int()); // iif(mask, scalar, scalar) should not appear usable
}
}

TEST(testIifBuiltin)
{
COMPARE(Vc::iif(true, 1, 2), true ? 1 : 2);
COMPARE(Vc::iif(false, 1, 2), false ? 1 : 2);
sfinaeIifIsNotCallable(bool(), int(), float(), int());
}

TEST_TYPES(V, rangeFor, (ALL_VECTORS))
{
typedef typename V::EntryType T;
Expand Down

0 comments on commit 2e2d4c8

Please sign in to comment.