Merge pull request #43228 from HarryLeeIBM/hlee-s390x-wide-int-order

Fix byte order issue of wide integers for s390x
2024-11-23 16:12:01 +00:00 · 2022-12-08 09:50:54 +01:00 · 2022-12-08 09:50:54 +01:00 · f998d0c16b
commit f998d0c16b
parent 301267b0a4 8e07e37fe7
7 changed files with 108 additions and 53 deletions
--- a/base/base/wide_integer_impl.h
+++ b/base/base/wide_integer_impl.h
@ -187,8 +187,20 @@ struct integer<Bits, Signed>::_impl
    static_assert(Bits % base_bits == 0);

    /// Simple iteration in both directions
-    static constexpr unsigned little(unsigned idx) { return idx; }
-    static constexpr unsigned big(unsigned idx) { return item_count - 1 - idx; }
+    static constexpr unsigned little(unsigned idx)
+    {
+        if constexpr (std::endian::native == std::endian::little)
+            return idx;
+        else
+            return item_count - 1 - idx;
+    }
+    static constexpr unsigned big(unsigned idx)
+    {
+        if constexpr (std::endian::native == std::endian::little)
+            return item_count - 1 - idx;
+        else
+            return idx;
+    }
    static constexpr unsigned any(unsigned idx) { return idx; }

    template <class T>
@ -240,20 +252,20 @@ struct integer<Bits, Signed>::_impl
    {
        static_assert(sizeof(Integral) <= sizeof(base_type));

-        self.items[0] = _impl::to_Integral(rhs);
+        self.items[little(0)] = _impl::to_Integral(rhs);

        if constexpr (std::is_signed_v<Integral>)
        {
            if (rhs < 0)
            {
-                for (size_t i = 1; i < item_count; ++i)
-                    self.items[i] = -1;
+                for (unsigned i = 1; i < item_count; ++i)
+                    self.items[little(i)] = -1;
                return;
            }
        }

-        for (size_t i = 1; i < item_count; ++i)
-            self.items[i] = 0;
+        for (unsigned i = 1; i < item_count; ++i)
+            self.items[little(i)] = 0;
    }

    template <typename TupleLike, size_t i = 0>
@ -348,7 +360,7 @@ struct integer<Bits, Signed>::_impl
        constexpr const unsigned to_copy = min_bits / base_bits;

        for (unsigned i = 0; i < to_copy; ++i)
-            self.items[i] = rhs.items[i];
+            self.items[little(i)] = rhs.items[little(i)];

        if constexpr (Bits > Bits2)
        {
@ -357,13 +369,13 @@ struct integer<Bits, Signed>::_impl
                if (rhs < 0)
                {
                    for (unsigned i = to_copy; i < item_count; ++i)
-                        self.items[i] = -1;
+                        self.items[little(i)] = -1;
                    return;
                }
            }

            for (unsigned i = to_copy; i < item_count; ++i)
-                self.items[i] = 0;
+                self.items[little(i)] = 0;
        }
    }

