From 874c04b291ea05e41cb05a24732b5f63524ab0f2 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 30 Aug 2018 10:09:59 -0700 Subject: [PATCH] Updating Number.Formatting to properly print -0 Commit migrated from https://github.com/dotnet/coreclr/commit/ad3c8cc8934ec89631c221713b86238f6c86e1b6 --- src/coreclr/src/classlibnative/bcltype/grisu3.cpp | 4 +- src/coreclr/src/classlibnative/bcltype/grisu3.h | 5 ++ src/coreclr/src/classlibnative/bcltype/number.cpp | 2 +- src/coreclr/src/classlibnative/bcltype/number.h | 20 ++++--- src/coreclr/src/pal/inc/pal.h | 2 + src/coreclr/src/pal/src/cruntime/math.cpp | 56 +++++++++++++++++++ .../src/System/Number.Formatting.cs | 62 ++++++++++++---------- .../src/System/Number.NumberBuffer.cs | 20 +++++-- 8 files changed, 130 insertions(+), 41 deletions(-) diff --git a/src/coreclr/src/classlibnative/bcltype/grisu3.cpp b/src/coreclr/src/classlibnative/bcltype/grisu3.cpp index 0fc9e88..e0138f6 100644 --- a/src/coreclr/src/classlibnative/bcltype/grisu3.cpp +++ b/src/coreclr/src/classlibnative/bcltype/grisu3.cpp @@ -44,7 +44,7 @@ bool Grisu3::Run(double value, int count, int* dec, int* sign, wchar_t* digits) // kappa: A factor used for generating digits. See step 5 of the Grisu3 procedure in the paper. // Handle sign bit. - if (value < 0) + if (_signbit(value) != 0) { value = -value; *sign = 1; @@ -378,4 +378,4 @@ void Grisu3::BiggestPowerTenLessThanOrEqualTo(UINT32 number, *exponent = 0; UNREACHABLE(); } -} \ No newline at end of file +} diff --git a/src/coreclr/src/classlibnative/bcltype/grisu3.h b/src/coreclr/src/classlibnative/bcltype/grisu3.h index b3c5a40..7f8ec82 100644 --- a/src/coreclr/src/classlibnative/bcltype/grisu3.h +++ b/src/coreclr/src/classlibnative/bcltype/grisu3.h @@ -12,6 +12,11 @@ #include "diyfp.h" +#ifdef _MSC_VER +#define _signbit signbit +#define _signbitf signbit +#endif + struct PowerOfTen { UINT64 significand; diff --git a/src/coreclr/src/classlibnative/bcltype/number.cpp b/src/coreclr/src/classlibnative/bcltype/number.cpp index b20044e..ea63122 100644 --- a/src/coreclr/src/classlibnative/bcltype/number.cpp +++ b/src/coreclr/src/classlibnative/bcltype/number.cpp @@ -288,7 +288,7 @@ void DoubleToNumberWorker( double value, int count, int* dec, int* sign, wchar_t if (value == 0.0) { *dec = 0; - *sign = 0; + *sign = _signbit(value); // Instead of zeroing digits, we just make it as an empty string due to performance reason. *digits = 0; diff --git a/src/coreclr/src/classlibnative/bcltype/number.h b/src/coreclr/src/classlibnative/bcltype/number.h index 22c74ca..480e6ad 100644 --- a/src/coreclr/src/classlibnative/bcltype/number.h +++ b/src/coreclr/src/classlibnative/bcltype/number.h @@ -19,13 +19,21 @@ static const double LOG10V2 = 0.30102999566398119521373889472449; // DRIFT_FACTOR = 1 - LOG10V2 - epsilon (a small number account for drift of floating point multiplication) static const double DRIFT_FACTOR = 0.69; +enum NUMBER_KIND : int { + NUMBER_KIND_Unknown = 0, + NUMBER_KIND_Integer = 1, + NUMBER_KIND_Decimal = 2, + NUMBER_KIND_Double = 3 +}; + struct NUMBER { - int precision; - int scale; - int sign; - wchar_t digits[NUMBER_MAXDIGITS + 1]; - wchar_t* allDigits; - NUMBER() : precision(0), scale(0), sign(0), allDigits(NULL) {} + int precision; // 0 + int scale; // 4 + int sign; // 8 + NUMBER_KIND kind; // 12 + wchar_t* allDigits; // 16 + wchar_t digits[NUMBER_MAXDIGITS + 1]; // 20 or 24 + NUMBER() : precision(0), scale(0), sign(0), kind(NUMBER_KIND_Unknown), allDigits(NULL) {} }; class COMNumber diff --git a/src/coreclr/src/pal/inc/pal.h b/src/coreclr/src/pal/inc/pal.h index e7ec886..2d0c471 100644 --- a/src/coreclr/src/pal/inc/pal.h +++ b/src/coreclr/src/pal/inc/pal.h @@ -4459,6 +4459,7 @@ PALIMPORT LONG __cdecl labs(LONG); // clang complains if this is declared with __int64 PALIMPORT long long __cdecl llabs(long long); +PALIMPORT int __cdecl _signbit(double); PALIMPORT int __cdecl _finite(double); PALIMPORT int __cdecl _isnan(double); PALIMPORT double __cdecl _copysign(double, double); @@ -4487,6 +4488,7 @@ PALIMPORT double __cdecl sqrt(double); PALIMPORT double __cdecl tan(double); PALIMPORT double __cdecl tanh(double); +PALIMPORT int __cdecl _signbitf(float); PALIMPORT int __cdecl _finitef(float); PALIMPORT int __cdecl _isnanf(float); PALIMPORT float __cdecl _copysignf(float, float); diff --git a/src/coreclr/src/pal/src/cruntime/math.cpp b/src/coreclr/src/pal/src/cruntime/math.cpp index a36ac9a..af2f994 100644 --- a/src/coreclr/src/pal/src/cruntime/math.cpp +++ b/src/coreclr/src/pal/src/cruntime/math.cpp @@ -45,6 +45,34 @@ SET_DEFAULT_DEBUG_CHANNEL(CRT); /*++ Function: + _signbit + +Determines whether given double-precision floating point value has a negative sign. + +Return Value + +_signbit returns a nonzero value (TRUE) if the sign of its argument x is negative. + +Parameter + +x Double-precision floating-point value + +--*/ +int __cdecl _signbit(double x) +{ + int ret; + PERF_ENTRY(_signbit); + ENTRY("_signbit (x=%f)\n", x); + + ret = signbit(x); + + LOGEXIT("_signbit returns int %d\n", ret); + PERF_EXIT(_signbit); + return ret; +} + +/*++ +Function: _finite Determines whether given double-precision floating point value is finite. @@ -455,6 +483,34 @@ PALIMPORT double __cdecl PAL_pow(double x, double y) /*++ Function: + _signbitf + +Determines whether given single-precision floating point value has a negative sign. + +Return Value + +_signbitf returns a nonzero value (TRUE) if the sign of its argument x is negative. + +Parameter + +x Single-precision floating-point value + +--*/ +int __cdecl _signbitf(float x) +{ + int ret; + PERF_ENTRY(_signbitf); + ENTRY("_signbitf (x=%f)\n", x); + + ret = signbit(x); + + LOGEXIT("_signbitf returns int %d\n", ret); + PERF_EXIT(_signbitf); + return ret; +} + +/*++ +Function: _finitef Determines whether given single-precision floating point value is finite. diff --git a/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs b/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs index 106fd15..3d3c15b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs @@ -301,7 +301,7 @@ namespace System if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, isDecimal:true); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -327,7 +327,7 @@ namespace System if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, isDecimal: true); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -342,6 +342,7 @@ namespace System char* buffer = number.digits; number.precision = DecimalPrecision; number.sign = d.IsNegative; + number.kind = NumberBufferKind.Decimal; char* p = buffer + DecimalPrecision; while ((d.Mid | d.High) != 0) @@ -388,6 +389,7 @@ namespace System char fmt = ParseFormatSpecifier(format, out int digits); int precision = DoublePrecision; NumberBuffer number = default; + number.kind = NumberBufferKind.Double; switch (fmt) { @@ -409,12 +411,12 @@ namespace System if (NumberToDouble(ref number) == value) { - NumberToString(ref sb, ref number, 'G', DoublePrecision, info, isDecimal: false); + NumberToString(ref sb, ref number, 'G', DoublePrecision, info); } else { DoubleToNumber(value, 17, ref number); - NumberToString(ref sb, ref number, 'G', 17, info, isDecimal: false); + NumberToString(ref sb, ref number, 'G', 17, info); } return null; @@ -451,7 +453,7 @@ namespace System if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, isDecimal: false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -488,6 +490,7 @@ namespace System char fmt = ParseFormatSpecifier(format, out int digits); int precision = FloatPrecision; NumberBuffer number = default; + number.kind = NumberBufferKind.Double; switch (fmt) { @@ -509,12 +512,12 @@ namespace System if ((float)NumberToDouble(ref number) == value) { - NumberToString(ref sb, ref number, 'G', FloatPrecision, info, isDecimal: false); + NumberToString(ref sb, ref number, 'G', FloatPrecision, info); } else { DoubleToNumber(value, 9, ref number); - NumberToString(ref sb, ref number, 'G', 9, info, isDecimal: false); + NumberToString(ref sb, ref number, 'G', 9, info); } return null; } @@ -550,7 +553,7 @@ namespace System if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -608,7 +611,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -653,7 +656,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -696,7 +699,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -739,7 +742,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -785,7 +788,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -831,7 +834,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -875,7 +878,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -919,7 +922,7 @@ namespace System } if (fmt != 0) { - NumberToString(ref sb, ref number, fmt, digits, info, false); + NumberToString(ref sb, ref number, fmt, digits, info); } else { @@ -949,6 +952,7 @@ namespace System int i = (int)(buffer + Int32Precision - p); number.scale = i; + number.kind = NumberBufferKind.Integer; char* dst = number.digits; while (--i >= 0) @@ -1065,6 +1069,7 @@ namespace System char* p = UInt32ToDecChars(buffer + UInt32Precision, value, 0); int i = (int)(buffer + UInt32Precision - p); number.scale = i; + number.kind = NumberBufferKind.Integer; char* dst = number.digits; while (--i >= 0) @@ -1184,6 +1189,7 @@ namespace System int i = (int)(buffer + Int64Precision - p); number.scale = i; + number.kind = NumberBufferKind.Integer; char* dst = number.digits; while (--i >= 0) @@ -1325,6 +1331,7 @@ namespace System int i = (int)(buffer + UInt64Precision - p); number.scale = i; + number.kind = NumberBufferKind.Integer; char* dst = number.digits; while (--i >= 0) @@ -1453,8 +1460,10 @@ namespace System '\0'; } - internal static unsafe void NumberToString(ref ValueStringBuilder sb, ref NumberBuffer number, char format, int nMaxDigits, NumberFormatInfo info, bool isDecimal) + internal static unsafe void NumberToString(ref ValueStringBuilder sb, ref NumberBuffer number, char format, int nMaxDigits, NumberFormatInfo info) { + Debug.Assert(number.kind != NumberBufferKind.Unknown); + switch (format) { case 'C': @@ -1522,14 +1531,9 @@ namespace System bool noRounding = false; if (nMaxDigits < 1) { - if (isDecimal && (nMaxDigits == -1)) + if ((number.kind == NumberBufferKind.Decimal) && (nMaxDigits == -1)) { noRounding = true; // Turn off rounding for ECMA compliance to output trailing 0's after decimal as significant - if (number.digits[0] == 0) - { - // Minus zero should be formatted as 0 - goto SkipSign; - } goto SkipRounding; } else @@ -1539,13 +1543,12 @@ namespace System } } - RoundNumber(ref number, nMaxDigits); // This also fixes up the minus zero case + RoundNumber(ref number, nMaxDigits); SkipRounding: if (number.sign) sb.Append(info.NegativeSign); -SkipSign: FormatGeneral(ref sb, ref number, nMaxDigits, info, (char)(format - ('G' - 'E')), noRounding); break; @@ -1572,6 +1575,8 @@ SkipSign: internal static unsafe void NumberToStringFormat(ref ValueStringBuilder sb, ref NumberBuffer number, ReadOnlySpan format, NumberFormatInfo info) { + Debug.Assert(number.kind != NumberBufferKind.Unknown); + int digitCount; int decimalPos; int firstDigit; @@ -1694,7 +1699,6 @@ SkipSign: } else { - number.sign = false; // We need to format -0 without the sign set. number.scale = 0; // Decimals with scale ('0.00') should be rounded. } @@ -2220,7 +2224,11 @@ SkipSign: if (i == 0) { number.scale = 0; - number.sign = false; + + if (number.kind == NumberBufferKind.Integer) + { + number.sign = false; + } } dig[i] = '\0'; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Number.NumberBuffer.cs b/src/libraries/System.Private.CoreLib/src/System/Number.NumberBuffer.cs index 2316f99..1730cf1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Number.NumberBuffer.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Number.NumberBuffer.cs @@ -15,17 +15,27 @@ namespace System [StructLayout(LayoutKind.Sequential, Pack = 1)] internal unsafe ref struct NumberBuffer // needs to match layout of NUMBER in coreclr's src/classlibnative/bcltype/number.h { - public int precision; - public int scale; - private int _sign; - private DigitsAndNullTerminator _digits; - private char* _allDigits; + public int precision; // 0 + public int scale; // 4 + private int _sign; // 8 + private NumberBufferKind _kind; // 12 + private char* _allDigits; // 16 + private DigitsAndNullTerminator _digits; // 20 or 24 public bool sign { get => _sign != 0; set => _sign = value ? 1 : 0; } public char* digits => (char*)Unsafe.AsPointer(ref _digits); + public NumberBufferKind kind { get => _kind; set => _kind = value; } [StructLayout(LayoutKind.Sequential, Size = (NumberMaxDigits + 1) * sizeof(char))] private struct DigitsAndNullTerminator { } } + + internal enum NumberBufferKind // needs to match NUMBER_KIND in coreclr's src/classlibnative/bcltype/number.h + { + Unknown = 0, + Integer = 1, + Decimal = 2, + Double = 3 + } } } -- 2.7.4