Core: Name the Array shift operators for the shift they perform

libeigen/eigen!2816

Co-authored-by: Rasmus Munk Larsen <rmlarsen@gmail.com>
This commit is contained in:
Rasmus Munk Larsen
2026-08-15 18:19:50 -07:00
co-authored by Rasmus Munk Larsen
parent 9b1c027e6d
commit a0edc3964b
5 changed files with 117 additions and 49 deletions
+4
View File
@@ -5,6 +5,7 @@
New features:
- ComplexQZ implementation [!1962]
- Generic clang vector extension backend [!2051]
- `ArrayBase::logicalShiftRight<N>()`, a zero-filling right shift, alongside `arithmeticShiftRight<N>()` and `logicalShiftLeft<N>()` as the spelled-out names for the existing shift operators [!2816]
Bug fixes:
- Row-major triangular matrix-vector products could return incorrect results or double-destroy temporaries for scalars requiring initialization, and could leak some large right-hand-side buffers [!2818]
@@ -14,6 +15,9 @@ Bug fixes:
- `Transform::inverse(Projective)` returned an uninitialized result for every mode other than `Projective` [!2814]
- `DGMRES::iterations()` returned `maxIterations()` after every solve, including converged ones [!2814]
Deprecations:
- `ArrayBase::shiftRight<N>()` and `ArrayBase::shiftLeft<N>()`, in favour of `arithmeticShiftRight<N>()` and `logicalShiftLeft<N>()` [!2816]
## [5.0.1] - 2025-11-11
A few bug-fixes from the master branch, including
+10 -7
View File
@@ -1945,28 +1945,31 @@ EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE double fmod(const double& a, const double&
#undef SYCL_SPECIALIZE_BINARY_FUNC
#endif
// A logical shift is a shift of the bit pattern, so it goes through the unsigned type of the same
// width whatever Scalar's signedness. A Scalar narrower than int promotes to int for the shift
// itself, which the explicit truncation undoes: for the left shift the promoted value can carry set
// bits above Scalar's width, and dropping them is the operation rather than an accident of the cast.
template <typename Scalar, typename Enable = std::enable_if_t<std::is_integral<Scalar>::value>>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar logical_shift_left(const Scalar& a, int n) {
using UnsignedScalar = typename numext::get_integer_by_size<sizeof(Scalar)>::unsigned_type;
return bit_cast<Scalar, UnsignedScalar>(bit_cast<UnsignedScalar, Scalar>(a) << n);
return bit_cast<Scalar, UnsignedScalar>(static_cast<UnsignedScalar>(bit_cast<UnsignedScalar, Scalar>(a) << n));
}
template <typename Scalar, typename Enable = std::enable_if_t<std::is_integral<Scalar>::value>>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar logical_shift_right(const Scalar& a, int n) {
using UnsignedScalar = typename numext::get_integer_by_size<sizeof(Scalar)>::unsigned_type;
return bit_cast<Scalar, UnsignedScalar>(bit_cast<UnsignedScalar, Scalar>(a) >> n);
return bit_cast<Scalar, UnsignedScalar>(static_cast<UnsignedScalar>(bit_cast<UnsignedScalar, Scalar>(a) >> n));
}
// An arithmetic shift propagates the sign bit, so it coincides with the logical shift when Scalar is
// unsigned and has none. Shifting through the signed type unconditionally would sign-extend an
// ordinary value bit, which is what every backend's parithmetic_shift_right on unsigned packets
// avoids, leaving the scalar and vectorized paths of one expression disagreeing.
// avoids, leaving the scalar and vectorized paths of one expression disagreeing. Scalar's own
// operator>> already selects on its signedness, so no reinterpretation is needed; the cast only
// undoes the integral promotion a Scalar narrower than int is subject to.
template <typename Scalar, typename Enable = std::enable_if_t<std::is_integral<Scalar>::value>>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar arithmetic_shift_right(const Scalar& a, int n) {
using IntegerBySize = numext::get_integer_by_size<sizeof(Scalar)>;
using ShiftScalar = std::conditional_t<std::is_signed<Scalar>::value, typename IntegerBySize::signed_type,
typename IntegerBySize::unsigned_type>;
return bit_cast<Scalar, ShiftScalar>(bit_cast<ShiftScalar, Scalar>(a) >> n);
return static_cast<Scalar>(a >> n);
}
template <typename Scalar>
+26 -6
View File
@@ -229,10 +229,10 @@ struct functor_traits<core_cast_op<SrcType, DstType>> {
/** \internal
* \brief Template functor to arithmetically shift a scalar right by a number of bits
*
* \sa class CwiseUnaryOp, ArrayBase::shiftRight()
* \sa class CwiseUnaryOp, ArrayBase::arithmeticShiftRight()
*/
template <typename Scalar, int N>
struct scalar_shift_right_op {
struct scalar_arithmetic_shift_right_op {
EIGEN_DEVICE_FUNC constexpr EIGEN_STRONG_INLINE Scalar operator()(const Scalar& a) const {
return numext::arithmetic_shift_right(a, N);
}
@@ -242,17 +242,37 @@ struct scalar_shift_right_op {
}
};
template <typename Scalar, int N>
struct functor_traits<scalar_shift_right_op<Scalar, N>> {
struct functor_traits<scalar_arithmetic_shift_right_op<Scalar, N>> {
enum { Cost = NumTraits<Scalar>::AddCost, PacketAccess = packet_traits<Scalar>::HasShift };
};
/** \internal
* \brief Template functor to logically shift a scalar right by a number of bits
*
* \sa class CwiseUnaryOp, ArrayBase::logicalShiftRight()
*/
template <typename Scalar, int N>
struct scalar_logical_shift_right_op {
EIGEN_DEVICE_FUNC constexpr EIGEN_STRONG_INLINE Scalar operator()(const Scalar& a) const {
return numext::logical_shift_right(a, N);
}
template <typename Packet>
EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Packet packetOp(const Packet& a) const {
return internal::plogical_shift_right<N>(a);
}
};
template <typename Scalar, int N>
struct functor_traits<scalar_logical_shift_right_op<Scalar, N>> {
enum { Cost = NumTraits<Scalar>::AddCost, PacketAccess = packet_traits<Scalar>::HasShift };
};
/** \internal
* \brief Template functor to logically shift a scalar left by a number of bits
*
* \sa class CwiseUnaryOp, ArrayBase::shiftLeft()
* \sa class CwiseUnaryOp, ArrayBase::logicalShiftLeft()
*/
template <typename Scalar, int N>
struct scalar_shift_left_op {
struct scalar_logical_shift_left_op {
EIGEN_DEVICE_FUNC constexpr EIGEN_STRONG_INLINE Scalar operator()(const Scalar& a) const {
return numext::logical_shift_left(a, N);
}
@@ -262,7 +282,7 @@ struct scalar_shift_left_op {
}
};
template <typename Scalar, int N>
struct functor_traits<scalar_shift_left_op<Scalar, N>> {
struct functor_traits<scalar_logical_shift_left_op<Scalar, N>> {
enum { Cost = NumTraits<Scalar>::AddCost, PacketAccess = packet_traits<Scalar>::HasShift };
};
+68 -10
View File
@@ -346,8 +346,8 @@ EIGEN_MAKE_CWISE_UNARY_OP(ceil, internal::scalar_ceil_op, CeilReturnType)
EIGEN_MAKE_CWISE_UNARY_OP(trunc, internal::scalar_trunc_op, TruncReturnType)
template <int N>
struct ShiftRightXpr {
using Type = CwiseUnaryOp<internal::scalar_shift_right_op<Scalar, N>, const Derived>;
struct ArithmeticShiftRightXpr {
using Type = CwiseUnaryOp<internal::scalar_arithmetic_shift_right_op<Scalar, N>, const Derived>;
};
/** \returns an expression of \c *this with the \a Scalar type arithmetically
@@ -355,30 +355,88 @@ struct ShiftRightXpr {
*
* The template parameter \a N specifies the number of bit positions to shift.
* The vacated high bits are filled with the sign bit for a signed \a Scalar and
* with zero for an unsigned one, which has no sign bit to propagate.
* with zero for an unsigned one, which has no sign bit to propagate. Use
* logicalShiftRight() to fill with zero regardless of signedness.
*
* \sa shiftLeft()
* \sa logicalShiftRight(), logicalShiftLeft()
*/
template <int N>
EIGEN_DEVICE_FUNC constexpr typename ShiftRightXpr<N>::Type shiftRight() const {
return typename ShiftRightXpr<N>::Type(derived());
EIGEN_DEVICE_FUNC constexpr typename ArithmeticShiftRightXpr<N>::Type arithmeticShiftRight() const {
return typename ArithmeticShiftRightXpr<N>::Type(derived());
}
template <int N>
struct ShiftLeftXpr {
using Type = CwiseUnaryOp<internal::scalar_shift_left_op<Scalar, N>, const Derived>;
struct LogicalShiftRightXpr {
using Type = CwiseUnaryOp<internal::scalar_logical_shift_right_op<Scalar, N>, const Derived>;
};
/** \returns an expression of \c *this with the \a Scalar type logically
* shifted right by \a N bit positions.
*
* The template parameter \a N specifies the number of bit positions to shift.
* The vacated high bits are filled with zero even when \a Scalar is signed; use
* arithmeticShiftRight() to propagate the sign bit instead.
*
* \sa arithmeticShiftRight(), logicalShiftLeft()
*/
template <int N>
EIGEN_DEVICE_FUNC constexpr typename LogicalShiftRightXpr<N>::Type logicalShiftRight() const {
return typename LogicalShiftRightXpr<N>::Type(derived());
}
template <int N>
struct LogicalShiftLeftXpr {
using Type = CwiseUnaryOp<internal::scalar_logical_shift_left_op<Scalar, N>, const Derived>;
};
/** \returns an expression of \c *this with the \a Scalar type logically
* shifted left by \a N bit positions.
*
* The template parameter \a N specifies the number of bit positions to shift.
* The vacated low bits are filled with zero. There is no separate arithmetic
* left shift: for every integral \a Scalar it produces the same bit pattern.
*
* \sa shiftRight()
* \sa arithmeticShiftRight(), logicalShiftRight()
*/
template <int N>
EIGEN_DEVICE_FUNC constexpr typename LogicalShiftLeftXpr<N>::Type logicalShiftLeft() const {
return typename LogicalShiftLeftXpr<N>::Type(derived());
}
template <int N>
struct ShiftRightXpr {
using Type = typename ArithmeticShiftRightXpr<N>::Type;
};
/** \deprecated Use arithmeticShiftRight() instead.
*
* \returns an expression of \c *this with the \a Scalar type arithmetically
* shifted right by \a N bit positions.
*
* \sa arithmeticShiftRight()
*/
template <int N>
EIGEN_DEPRECATED_WITH_REASON("Use arithmeticShiftRight() instead.")
EIGEN_DEVICE_FUNC constexpr typename ShiftRightXpr<N>::Type shiftRight() const {
return arithmeticShiftRight<N>();
}
template <int N>
struct ShiftLeftXpr {
using Type = typename LogicalShiftLeftXpr<N>::Type;
};
/** \deprecated Use logicalShiftLeft() instead.
*
* \returns an expression of \c *this with the \a Scalar type logically
* shifted left by \a N bit positions.
*
* \sa logicalShiftLeft()
*/
template <int N>
EIGEN_DEPRECATED_WITH_REASON("Use logicalShiftLeft() instead.")
EIGEN_DEVICE_FUNC constexpr typename ShiftLeftXpr<N>::Type shiftLeft() const {
return typename ShiftLeftXpr<N>::Type(derived());
return logicalShiftLeft<N>();
}
/** \returns an expression of the coefficient-wise isnan of *this.
+9 -26
View File
@@ -8,6 +8,9 @@
// with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
// SPDX-License-Identifier: MPL-2.0
// Silence warnings about the deprecated shiftLeft()/shiftRight(), which are still being tested.
#define EIGEN_NO_DEPRECATED_WARNING
#include <vector>
#include "main.h"
#include "random_without_cast_overflow.h"
@@ -1170,29 +1173,6 @@ void min_max(const ArrayType& m) {
}
}
template <typename Scalar>
struct shift_imm_traits {
enum { Cost = 1, PacketAccess = internal::packet_traits<Scalar>::HasShift };
};
// ArrayBase exposes an arithmetic right shift and a logical left shift, but no logical right shift,
// so that packet op is still reached through a functor of its own.
template <int N, typename Scalar>
struct logical_right_shift_op {
Scalar operator()(const Scalar& v) const { return numext::logical_shift_right(v, N); }
template <typename Packet>
Packet packetOp(const Packet& v) const {
return internal::plogical_shift_right<N>(v);
}
};
namespace Eigen {
namespace internal {
template <int N, typename Scalar>
struct functor_traits<logical_right_shift_op<N, Scalar>> : shift_imm_traits<Scalar> {};
} // namespace internal
} // namespace Eigen
// A lambda takes the default functor_traits, so each reference arm stays scalar while the arm under
// test vectorizes. Comparing the reference against the shift expression as well as against the
// assigned result evaluates the expression coefficient-wise, which is the only path that reaches the
@@ -1217,20 +1197,23 @@ struct shift_test_impl {
m1(rows - 1, cols - 1) = NumTraits<Scalar>::highest();
m2 = m1.unaryExpr([](const Scalar& v) { return numext::logical_shift_left(v, N); });
m3 = m1.template shiftLeft<N>();
m3 = m1.template logicalShiftLeft<N>();
VERIFY_IS_CWISE_EQUAL(m2, m3);
VERIFY_IS_CWISE_EQUAL(m2, m1.template logicalShiftLeft<N>());
VERIFY_IS_CWISE_EQUAL(m2, m1.template shiftLeft<N>());
m2 = m1.unaryExpr([](const Scalar& v) { return numext::logical_shift_right(v, N); });
m3 = m1.unaryExpr(logical_right_shift_op<N, Scalar>());
m3 = m1.template logicalShiftRight<N>();
VERIFY_IS_CWISE_EQUAL(m2, m3);
VERIFY_IS_CWISE_EQUAL(m2, m1.template logicalShiftRight<N>());
// Referencing Scalar's own operator>> rather than the numext helper the functor calls keeps this
// arm independent of the implementation under test, and states the semantics: fill with the sign
// bit when Scalar is signed and with zero when it is not.
m2 = m1.unaryExpr([](const Scalar& v) { return static_cast<Scalar>(v >> N); });
m3 = m1.template shiftRight<N>();
m3 = m1.template arithmeticShiftRight<N>();
VERIFY_IS_CWISE_EQUAL(m2, m3);
VERIFY_IS_CWISE_EQUAL(m2, m1.template arithmeticShiftRight<N>());
VERIFY_IS_CWISE_EQUAL(m2, m1.template shiftRight<N>());
run<N + 1>(m);