@ -454,7 +466,7 @@ private:
        {
            if constexpr (sizeof(T) <= sizeof(base_type))
            {
-                if (0 == idx)
+                if (little(0) == idx)
                    return static_cast<base_type>(x);
            }
            else if (idx * sizeof(base_type) < sizeof(T))
@ -475,7 +487,7 @@ private:

        for (unsigned i = 0; i < op_items; ++i)
        {
-            base_type rhs_item = get_item(rhs, i);
+            base_type rhs_item = get_item(rhs, little(i));
            base_type & res_item = res.items[little(i)];

            underflows[i] = res_item < rhs_item;
@ -508,7 +520,7 @@ private:

        for (unsigned i = 0; i < op_items; ++i)
        {
-            base_type rhs_item = get_item(rhs, i);
+            base_type rhs_item = get_item(rhs, little(i));
            base_type & res_item = res.items[little(i)];

            res_item += rhs_item;
@ -580,12 +592,12 @@ private:
        else if constexpr (Bits == 128 && sizeof(base_type) == 8)
        {
            using CompilerUInt128 = unsigned __int128;
-            CompilerUInt128 a = (CompilerUInt128(lhs.items[1]) << 64) + lhs.items[0]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
-            CompilerUInt128 b = (CompilerUInt128(rhs.items[1]) << 64) + rhs.items[0]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
+            CompilerUInt128 a = (CompilerUInt128(lhs.items[little(1)]) << 64) + lhs.items[little(0)]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
+            CompilerUInt128 b = (CompilerUInt128(rhs.items[little(1)]) << 64) + rhs.items[little(0)]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
            CompilerUInt128 c = a * b;
            integer<Bits, Signed> res;
-            res.items[0] = c;
-            res.items[1] = c >> 64;
+            res.items[little(0)] = c;
+            res.items[little(1)] = c >> 64;
            return res;
        }
        else
@ -597,7 +609,7 @@ private:
 #endif
            for (unsigned i = 0; i < item_count; ++i)
            {
-                base_type rhs_item = get_item(rhs, i);
+                base_type rhs_item = get_item(rhs, little(i));
                unsigned pos = i * base_bits;

                while (rhs_item)
@ -792,7 +804,7 @@ public:
            integer<Bits, Signed> res;

            for (unsigned i = 0; i < item_count; ++i)
-                res.items[little(i)] = lhs.items[little(i)] | get_item(rhs, i);
+                res.items[little(i)] = lhs.items[little(i)] | get_item(rhs, little(i));
            return res;
        }
        else
@ -810,7 +822,7 @@ public:
            integer<Bits, Signed> res;

            for (unsigned i = 0; i < item_count; ++i)
-                res.items[little(i)] = lhs.items[little(i)] & get_item(rhs, i);
+                res.items[little(i)] = lhs.items[little(i)] & get_item(rhs, little(i));
            return res;
        }
        else
@ -845,17 +857,17 @@ public:
        {
            using CompilerUInt128 = unsigned __int128;

-            CompilerUInt128 a = (CompilerUInt128(numerator.items[1]) << 64) + numerator.items[0]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
-            CompilerUInt128 b = (CompilerUInt128(denominator.items[1]) << 64) + denominator.items[0]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
+            CompilerUInt128 a = (CompilerUInt128(numerator.items[little(1)]) << 64) + numerator.items[little(0)]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
+            CompilerUInt128 b = (CompilerUInt128(denominator.items[little(1)]) << 64) + denominator.items[little(0)]; // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
            CompilerUInt128 c = a / b; // NOLINT

            integer<Bits, Signed> res;
-            res.items[0] = c;
-            res.items[1] = c >> 64;
+            res.items[little(0)] = c;
+            res.items[little(1)] = c >> 64;

            CompilerUInt128 remainder = a - b * c;
-            numerator.items[0] = remainder;
-            numerator.items[1] = remainder >> 64;
+            numerator.items[little(0)] = remainder;
+            numerator.items[little(1)] = remainder >> 64;

            return res;
        }
@ -1039,15 +1051,15 @@ constexpr integer<Bits, Signed>::integer(std::initializer_list<T> il) noexcept
    else
    {
        auto it = il.begin();
-        for (size_t i = 0; i < _impl::item_count; ++i)
+        for (unsigned i = 0; i < _impl::item_count; ++i)
        {
            if (it < il.end())
            {
-                items[i] = *it;
+                items[_impl::little(i)] = *it;
                ++it;
            }
            else
-                items[i] = 0;
+                items[_impl::little(i)] = 0;
        }
    }
 }
@ -1208,7 +1220,7 @@ constexpr integer<Bits, Signed>::operator T() const noexcept

    UnsignedT res{};
    for (unsigned i = 0; i < _impl::item_count && i < (sizeof(T) + sizeof(base_type) - 1) / sizeof(base_type); ++i)
-        res += UnsignedT(items[i]) << (sizeof(base_type) * 8 * i); // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
+        res += UnsignedT(items[_impl::little(i)]) << (sizeof(base_type) * 8 * i); // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)

    return res;
 }
