Skip to content

Commit

Permalink
Deprecate explicit Vector & Mask conversion ctors
Browse files Browse the repository at this point in the history
Code such as

    float_v indexes(int_v::IndexesFromZero());

compiled with Vc 0.7 and did the right thing. Since Vc 1.0 the code
still compiles but might generated zeros (with AVX) or drop data.
Sometimes the use of an explicit constructor might just follow from
coding style preference to use ctor syntax over assignment syntax. It
therefore is a safer choice to only allow explicit casts via `simd_cast`
and deprecate the explicit conversion constructors.

The code is #ifdefed such that it will disappear automatically starting
with Vc 1.70 (pre-alpha 2.0).

Fixes: gh-121
Signed-off-by: Matthias Kretz <kretz@kde.org>
  • Loading branch information
mattkretz committed Apr 11, 2016
1 parent ae7e5a8 commit b11e617
Show file tree
Hide file tree
Showing 24 changed files with 118 additions and 73 deletions.
9 changes: 6 additions & 3 deletions avx/mask.h
Expand Up @@ -129,11 +129,14 @@ template <typename T> class Mask<T, VectorAbi::Avx>
{
}

#if Vc_IS_VERSION_1
// explicit cast, implemented via simd_cast (in avx/simd_cast_caller.h)
template <typename U>
Vc_INTRINSIC explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> =
nullarg);
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"mask types") Vc_INTRINSIC
explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> = nullarg);
#endif

template<typename Flags = DefaultLoadTag> Vc_INTRINSIC explicit Mask(const bool *mem, Flags f = Flags()) { load(mem, f); }

Expand Down
8 changes: 4 additions & 4 deletions avx/math.h
Expand Up @@ -171,7 +171,7 @@ inline AVX2::double_v frexp(AVX2::double_v::AsArg v, SimdArray<int, 4, SSE::int_
_mm256_broadcast_sd(reinterpret_cast<const double *>(&AVX::c_general::frexpMask)));
const double_m zeroMask = v == AVX2::double_v::Zero();
ret(isnan(v) || !isfinite(v) || zeroMask) = v;
exponent.setZero(static_cast<SSE::int_m>(zeroMask));
exponent.setZero(simd_cast<SSE::int_m>(zeroMask));
internal_data(*e) = exponent;
return ret;
}
Expand Down Expand Up @@ -199,7 +199,7 @@ inline AVX2::float_v frexp(AVX2::float_v::AsArg v, AVX2::float_v::IndexType *e)
const __m256 exponentMaximized = or_(v.data(), exponentBits);
float_v ret = _mm256_and_ps(exponentMaximized, avx_cast<__m256>(set1_epi32(0xbf7fffffu)));
ret(isnan(v) || !isfinite(v) || v == float_v::Zero()) = v;
e->setZero(static_cast<decltype(*e == *e)>(v == float_v::Zero()));
e->setZero(simd_cast<decltype(*e == *e)>(v == float_v::Zero()));
return ret;
}

