jacktengg commented on code in PR #67463:
URL: https://github.com/apache/doris/pull/67463#discussion_r3921648373
##########
be/src/core/wide_integer_impl.h:
##########
@@ -313,91 +286,66 @@ struct integer<Bits, Signed>::_impl {
}
}
- /**
- * N.B. t is constructed from double, so max(t) = max(double) ~ 2^310
- * the recursive call happens when t / 2^64 > 2^64, so there won't be more
than 5 of them.
- *
- * t = a1 * max_int + b1, a1 > max_int, b1 < max_int
- * a1 = a2 * max_int + b2, a2 > max_int, b2 < max_int
- * a_(n - 1) = a_n * max_int + b2, a_n <= max_int <- base case.
- */
- template <class T>
- CONSTEXPR_FROM_DOUBLE static void set_multiplier(integer<Bits, Signed>&
self, T t) noexcept
-#if !DORIS_WIDE_FROM_DOUBLE_INLINE
- ;
-#else
- {
- constexpr uint64_t max_int = std::numeric_limits<uint64_t>::max();
- static_assert(std::is_same_v<T, double> || std::is_same_v<T,
FromDoubleIntermediateType>);
- /// Implementation specific behaviour on overflow (if we don't check
here, stack overflow will triggered in bigint_cast).
- if constexpr (std::is_same_v<T, double>) {
- if (!std::isfinite(t)) {
- self = 0;
- return;
- }
- } else {
- if (!boost::math::isfinite(t)) {
- self = 0;
- return;
- }
+ constexpr static void wide_integer_from_builtin(integer<Bits, Signed>&
self,
+ double rhs) noexcept {
+ static_assert(sizeof(double) == sizeof(uint64_t));
+ static_assert(std::numeric_limits<double>::is_iec559);
+ static_assert(std::numeric_limits<double>::digits == 53);
+
+ constexpr unsigned fraction_bits = std::numeric_limits<double>::digits
- 1;
+ constexpr uint64_t fraction_mask = (uint64_t(1) << fraction_bits) - 1;
+ constexpr uint64_t exponent_mask = uint64_t(0x7ff) << fraction_bits;
+ constexpr uint64_t sign_mask = uint64_t(1) << 63;
+ constexpr int exponent_bias = 1023;
+
+ const uint64_t bits = std::bit_cast<uint64_t>(rhs);
+ const uint64_t biased_exponent = (bits & exponent_mask) >>
fraction_bits;
+
+ // Zero and subnormal values have an absolute value less than one.
Preserve the previous
+ // behavior of mapping infinity and NaN to zero as well.
+ if (biased_exponent == 0 || biased_exponent == 0x7ff) {
+ self = 0;
+ return;
}
- const T alpha = t / static_cast<T>(max_int);
-
- /** Here we have to use strict comparison.
- * The max_int is 2^64 - 1.
- * When casted to floating point type, it will be rounded to the
closest representable number,
- * which is 2^64.
- * But 2^64 is not representable in uint64_t,
- * so the maximum representable number will be strictly less.
- */
- if (alpha < static_cast<T>(max_int)) {
- self = static_cast<uint64_t>(alpha);
- } else { // max(double) / 2^64 will surely contain less than 52
precision bits, so speed up computations.
- set_multiplier<double>(self, static_cast<double>(alpha));
+ const int exponent = static_cast<int>(biased_exponent) - exponent_bias;
+ if (exponent < 0) {
+ self = 0;
+ return;
}
- self *= max_int;
- self += static_cast<uint64_t>(t - floor(static_cast<double>(alpha)) *
- static_cast<T>(max_int)); //
+= b_i
- }
-#endif
+ const uint64_t significand = (bits & fraction_mask) | (uint64_t(1) <<
fraction_bits);
+ self = significand;
- CONSTEXPR_FROM_DOUBLE static void wide_integer_from_builtin(integer<Bits,
Signed>& self,
- double rhs)
noexcept
-#if !DORIS_WIDE_FROM_DOUBLE_INLINE
- ;
-#else
- {
- constexpr int64_t max_int = std::numeric_limits<int64_t>::max();
- constexpr int64_t min_int = std::numeric_limits<int64_t>::lowest();
-
- /// There are values in int64 that have more than 53 significant bits
(in terms of double
- /// representation). Such values, being promoted to double, are
rounded up or down. If they are rounded up,
- /// the result may not fit in 64 bits.
- /// The example of such a number is 9.22337e+18.
- /// As to_Integral does a static_cast to int64_t, it may result in UB.
- /// The necessary check here is that FromDoubleIntermediateType has
enough significant (mantissa) bits to store the
- /// int64_t max value precisely.
-
- if (rhs > static_cast<FromDoubleIntermediateType>(min_int) &&
- rhs < static_cast<FromDoubleIntermediateType>(max_int)) {
- self = static_cast<int64_t>(rhs);
- return;
+ const int shift = exponent - static_cast<int>(fraction_bits);
+ if (shift < 0) {
+ self >>= -shift;
+ } else {
+ self <<= shift;
}
- const FromDoubleIntermediateType rhs_long_double =
- (static_cast<FromDoubleIntermediateType>(rhs) < 0)
- ? -static_cast<FromDoubleIntermediateType>(rhs)
- : rhs;
+ if ((bits & sign_mask) != 0) {
+ self = -self;
+ }
+ }
- set_multiplier(self, rhs_long_double);
+ constexpr static void wide_integer_from_builtin(integer<Bits, Signed>&
self,
+ float rhs) noexcept {
+ // Every Float32 value is exactly representable as a double.
+ wide_integer_from_builtin(self, static_cast<double>(rhs));
+ }
- if (rhs < 0) {
- self = -self;
+ constexpr static void wide_integer_from_builtin(integer<Bits, Signed>&
self,
+ long double rhs) noexcept {
+ // Direct long-double construction previously fell through the
integral helper and could
+ // perform an out-of-range cast to int64_t. Convert through the
well-defined double path.
+ constexpr long double max_double = std::numeric_limits<double>::max();
+ if (!(rhs >= -max_double && rhs <= max_double)) {
+ self = 0;
+ return;
}
+ wide_integer_from_builtin(self, static_cast<double>(rhs));
Review Comment:
fixed
##########
be/test/core/wide_integer_test.cpp:
##########
@@ -44,6 +47,40 @@ TEST(WideInteger, Conversions) {
1e72);
}
+TEST(WideInteger, FloatingPointConversions) {
+ constexpr wide::Int256 constexpr_value(123.75);
+ static_assert(constexpr_value.items[0] == 123);
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