--- a/src/Common/SipHash.h
+++ b/src/Common/SipHash.h
@ -164,8 +164,13 @@ public:
    void get128(char * out)
    {
        finalize();
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+        unalignedStore<UInt64>(out + 8, v0 ^ v1);
+        unalignedStore<UInt64>(out, v2 ^ v3);
+#else
        unalignedStore<UInt64>(out, v0 ^ v1);
        unalignedStore<UInt64>(out + 8, v2 ^ v3);
+#endif
    }

    template <typename T>
--- a/src/Common/hex.h
+++ b/src/Common/hex.h
@ -58,9 +58,13 @@ inline void writeHexUIntImpl(TUInt uint_, char * out, const char * const table)

    value = uint_;

-    /// Use little endian
    for (size_t i = 0; i < sizeof(TUInt); ++i)
-        memcpy(out + i * 2, &table[static_cast<size_t>(uint8[sizeof(TUInt) - 1 - i]) * 2], 2);
+    {
+        if constexpr (std::endian::native == std::endian::little)
+            memcpy(out + i * 2, &table[static_cast<size_t>(uint8[sizeof(TUInt) - 1 - i]) * 2], 2);
+        else
+            memcpy(out + i * 2, &table[static_cast<size_t>(uint8[i]) * 2], 2);
+    }
 }

 template <typename TUInt>
--- a/src/Common/tests/gtest_wide_integer.cpp
+++ b/src/Common/tests/gtest_wide_integer.cpp
@ -62,7 +62,7 @@ GTEST_TEST(WideInteger, Conversions)

    zero += minus_one;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&zero, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(zero)));
+    ASSERT_EQ(0, memcmp(&zero, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFE", sizeof(zero)));
 #else
    ASSERT_EQ(0, memcmp(&zero, "\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(zero)));
 #endif
@ -160,7 +160,7 @@ GTEST_TEST(WideInteger, Arithmetic)

    zero += minus_one;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&zero, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(zero)));
+    ASSERT_EQ(0, memcmp(&zero, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFE", sizeof(zero)));
 #else
    ASSERT_EQ(0, memcmp(&zero, "\xFE\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(zero)));
 #endif
@ -244,7 +244,7 @@ GTEST_TEST(WideInteger, Shift)
    auto y = x << 64;

 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01", sizeof(Int128)));
+    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00", sizeof(Int128)));
 #else
    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00", sizeof(Int128)));
 #endif
@ -261,7 +261,7 @@ GTEST_TEST(WideInteger, Shift)
    y = x << 16;

 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\xFF\xFF\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));
+    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00\x00", sizeof(Int128)));
 #else
    ASSERT_EQ(0, memcmp(&y, "\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));
 #endif
@ -269,18 +269,21 @@ GTEST_TEST(WideInteger, Shift)
    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));

    y <<= 64;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00\x00\x00\x00\x00\x00\x00\x00", sizeof(Int128)));
+#else
    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));
-
+#endif
    y >>= 32;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\x00\x00\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));
+    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x00\x00\x00\x00", sizeof(Int128)));
 #else
    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF", sizeof(Int128)));
 #endif

    y <<= 64;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF\x00\x00\x00\x00", sizeof(Int128)));
+    ASSERT_EQ(0, memcmp(&y, "\xFF\xFF\xFF\xFF\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", sizeof(Int128)));
 #else
    ASSERT_EQ(0, memcmp(&y, "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF", sizeof(Int128)));
 #endif
--- a/src/Functions/FunctionsBinaryRepresentation.cpp
+++ b/src/Functions/FunctionsBinaryRepresentation.cpp
@ -65,13 +65,27 @@ struct HexImpl
        }
    }