Expand All @@ -212,7 +212,7 @@ inline AVX2::double_v ldexp(AVX2::double_v::AsArg v,
const SimdArray<int, 4, SSE::int_v, 4> &_e)
{
SSE::int_v e = internal_data(_e);
e.setZero(SSE::int_m{v == AVX2::double_v::Zero()});
e.setZero(simd_cast<SSE::int_m>(v == AVX2::double_v::Zero()));
const __m256i exponentBits =
AVX::concat(_mm_slli_epi64(_mm_unpacklo_epi32(e.data(), e.data()), 52),
_mm_slli_epi64(_mm_unpackhi_epi32(e.data(), e.data()), 52));
Expand All @@ -221,7 +221,7 @@ inline AVX2::double_v ldexp(AVX2::double_v::AsArg v,
}
inline AVX2::float_v ldexp(AVX2::float_v::AsArg v, SimdArray<int, 8, SSE::int_v, 4> e)
{
e.setZero(static_cast<decltype(e == e)>(v == AVX2::float_v::Zero()));
e.setZero(simd_cast<decltype(e == e)>(v == AVX2::float_v::Zero()));
e <<= 23;
return {AVX::avx_cast<__m256>(
AVX::concat(_mm_add_epi32(AVX::avx_cast<__m128i>(AVX::lo128(v.data())),
Expand Down
2 changes: 2 additions & 0 deletions avx/simd_cast_caller.tcc
Expand Up @@ -32,6 +32,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

namespace Vc_VERSIONED_NAMESPACE
{
#if Vc_IS_VERSION_1
template <typename T>
template <typename U, typename>
Vc_INTRINSIC Vector<T, VectorAbi::Avx>::Vector(U &&x)
Expand All @@ -46,6 +47,7 @@ Vc_INTRINSIC Mask<T, VectorAbi::Avx>::Mask(U &&rhs,
: Mask(simd_cast<Mask>(std::forward<U>(rhs)))
{
}
#endif // Vc_IS_VERSION_1
}

#endif // Vc_AVX_SIMD_CAST_CALLER_TCC_
Expand Down
9 changes: 7 additions & 2 deletions avx/vector.h
Expand Up @@ -157,9 +157,11 @@ template <typename T> class Vector<T, VectorAbi::Avx>
{
}

#if Vc_IS_VERSION_1
// static_cast from the remaining Vector<U, abi>
template <typename U>
Vc_INTRINSIC explicit Vector(
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"vector types") Vc_INTRINSIC explicit Vector(
Vc_ALIGNED_PARAMETER(V<U>) x,
typename std::enable_if<!Traits::is_implicit_cast_allowed<U, T>::value,
void *>::type = nullptr)
Expand All @@ -172,7 +174,10 @@ template <typename T> class Vector<T, VectorAbi::Avx>
template <typename U,
typename = enable_if<Traits::is_simd_vector<U>::value &&
!std::is_same<Vector, Traits::decay<U>>::value>>
Vc_INTRINSIC_L explicit Vector(U &&x) Vc_INTRINSIC_R;
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"vector types") Vc_INTRINSIC_L
explicit Vector(U &&x) Vc_INTRINSIC_R;
#endif

///////////////////////////////////////////////////////////////////////////////////////////
// broadcast
Expand Down
10 changes: 7 additions & 3 deletions common/mask.h
Expand Up @@ -164,6 +164,7 @@ template <typename T, typename Abi = VectorAbi::Best<T>> class Mask
Vc_INTRINSIC Mask(U &&otherMask,
Common::enable_if_mask_converts_implicitly<T, U> = nullarg);

#if Vc_IS_VERSION_1
/**
* Explicit conversion (static_cast) from a mask object that potentially has a
* different \VSize{T}.
Expand All @@ -173,10 +174,13 @@ template <typename T, typename Abi = VectorAbi::Best<T>> class Mask
* \internal This is implemented via simd_cast in scalar/simd_cast_caller.h
*/
template <typename U>
Vc_INTRINSIC_L explicit Mask(
U &&otherMask,
Common::enable_if_mask_converts_explicitly<T, U> = nullarg) Vc_INTRINSIC_R;
Vc_DEPRECATED(
"use simd_cast instead of explicit type casting to convert between mask types")
Vc_INTRINSIC_L
explicit Mask(U &&otherMask, Common::enable_if_mask_converts_explicitly<T, U> =
nullarg) Vc_INTRINSIC_R;
///@}
#endif

/**
* \name Loads & Stores
Expand Down
5 changes: 4 additions & 1 deletion common/vector.h
Expand Up @@ -286,6 +286,7 @@ template<typename T, typename Abi = VectorAbi::Best<T>> class Vector
inline Vector(Vector<U, abi> x,
enable_if<Traits::is_implicit_cast_allowed<U, T>::value> = nullarg);

#if Vc_IS_VERSION_1
/**
* Explicit conversion (i.e. `static_cast`) from the remaining Vector<U, Abi> types.
*
Expand All @@ -297,9 +298,11 @@ template<typename T, typename Abi = VectorAbi::Best<T>> class Vector
* fundamental arithmetic types.
*/
template <typename U>
inline explicit Vector(
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"vector types") inline explicit Vector(
Vector<U, abi> x,
enable_if<!Traits::is_implicit_cast_allowed<U, T>::value> = nullarg);
#endif

/**
* Broadcast Constructor.
Expand Down
12 changes: 8 additions & 4 deletions mic/mask.h
Expand Up @@ -134,11 +134,15 @@ template <typename T> class Mask<T, VectorAbi::Mic>
Vc_INTRINSIC Mask(U &&rhs, Common::enable_if_mask_converts_implicitly<T, U> = nullarg)
: k(MaskHelper<Size>::cast(rhs.data())) {}

// explicit cast, implemented via simd_cast (in scalar/simd_cast_caller.h)
#if Vc_IS_VERSION_1
// explicit cast, implemented via simd_cast (in mic/simd_cast_caller.h)
template <typename U>
Vc_INTRINSIC_L explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> =
nullarg) Vc_INTRINSIC_R;
Vc_DEPRECATED(
"use simd_cast instead of explicit type casting to convert between mask types")
Vc_INTRINSIC_L
explicit Mask(U &&rhs, Common::enable_if_mask_converts_explicitly<T, U> = nullarg)
Vc_INTRINSIC_R;
#endif

inline explicit Mask(const bool *mem) { load(mem, Aligned); }
template<typename Flags>
Expand Down
2 changes: 2 additions & 0 deletions mic/simd_cast_caller.tcc
Expand Up @@ -32,13 +32,15 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

namespace Vc_VERSIONED_NAMESPACE
{
#if Vc_IS_VERSION_1
template <typename T>
template <typename U>
Vc_INTRINSIC Mask<T, VectorAbi::Mic>::Mask(
U &&rhs, Common::enable_if_mask_converts_explicitly<T, U>)
: Mask(simd_cast<Mask>(std::forward<U>(rhs)))
{
}
#endif
} // namespace Vc

#endif // Vc_MIC_SIMD_CAST_CALLER_TCC_
Expand Down
12 changes: 8 additions & 4 deletions mic/vector.h
Expand Up @@ -140,15 +140,19 @@ class Vector<T, VectorAbi::Mic> : public MIC::StoreMixin<MIC::Vector<T>, T>
{
}

#if Vc_IS_VERSION_1
// static_cast from the remaining Vector<U>
template <typename U>
Vc_INTRINSIC explicit Vector(
Vc_ALIGNED_PARAMETER(Vector<U>) x,
typename std::enable_if<!Traits::is_implicit_cast_allowed<U, T>::value,
void *>::type = nullptr)
Vc_DEPRECATED("use simd_cast instead of explicit type casting to "
"convert between vector types") Vc_INTRINSIC
explicit Vector(
Vc_ALIGNED_PARAMETER(Vector<U>) x,
typename std::enable_if<!Traits::is_implicit_cast_allowed<U, T>::value,
void *>::type = nullptr)
: d(MIC::convert<U, T>(x.data()))
{
}
#endif

///////////////////////////////////////////////////////////////////////////////////////////
// broadcast
Expand Down
10 changes: 7 additions & 3 deletions scalar/mask.h
Expand Up @@ -86,11 +86,15 @@ template <typename T> class Mask<T, VectorAbi::Scalar>
Vc_INTRINSIC Mask(U &&rhs, Common::enable_if_mask_converts_implicitly<T, U> = nullarg)
: m(rhs.m) {}

#if Vc_IS_VERSION_1
// explicit cast, implemented via simd_cast (in scalar/simd_cast_caller.h)
template <typename U>
Vc_INTRINSIC_L explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> =
nullarg) Vc_INTRINSIC_R;
Vc_DEPRECATED(
"use simd_cast instead of explicit type casting to convert between mask types")
Vc_INTRINSIC_L
explicit Mask(U &&rhs, Common::enable_if_mask_converts_explicitly<T, U> = nullarg)
Vc_INTRINSIC_R;
#endif

Vc_ALWAYS_INLINE explicit Mask(const bool *mem) : m(mem[0]) {}
template<typename Flags> Vc_ALWAYS_INLINE explicit Mask(const bool *mem, Flags) : m(mem[0]) {}
Expand Down
2 changes: 2 additions & 0 deletions scalar/simd_cast_caller.tcc
Expand Up @@ -32,13 +32,15 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

namespace Vc_VERSIONED_NAMESPACE
{
#if Vc_IS_VERSION_1
template <typename T>
template <typename U>
Vc_INTRINSIC Mask<T, VectorAbi::Scalar>::Mask(
U &&rhs, Common::enable_if_mask_converts_explicitly<T, U>)
: Mask(simd_cast<Mask>(std::forward<U>(rhs)))
{
}
#endif
} // namespace Vc

#endif // Vc_SCALAR_SIMD_CAST_CALLER_TCC_
Expand Down
5 changes: 4 additions & 1 deletion scalar/vector.h
Expand Up @@ -90,15 +90,18 @@ template <typename T> class Vector<T, VectorAbi::Scalar>
{
}

#if Vc_IS_VERSION_1
// static_cast from the remaining Vector<U, abi>
template <typename U>
Vc_INTRINSIC explicit Vector(
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"vector types") Vc_INTRINSIC explicit Vector(
Vc_ALIGNED_PARAMETER(V<U>) x,
typename std::enable_if<!Traits::is_implicit_cast_allowed<U, T>::value,
void *>::type = nullptr)
: m_data(static_cast<EntryType>(x.data()))
{
}
#endif

///////////////////////////////////////////////////////////////////////////////////////////
// broadcast
Expand Down
40 changes: 20 additions & 20 deletions src/trigonometric.cpp
Expand Up @@ -109,13 +109,13 @@ static Vc_ALWAYS_INLINE Vector<_T, Abi> foldInput(const Vector<_T, Abi> &_x, IV

const V x = abs(_x);
#if defined(Vc_IMPL_FMA4) || defined(Vc_IMPL_FMA)
quadrant = static_cast<IV>(x * C::_4_pi() + V::One()); // prefer the fma here
quadrant = simd_cast<IV>(x * C::_4_pi() + V::One()); // prefer the fma here
quadrant &= ~IV::One();
#else
quadrant = static_cast<IV>(x * C::_4_pi());
quadrant = simd_cast<IV>(x * C::_4_pi());
quadrant += quadrant & IV::One();
#endif
const V y = static_cast<V>(quadrant);
const V y = simd_cast<V>(quadrant);
quadrant &= 7;

return ((x - y * C::_pi_4_hi()) - y * C::_pi_4_rem1()) - y * C::_pi_4_rem2();
Expand All @@ -133,7 +133,7 @@ static Vc_ALWAYS_INLINE double_v<Abi> foldInput(const double_v<Abi> &_x,
quadrant = simd_cast<double_int_v<Abi>>(z);
int_m mask = (quadrant & double_int_v<Abi>::One()) != double_int_v<Abi>::Zero();
++quadrant(mask);
y(static_cast<double_m>(mask)) += V::One();
y(simd_cast<double_m>(mask)) += V::One();
quadrant &= 7;

// since y is an integer we don't need to split y into low and high parts until the integer
Expand Down Expand Up @@ -165,7 +165,7 @@ template<> template<typename V> V Trigonometric<Vc::Detail::TrigonometricImpleme

IV quadrant;
const V z = foldInput(_x, quadrant);
const M sign = (_x < V::Zero()) ^ static_cast<M>(quadrant > 3);
const M sign = (_x < V::Zero()) ^ simd_cast<M>(quadrant > 3);
quadrant(quadrant > 3) -= 4;

V y = sinSeries(z);
Expand All @@ -182,11 +182,11 @@ template<> template<> Vc::double_v Trigonometric<Vc::Detail::TrigonometricImplem
double_int_v<V::abi> quadrant;
M sign = _x < V::Zero();
const V x = foldInput(_x, quadrant);
sign ^= static_cast<M>(quadrant > 3);
sign ^= simd_cast<M>(quadrant > 3);
quadrant(quadrant > 3) -= 4;

V y = sinSeries(x);
y(static_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = cosSeries(x);
y(simd_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = cosSeries(x);
y(sign) = -y;
return y;
}
Expand All @@ -212,12 +212,12 @@ template<> template<> Vc::double_v Trigonometric<Vc::Detail::TrigonometricImplem

double_int_v<V::abi> quadrant;
const V x = foldInput(_x, quadrant);
M sign = static_cast<M>(quadrant > 3);
M sign = simd_cast<M>(quadrant > 3);
quadrant(quadrant > 3) -= 4;
sign ^= static_cast<M>(quadrant > double_int_v<V::abi>::One());
sign ^= simd_cast<M>(quadrant > double_int_v<V::abi>::One());

V y = cosSeries(x);
y(static_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = sinSeries(x);
y(simd_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = sinSeries(x);
y(sign) = -y;
return y;
}
Expand All @@ -227,20 +227,20 @@ template<> template<typename V> void Trigonometric<Vc::Detail::TrigonometricImpl

IV quadrant;
const V x = foldInput(_x, quadrant);
M sign = static_cast<M>(quadrant > 3);
M sign = simd_cast<M>(quadrant > 3);
quadrant(quadrant > 3) -= 4;

const V cos_s = cosSeries(x);
const V sin_s = sinSeries(x);

V c = cos_s;
c(static_cast<M>(quadrant == IV::One() || quadrant == 2)) = sin_s;
c(sign ^ static_cast<M>(quadrant > IV::One())) = -c;
c(simd_cast<M>(quadrant == IV::One() || quadrant == 2)) = sin_s;
c(sign ^ simd_cast<M>(quadrant > IV::One())) = -c;
*_cos = c;

V s = sin_s;
s(static_cast<M>(quadrant == IV::One() || quadrant == 2)) = cos_s;
s(sign ^ static_cast<M>(_x < V::Zero())) = -s;
s(simd_cast<M>(quadrant == IV::One() || quadrant == 2)) = cos_s;
s(sign ^ simd_cast<M>(_x < V::Zero())) = -s;
*_sin = s;
}
template<> template<> void Trigonometric<Vc::Detail::TrigonometricImplementation<Vc::CurrentImplementation::current()>>::sincos(const Vc::double_v &_x, Vc::double_v *_sin, Vc::double_v *_cos) {
Expand All @@ -249,20 +249,20 @@ template<> template<> void Trigonometric<Vc::Detail::TrigonometricImplementation

double_int_v<V::abi> quadrant;
const V x = foldInput(_x, quadrant);
M sign = static_cast<M>(quadrant > 3);
M sign = simd_cast<M>(quadrant > 3);
quadrant(quadrant > 3) -= 4;

const V cos_s = cosSeries(x);
const V sin_s = sinSeries(x);

V c = cos_s;
c(static_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = sin_s;
c(sign ^ static_cast<M>(quadrant > double_int_v<V::abi>::One())) = -c;
c(simd_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = sin_s;
c(sign ^ simd_cast<M>(quadrant > double_int_v<V::abi>::One())) = -c;
*_cos = c;

V s = sin_s;
s(static_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = cos_s;
s(sign ^ static_cast<M>(_x < V::Zero())) = -s;
s(simd_cast<M>(quadrant == double_int_v<V::abi>::One() || quadrant == 2)) = cos_s;
s(sign ^ simd_cast<M>(_x < V::Zero())) = -s;
*_sin = s;
}
template<> template<typename V> V Trigonometric<Vc::Detail::TrigonometricImplementation<Vc::CurrentImplementation::current()>>::asin (const V &_x) {
Expand Down
9 changes: 6 additions & 3 deletions sse/mask.h
Expand Up @@ -129,11 +129,14 @@ template <typename T> class Mask<T, VectorAbi::Sse>
{
}

#if Vc_IS_VERSION_1
// explicit cast, implemented via simd_cast (implementation in sse/simd_cast.h)
template <typename U>
Vc_INTRINSIC explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> =
nullarg);
Vc_DEPRECATED("use simd_cast instead of explicit type casting to convert between "
"mask types") Vc_INTRINSIC
explicit Mask(U &&rhs,
Common::enable_if_mask_converts_explicitly<T, U> = nullarg);
#endif

Vc_ALWAYS_INLINE explicit Mask(const bool *mem) { load(mem); }
template<typename Flags> Vc_ALWAYS_INLINE explicit Mask(const bool *mem, Flags f) { load(mem, f); }
Expand Down

0 comments on commit b11e617

Please sign in to comment.