Branchless wide-integer comparisons and sign extension

base/base/wide_integer_impl.h

@@ -372,18 +372,14 @@ struct integer<Bits, Signed>::_impl
 
         self.items[little(0)] = _impl::to_Integral(rhs);
 
+        /// Sign-extend with a value select instead of a branch: a data-dependent branch here
+        /// mispredicts on mixed-sign data and prevents vectorization of columnar conversion loops.
+        base_type extension = 0;
         if constexpr (std::is_signed_v<Integral>)
-        {
-            if (rhs < 0)
-            {
-                for (unsigned i = 1; i < item_count; ++i)
-                    self.items[little(i)] = -1;
-                return;
-            }
-        }
+            extension = rhs < 0 ? base_type(-1) : base_type(0);
 
         for (unsigned i = 1; i < item_count; ++i)
-            self.items[little(i)] = 0;
+            self.items[little(i)] = extension;
     }
 
     template <typename TupleLike, size_t i = 0>
@@ -531,18 +527,13 @@ struct integer<Bits, Signed>::_impl
 
         if constexpr (Bits > Bits2)
         {
+            /// See the rationale in wide_integer_from_builtin.
+            base_type extension = 0;
             if constexpr (std::is_signed_v<Signed2>)
-            {
-                if (rhs < 0)
-                {
-                    for (unsigned i = to_copy; i < item_count; ++i)
-                        self.items[little(i)] = -1;
-                    return;
-                }
-            }
+                extension = rhs < 0 ? base_type(-1) : base_type(0);
 
             for (unsigned i = to_copy; i < item_count; ++i)
-                self.items[little(i)] = 0;
+                self.items[little(i)] = extension;
         }
     }
 