-    static void executeOneString(const UInt8 * pos, const UInt8 * end, char *& out)
+    static void executeOneString(const UInt8 * pos, const UInt8 * end, char *& out, bool reverse_order = false)
    {
-        while (pos < end)
+        if (!reverse_order)
        {
-            writeHexByteUppercase(*pos, out);
-            ++pos;
-            out += word_size;
+            while (pos < end)
+            {
+                writeHexByteUppercase(*pos, out);
+                ++pos;
+                out += word_size;
+            }
+        }
+        else
+        {
+            const auto * start_pos = pos;
+            pos = end - 1;
+            while (pos >= start_pos)
+            {
+                writeHexByteUppercase(*pos, out);
+                --pos;
+                out += word_size;
+            }
        }
        *out = '\0';
        ++out;
@ -95,7 +109,8 @@ struct HexImpl
        for (size_t i = 0; i < size; ++i)
        {
            const UInt8 * in_pos = reinterpret_cast<const UInt8 *>(&in_vec[i]);
-            executeOneString(in_pos, in_pos + type_size_in_bytes, out);
+            bool reverse_order = (std::endian::native == std::endian::big);
+            executeOneString(in_pos, in_pos + type_size_in_bytes, out, reverse_order);

            pos += hex_length;
            out_offsets[i] = pos;
@ -174,7 +189,9 @@ struct BinImpl
        for (size_t i = 0; i < size; ++i)
        {
            const UInt8 * in_pos = reinterpret_cast<const UInt8 *>(&in_vec[i]);
-            executeOneString(in_pos, in_pos + type_size_in_bytes, out);
+
+            bool reverse_order = (std::endian::native == std::endian::big);
+            executeOneString(in_pos, in_pos + type_size_in_bytes, out, reverse_order);

            pos += hex_length;
            out_offsets[i] = pos;
@ -182,13 +199,27 @@ struct BinImpl
        col_res = std::move(col_str);
    }

-    static void executeOneString(const UInt8 * pos, const UInt8 * end, char *& out)
+    static void executeOneString(const UInt8 * pos, const UInt8 * end, char *& out, bool reverse_order = false)
    {
-        while (pos < end)
+        if (!reverse_order)
        {
-            writeBinByte(*pos, out);
-            ++pos;
-            out += word_size;
+            while (pos < end)
+            {
+                writeBinByte(*pos, out);
+                ++pos;
+                out += word_size;
+            }
+        }
+        else
+        {
+            const auto * start_pos = pos;
+            pos = end - 1;
+            while (pos >= start_pos)
+            {
+                writeBinByte(*pos, out);
+                --pos;
+                out += word_size;
+            }
        }
        *out = '\0';
        ++out;
--- a/src/IO/ReadHelpers.h
+++ b/src/IO/ReadHelpers.h
@ -1078,7 +1078,7 @@ inline void readBinaryBigEndian(T & x, ReadBuffer & buf)    /// Assuming little
 {
    for (size_t i = 0; i != std::size(x.items); ++i)
    {
-        auto & item = x.items[std::size(x.items) - i - 1];
+        auto & item = x.items[(std::endian::native == std::endian::little) ? std::size(x.items) - i - 1 : i];
        readBinaryBigEndian(item, buf);
    }
 }
--- a/src/IO/WriteHelpers.h
+++ b/src/IO/WriteHelpers.h
@ -1126,7 +1126,7 @@ inline void writeBinaryBigEndian(const T & x, WriteBuffer & buf)    /// Assuming
 {
    for (size_t i = 0; i != std::size(x.items); ++i)
    {
-        const auto & item = x.items[std::size(x.items) - i - 1];
+        const auto & item = x.items[(std::endian::native == std::endian::little) ? std::size(x.items) - i - 1 : i];
        writeBinaryBigEndian(item, buf);
    }
 }