Right shift of a signed value will sign-extend

chromium · Oct 2, 2023 · 8a2541a · 8a2541a
1 parent 5faab4a
commit 8a2541a
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 28 deletions.
diff --git a/sus/num/__private/intrinsics.h b/sus/num/__private/intrinsics.h
@@ -121,20 +121,6 @@ sus_pure_const sus_always_inline constexpr T unchecked_xor(T x, T y) noexcept {
   return static_cast<T>(MathType<T>{x} ^ MathType<T>{y});
 }
 
-template <class T>
-  requires(std::is_integral_v<T> && !std::is_signed_v<T>)
-sus_pure_const sus_always_inline constexpr T unchecked_shl(
-    T x, uint64_t y) noexcept {
-  return static_cast<T>(MathType<T>{x} << y);
-}
-
-template <class T>
-  requires(std::is_integral_v<T> && !std::is_signed_v<T>)
-sus_pure_const sus_always_inline constexpr T unchecked_shr(
-    T x, uint64_t y) noexcept {
-  return static_cast<T>(MathType<T>{x} >> y);
-}
-
 template <class T>
   requires(std::is_integral_v<T>)
 sus_pure_const sus_always_inline constexpr uint32_t num_bits() noexcept {
@@ -466,6 +452,28 @@ sus_pure_const sus_always_inline constexpr T min_value() noexcept {
     return -179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368.0;
 }
 
+template <class T>
+  requires(std::is_integral_v<T> && !std::is_signed_v<T>)
+sus_pure_const sus_always_inline constexpr T unchecked_shl(
+    T x, uint64_t y) noexcept {
+  return static_cast<T>(MathType<T>{x} << y);
+}
+
+template <class T>
+  requires(std::is_integral_v<T> && !std::is_signed_v<T>)
+sus_pure_const sus_always_inline constexpr T unchecked_shr(
+    T x, uint64_t y) noexcept {
+  return static_cast<T>(MathType<T>{x} >> y);
+}
+
+template <class T>
+  requires(std::is_integral_v<T> && std::is_signed_v<T>)
+sus_pure_const sus_always_inline constexpr T unchecked_shr(
+    T x, uint64_t y) noexcept {
+  // Performs sign extension.
+  return static_cast<T>(MathType<T>{x} >> y);
+}
+
 template <class T>
   requires(std::is_integral_v<T> && std::is_unsigned_v<T> &&
            ::sus::mem::size_of<T>() <= 8)
@@ -1105,9 +1113,8 @@ sus_pure_const inline constexpr OverflowOut<T> shr_with_overflow(
   const bool overflow = shift >= num_bits<T>();
   if (overflow) [[unlikely]]
     shift = shift & (unchecked_sub(num_bits<T>(), uint32_t{1}));
-  return OverflowOut sus_clang_bug_56394(<T>){
-      .overflow = overflow,
-      .value = into_signed(unchecked_shr(into_unsigned(x), shift))};
+  return OverflowOut sus_clang_bug_56394(<T>){.overflow = overflow,
+                                              .value = unchecked_shr(x, shift)};
 }
 
 template <class T>

diff --git a/sus/num/__private/signed_integer_methods.inc b/sus/num/__private/signed_integer_methods.inc
@@ -952,6 +952,8 @@ constexpr inline void operator<<=(u32 r) & noexcept {
   primitive_value = out.value;
 }
 /// Satisfies the [`ShrAssign<@doc.self>`]($sus::num::ShrAssign) concept.
+///
+/// Performs sign extension, copying the sign bit to the right if its set.
 constexpr inline void operator>>=(u32 r) & noexcept {
   const auto out =
       __private::shr_with_overflow(primitive_value, r.primitive_value);

diff --git a/sus/num/i16_unittest.cc b/sus/num/i16_unittest.cc
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <bit>
 #include <type_traits>
 
 #include "googletest/include/gtest/gtest.h"
@@ -749,4 +750,28 @@ TEST(i16, fmt) {
   EXPECT_EQ(fmt::format("{:+#x}", 12345_i16), "+0x3039");
 }
 
+TEST(i16, Shr) {
+  constexpr auto a = (5_i16) >> 1_u32;
+  EXPECT_EQ(a, 2_i16);
+  EXPECT_EQ(4_i16 >> 1_u32, 2_i16);
+
+  // ** Signed only.
+  EXPECT_EQ(-4_i16 >> 1_u32,
+            std::bit_cast<i16>((std::bit_cast<u16>(int16_t{-4}) >> 1u) |
+                               0b1000'0000'0000'0000_u16));
+  EXPECT_EQ(-1_i16 >> 15_u32, std::bit_cast<i16>(0xffff_u16));
+
+  auto x = 4_i16;
+  x >>= 1_u32;
+  EXPECT_EQ(x, 2_i16);
+
+  // ** Signed only.
+  x = -4_i16;
+  x >>= 1_u32;
+  EXPECT_EQ(x, std::bit_cast<i16>((std::bit_cast<u16>(int16_t{-4}) >> 1u) |
+                                  0b1000'0000'0000'0000_u16));
+
+  EXPECT_EQ(i16::MIN >> 7u, std::bit_cast<int16_t>(uint16_t{0xff00u}));
+}
+
 }  // namespace
diff --git a/sus/num/i32_unittest.cc b/sus/num/i32_unittest.cc
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <bit>
 #include <sstream>
 #include <type_traits>
 
@@ -1798,8 +1799,9 @@ TEST(i32, Shr) {
 
   // ** Signed only.
   EXPECT_EQ(-4_i32 >> 1_u32,
-            i32(static_cast<int32_t>(static_cast<uint32_t>(-4) >> 1)));
-  EXPECT_EQ(-1_i32 >> 31_u32, 1_i32);
+            std::bit_cast<i32>((std::bit_cast<u32>(-4) >> 1u) |
+                               0b1000'0000'0000'0000'0000'0000'0000'0000));
+  EXPECT_EQ(-1_i32 >> 31_u32, std::bit_cast<i32>(0xffffffff));
 
   auto x = 4_i32;
   x >>= 1_u32;
@@ -1808,7 +1810,10 @@ TEST(i32, Shr) {
   // ** Signed only.
   x = -4_i32;
   x >>= 1_u32;
-  EXPECT_EQ(x, i32(static_cast<int32_t>(static_cast<uint32_t>(-4) >> 1)));
+  EXPECT_EQ(x, std::bit_cast<i32>((std::bit_cast<u32>(-4) >> 1u) |
+                                  0b1000'0000'0000'0000'0000'0000'0000'0000));
+
+  EXPECT_EQ(i32::MIN >> 7u, std::bit_cast<int32_t>(uint32_t{0xff000000u}));
 }
 
 TEST(i32DeathTest, ShrOverflow) {
@@ -1845,9 +1850,10 @@ TEST(i32, CheckedShr) {
   EXPECT_EQ((1_i32).checked_shr(64_u32), None);
 
   // ** Signed only.
-  EXPECT_EQ(
-      (-2_i32).checked_shr(1_u32),
-      Option<i32>(i32(static_cast<int32_t>(static_cast<uint32_t>(-2) >> 1))));
+  EXPECT_EQ((-2_i32).checked_shr(1_u32),
+            Option<i32>(std::bit_cast<i32>(
+                (std::bit_cast<u32>(-2) >> 1u) |
+                0b1000'0000'0000'0000'0000'0000'0000'0000u)));
   EXPECT_EQ((-1_i32).checked_shr(32_u32), None);
 }
 
@@ -1862,10 +1868,11 @@ TEST(i32, OverflowingShr) {
   EXPECT_EQ((4_i32).overflowing_shr(33_u32), (Tuple<i32, bool>(2_i32, true)));
 
   // ** Signed only.
-  EXPECT_EQ(
-      (-2_i32).overflowing_shr(1_u32),
-      (Tuple<i32, bool>(
-          i32(static_cast<int32_t>(static_cast<uint32_t>(-2) >> 1u)), false)));
+  EXPECT_EQ((-2_i32).overflowing_shr(1_u32),
+            (Tuple<i32, bool>(
+                std::bit_cast<i32>((std::bit_cast<u32>(-2) >> 1u) |
+                                   0b1000'0000'0000'0000'0000'0000'0000'0000u),
+                false)));
 }
 
 TEST(i32, WrappingShr) {
@@ -1880,7 +1887,8 @@ TEST(i32, WrappingShr) {
 
   // ** Signed only.
   EXPECT_EQ((-2_i32).wrapping_shr(1_u32),
-            i32(static_cast<int32_t>(static_cast<uint32_t>(-2) >> 1u)));
+            std::bit_cast<i32>((std::bit_cast<u32>(-2) >> 1u) |
+                               0b1000'0000'0000'0000'0000'0000'0000'0000u));
 }
 
 TEST(i32, Sub) {

diff --git a/sus/num/signed_integer.h b/sus/num/signed_integer.h
@@ -195,6 +195,8 @@ constexpr inline P operator<<(P l, U r) noexcept = delete;
 
 /// Satisfies the [`Shr`]($sus::num::Shr) concept for signed primitive integers
 /// shifted by [`u64`]($sus::num::u64).
+///
+/// Performs sign extension, copying the sign bit to the right if its set.
 /// #[doc.overloads=signed.prim.>>u64]
 template <class P, Integer U>
   requires((SignedPrimitiveInteger<P> || SignedPrimitiveEnum<P>) &&
@@ -235,6 +237,8 @@ template <class U>
 constexpr inline i8 operator<<(i8 l, U r) noexcept = delete;
 /// Satisfies the [`Shr`]($sus::num::Shr) concept for signed integers.
 ///
+/// Performs sign extension, copying the sign bit to the right if its set.
+///
 /// #[doc.overloads=signedint.>>]
 [[nodiscard]] sus_pure_const constexpr inline i8 operator>>(
     i8 l, std::convertible_to<u64> auto r) noexcept {