@@ -933,19 +924,31 @@ struct integer<Bits, Signed>::_impl
     {
         if constexpr (should_keep_size<T>())
         {
-            if (std::numeric_limits<T>::is_signed && (is_negative(lhs) != is_negative(rhs)))
-                return is_negative(rhs);
-
             integer<Bits, Signed> t = rhs;
-            for (unsigned i = 0; i < item_count; ++i)
+
+            /// Branchless lexicographic comparison from the most significant limb: no early-out, so the
+            /// columnar comparison kernels stay branchless and vectorize (e.g. to AVX-512 vpcmpq). The top
+            /// limb is compared as signed for signed types, which subsumes the sign check; lower limbs unsigned.
+            base_type lhs_top = lhs.items[big(0)];
+            base_type rhs_top = get_item(t, big(0));
+
+            bool greater;
+            if constexpr (std::is_same_v<Signed, signed>)
+                greater = static_cast<signed_base_type>(lhs_top) > static_cast<signed_base_type>(rhs_top);
+            else
+                greater = lhs_top > rhs_top;
+
+            bool equal = lhs_top == rhs_top;
+            for (unsigned i = 1; i < item_count; ++i)
             {
+                base_type lhs_item = lhs.items[big(i)];
                 base_type rhs_item = get_item(t, big(i));
 
-                if (lhs.items[big(i)] != rhs_item)
-                    return lhs.items[big(i)] > rhs_item;
+                greater = greater | (equal & (lhs_item > rhs_item));
+                equal = equal & (lhs_item == rhs_item);
             }
 
-            return false;
+            return greater;
         }
         else
         {
@@ -959,19 +962,29 @@ struct integer<Bits, Signed>::_impl
     {
         if constexpr (should_keep_size<T>())
         {
-            if (std::numeric_limits<T>::is_signed && (is_negative(lhs) != is_negative(rhs)))
-                return is_negative(lhs);
-
             integer<Bits, Signed> t = rhs;
-            for (unsigned i = 0; i < item_count; ++i)
+
+            /// See the rationale in operator_greater.
+            base_type lhs_top = lhs.items[big(0)];
+            base_type rhs_top = get_item(t, big(0));
+
+            bool less;
+            if constexpr (std::is_same_v<Signed, signed>)
+                less = static_cast<signed_base_type>(lhs_top) < static_cast<signed_base_type>(rhs_top);
+            else
+                less = lhs_top < rhs_top;
+
+            bool equal = lhs_top == rhs_top;
+            for (unsigned i = 1; i < item_count; ++i)
             {
+                base_type lhs_item = lhs.items[big(i)];
                 base_type rhs_item = get_item(t, big(i));
 
-                if (lhs.items[big(i)] != rhs_item)
-                    return lhs.items[big(i)] < rhs_item;
+                less = less | (equal & (lhs_item < rhs_item));
+                equal = equal & (lhs_item == rhs_item);
             }
 
-            return false;
+            return less;
         }
         else
         {

src/Common/tests/gtest_wide_integer.cpp

@@ -180,6 +180,171 @@ GTEST_TEST(WideInteger, Arithmetic)
 }
 
 
+/// The ordering operators use a branchless limb-wise comparison
+/// (see operator_less / operator_greater in wide_integer_impl.h).
+/// Comparing each pair in both widths cross-checks the 2-limb and 4-limb instantiations.
+template <typename T128, typename T256>
+static void checkComparisonAgainstWiderOracle(const std::vector<T128> & values)
+{
+    for (const T128 & lhs : values)
+    {
+        for (const T128 & rhs : values)
+        {
+            const T256 wide_lhs = lhs;
+            const T256 wide_rhs = rhs;
+
+            EXPECT_EQ(lhs < rhs, wide_lhs < wide_rhs) << toString(lhs) << " < " << toString(rhs);
+            EXPECT_EQ(lhs > rhs, wide_lhs > wide_rhs) << toString(lhs) << " > " << toString(rhs);
+            EXPECT_EQ(lhs <= rhs, wide_lhs <= wide_rhs) << toString(lhs) << " <= " << toString(rhs);
+            EXPECT_EQ(lhs >= rhs, wide_lhs >= wide_rhs) << toString(lhs) << " >= " << toString(rhs);
+            EXPECT_EQ(lhs == rhs, wide_lhs == wide_rhs) << toString(lhs) << " == " << toString(rhs);
+            EXPECT_EQ(lhs != rhs, wide_lhs != wide_rhs) << toString(lhs) << " != " << toString(rhs);
+        }
+    }
+}
+
+
+/// Oracle for the full 4-limb walk: the widen-to-256 helper above keeps bits 255:128 fixed to
+/// sign extension or zero, so it never exercises a decisive difference in the upper two limbs.
+/// Here the caller supplies a strictly ascending sequence and every pair is checked against the
+/// index order, which is independent of the comparison implementation under test.
+template <typename T>
+static void checkStrictlyAscending(const std::vector<T> & values)
+{
+    for (size_t i = 0; i < values.size(); ++i)
+    {
+        for (size_t j = 0; j < values.size(); ++j)
+        {
+            const T & lhs = values[i];
+            const T & rhs = values[j];
+
+            EXPECT_EQ(lhs < rhs, i < j) << toString(lhs) << " < " << toString(rhs);
+            EXPECT_EQ(lhs > rhs, i > j) << toString(lhs) << " > " << toString(rhs);
+            EXPECT_EQ(lhs <= rhs, i <= j) << toString(lhs) << " <= " << toString(rhs);
+            EXPECT_EQ(lhs >= rhs, i >= j) << toString(lhs) << " >= " << toString(rhs);
+            EXPECT_EQ(lhs == rhs, i == j) << toString(lhs) << " == " << toString(rhs);
+            EXPECT_EQ(lhs != rhs, i != j) << toString(lhs) << " != " << toString(rhs);
+        }
+    }
+}
+
+
+GTEST_TEST(WideInteger, Comparison128Boundaries)
+{
+    /// Constexpr evaluation must take the same fast path.
+    static_assert(std::numeric_limits<Int128>::min() < Int128(-1));
+    static_assert(Int128(-1) < Int128(0));
+    static_assert(Int128(0) < std::numeric_limits<Int128>::max());
+    static_assert(!(Int128(-1) < Int128(-1)));
+    static_assert(UInt128(0) < std::numeric_limits<UInt128>::max());
+    static_assert((UInt128(1) << 127) > ((UInt128(1) << 127) - 1));
+
+    {
+        const Int128 min = std::numeric_limits<Int128>::min();
+        const Int128 max = std::numeric_limits<Int128>::max();
+        const Int128 two_pow_64 = Int128(1) << 64;
+        const Int128 high_limb = Int128(5) << 64;
+
+        ASSERT_LT(min, Int128(-1));
+        ASSERT_LT(Int128(-1), Int128(0));
+        ASSERT_LT(Int128(0), Int128(1));
+        ASSERT_LT(Int128(1), max);
+        ASSERT_LT(min, max);
+        ASSERT_LT(-two_pow_64, Int128(-1));
+        ASSERT_LT(high_limb, high_limb + 1);
+
+        checkComparisonAgainstWiderOracle<Int128, Int256>({
+            0, 1, -1, 2, -2,
+            min, min + 1, max, max - 1,
+            two_pow_64 - 1, two_pow_64, two_pow_64 + 1,
+            -(two_pow_64 - 1), -two_pow_64, -(two_pow_64 + 1),
+            high_limb - 1, high_limb, high_limb + 1,
+            -(high_limb - 1), -high_limb, -(high_limb + 1),
+        });
+    }
+
+    {
+        const UInt128 max = std::numeric_limits<UInt128>::max();
+        const UInt128 sign_bit = UInt128(1) << 127;
+        const UInt128 two_pow_64 = UInt128(1) << 64;
+        const UInt128 high_limb = UInt128(5) << 64;
+
+        ASSERT_LT(UInt128(0), UInt128(1));
+        ASSERT_LT(sign_bit - 1, sign_bit);
+        ASSERT_LT(sign_bit, max);
+        ASSERT_LT(high_limb, high_limb + 1);
+
+        checkComparisonAgainstWiderOracle<UInt128, UInt256>({
+            0, 1, 2,
+            max, max - 1,
+            two_pow_64 - 1, two_pow_64, two_pow_64 + 1,
+            sign_bit - 1, sign_bit, sign_bit + 1,
+            high_limb - 1, high_limb, high_limb + 1,
+        });
+    }
+}
+
+
+GTEST_TEST(WideInteger, Comparison256Boundaries)
+{
+    /// Constexpr evaluation must take the same fast path, with the decisive difference above bit 127.
+    static_assert((Int256(1) << 192) > (Int256(1) << 128));
+    static_assert((Int256(1) << 128) > Int256(0));
+    static_assert(std::numeric_limits<Int256>::min() < (Int256(1) << 128));
+    static_assert((UInt256(1) << 255) > (UInt256(1) << 192));
+
+    {
+        const Int256 min = std::numeric_limits<Int256>::min();
+        const Int256 max = std::numeric_limits<Int256>::max();
+        const Int256 limb2 = Int256(1) << 128;
+        const Int256 limb3 = Int256(1) << 192;
+
+        checkStrictlyAscending<Int256>({
+            min,
+            min + 1,
+            -limb3,
+            -limb3 + 1,
+            -limb2 - 1,
+            -limb2,
+            -limb2 + 1,
+            -1,
+            0,
+            1,
+            limb2 - 1,
+            limb2,
+            limb2 + 1,
+            limb3 - 1,
+            limb3,
+            limb3 + 1,
+            max - 1,
+            max,
+        });
+    }
+
+    {
+        const UInt256 max = std::numeric_limits<UInt256>::max();
+        const UInt256 limb2 = UInt256(1) << 128;
+        const UInt256 limb3 = UInt256(1) << 192;
+        const UInt256 top_bit = UInt256(1) << 255;
+
+        checkStrictlyAscending<UInt256>({
+            0,
+            1,
+            limb2 - 1,
+            limb2,
+            limb2 + 1,
+            limb3 - 1,
+            limb3,
+            limb3 + 1,
+            top_bit,
+            top_bit + 1,
+            max - 1,
+            max,
+        });
+    }
+}
+
+
 GTEST_TEST(WideInteger, DecimalArithmetic)
 {
     Decimal128 zero{};

tests/performance/wide_integer_comparison.xml

@@ -0,0 +1,43 @@
+<test>
+    <!-- Comparison of 128-bit integer columns. Data shapes are chosen to exercise branch-prediction-hostile
+         patterns: fully random values (random sign for Int128), values that fit in the low 64 bits,
+         and values where half of the rows share the high 64 bits. -->
+
+    <create_query>
+        CREATE TABLE cmp128
+        (
+            u1 UInt128, u2 UInt128,
+            i1 Int128, i2 Int128,
+            small_u1 UInt128, small_u2 UInt128,
+            mixed_u1 UInt128, mixed_u2 UInt128
+        ) ENGINE = Memory
+    </create_query>
+    <fill_query>
+        INSERT INTO cmp128 SELECT
+            bitShiftLeft(toUInt128(cityHash64(number, 1)), 64) + cityHash64(number, 2),
+            bitShiftLeft(toUInt128(cityHash64(number, 3)), 64) + cityHash64(number, 4),
+            toInt128(bitShiftLeft(toUInt128(cityHash64(number, 5)), 64) + cityHash64(number, 6)),
+            toInt128(bitShiftLeft(toUInt128(cityHash64(number, 7)), 64) + cityHash64(number, 8)),
+            toUInt128(cityHash64(number, 9)),
+            toUInt128(cityHash64(number, 10)),
+            bitShiftLeft(toUInt128(cityHash64(number, 11)), 64) + cityHash64(number, 12),
+            bitShiftLeft(toUInt128(cityHash64(number, if(number % 2 = 0, 11, 13))), 64) + cityHash64(number, 14)
+        FROM numbers(10000000)
+    </fill_query>
+
+    <settings>
+        <max_threads>1</max_threads>
+    </settings>
+
+    <query>SELECT count() FROM cmp128 WHERE u1 &lt; u2</query>
+    <query>SELECT count() FROM cmp128 WHERE i1 &lt; i2</query>
+    <query>SELECT count() FROM cmp128 WHERE small_u1 &lt; small_u2</query>
+    <query>SELECT count() FROM cmp128 WHERE mixed_u1 &lt; mixed_u2</query>
+    <query>SELECT count() FROM cmp128 WHERE u1 &gt; u2</query>
+    <query>SELECT count() FROM cmp128 WHERE i1 &gt;= i2</query>
+    <query>SELECT count() FROM cmp128 WHERE u1 &lt; bitShiftLeft(toUInt128(1), 127)</query>
+    <query>SELECT i1 FROM cmp128 ORDER BY i1 LIMIT 10</query>
+    <query>SELECT u1 FROM cmp128 ORDER BY u1 DESC LIMIT 10</query>
+
+    <drop_query>DROP TABLE IF EXISTS cmp128</drop_query>
+</test>

tests/performance/wide_integer_conversion.xml

@@ -0,0 +1,15 @@
+<test>
+    <!-- Conversion of native integers to 256-bit integers and widening of 128-bit to 256-bit integers.
+         Random 64-bit values give a random sign for the signed cases, which is hostile to a branchy
+         sign extension. The unsigned and 128-bit target queries guard the already fast paths. -->
+
+    <settings>
+        <max_threads>1</max_threads>
+    </settings>
+
+    <query>SELECT count() FROM numbers(50000000) WHERE NOT ignore(toInt256(toInt64(cityHash64(number))))</query>
+    <query>SELECT count() FROM numbers(50000000) WHERE NOT ignore(toInt256(toInt64(bitAnd(cityHash64(number), 4611686018427387903))))</query>
+    <query>SELECT count() FROM numbers(50000000) WHERE NOT ignore(toUInt256(cityHash64(number)))</query>
+    <query>SELECT count() FROM numbers(50000000) WHERE NOT ignore(toInt256(toInt128(toInt64(cityHash64(number)))))</query>
+    <query>SELECT count() FROM numbers(50000000) WHERE NOT ignore(toInt128(toInt64(cityHash64(number))))</query>
+</test>