From a31cb617dbe800763d19b924ad583570dbb4d699 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 9 Jun 2021 18:21:25 +0800 Subject: [PATCH 1/9] Replace union with BitConverter --- .../src/System/Numerics/NumericsHelpers.cs | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs index e89bc4acd13e8..28d2fe930f16c 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs @@ -2,32 +2,20 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Runtime.InteropServices; namespace System.Numerics { - [StructLayout(LayoutKind.Explicit)] - internal struct DoubleUlong - { - [FieldOffset(0)] - public double dbl; - [FieldOffset(0)] - public ulong uu; - } - internal static class NumericsHelpers { private const int kcbitUint = 32; public static void GetDoubleParts(double dbl, out int sign, out int exp, out ulong man, out bool fFinite) { - DoubleUlong du; - du.uu = 0; - du.dbl = dbl; + ulong bits = BitConverter.DoubleToUInt64Bits(dbl); - sign = 1 - ((int)(du.uu >> 62) & 2); - man = du.uu & 0x000FFFFFFFFFFFFF; - exp = (int)(du.uu >> 52) & 0x7FF; + sign = 1 - ((int)(bits >> 62) & 2); + man = bits & 0x000FFFFFFFFFFFFF; + exp = (int)(bits >> 52) & 0x7FF; if (exp == 0) { // Denormalized number. @@ -51,11 +39,10 @@ public static void GetDoubleParts(double dbl, out int sign, out int exp, out ulo public static double GetDoubleFromParts(int sign, int exp, ulong man) { - DoubleUlong du; - du.dbl = 0; + ulong bits; if (man == 0) - du.uu = 0; + bits = 0; else { // Normalize so that 0x0010 0000 0000 0000 is the highest bit set. @@ -74,7 +61,7 @@ public static double GetDoubleFromParts(int sign, int exp, ulong man) if (exp >= 0x7FF) { // Infinity. - du.uu = 0x7FF0000000000000; + bits = 0x7FF0000000000000; } else if (exp <= 0) { @@ -83,25 +70,25 @@ public static double GetDoubleFromParts(int sign, int exp, ulong man) if (exp < -52) { // Underflow to zero. - du.uu = 0; + bits = 0; } else { - du.uu = man >> -exp; - Debug.Assert(du.uu != 0); + bits = man >> -exp; + Debug.Assert(bits != 0); } } else { // Mask off the implicit high bit. - du.uu = (man & 0x000FFFFFFFFFFFFF) | ((ulong)exp << 52); + bits = (man & 0x000FFFFFFFFFFFFF) | ((ulong)exp << 52); } } if (sign < 0) - du.uu |= 0x8000000000000000; + bits |= 0x8000000000000000; - return du.dbl; + return BitConverter.UInt64BitsToDouble(bits); } // Do an in-place two's complement. "Dangerous" because it causes From e3108be7141f3bc64b30b41bfb3dda5d92714cc5 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 9 Jun 2021 18:24:02 +0800 Subject: [PATCH 2/9] Replace CombineHash with HashCode --- .../src/System/Numerics/BigInteger.cs | 8 +++++--- .../src/System/Numerics/NumericsHelpers.cs | 10 ---------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index aa90a8bbb1f32..86d7c072bb27c 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -989,10 +989,12 @@ public override int GetHashCode() if (_bits == null) return _sign; - int hash = _sign; + + HashCode hash = default; + hash.Add(_sign); for (int iv = _bits.Length; --iv >= 0;) - hash = NumericsHelpers.CombineHash(hash, unchecked((int)_bits[iv])); - return hash; + hash.Add(_bits[iv]); + return hash.ToHashCode(); } public override bool Equals([NotNullWhen(true)] object? obj) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs index 28d2fe930f16c..72c177ecb6e3d 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs @@ -127,16 +127,6 @@ public static uint Abs(int a) } } - public static uint CombineHash(uint u1, uint u2) - { - return ((u1 << 7) | (u1 >> 25)) ^ u2; - } - - public static int CombineHash(int n1, int n2) - { - return unchecked((int)CombineHash((uint)n1, (uint)n2)); - } - public static int CbitHighZero(uint u) { if (u == 0) From 2f79a0a5579ae86ee11df1c11d0701b353031850 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 9 Jun 2021 18:30:23 +0800 Subject: [PATCH 3/9] Replace CbitHighZero with LeadingZeroCount --- .../src/System/Numerics/BigInteger.cs | 4 +- .../src/System/Numerics/NumericsHelpers.cs | 40 +------------------ 2 files changed, 3 insertions(+), 41 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 86d7c072bb27c..9af0a60685513 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -823,7 +823,7 @@ public static double Log(BigInteger value, double baseValue) ulong l = value._bits.Length > 2 ? value._bits[value._bits.Length - 3] : 0; // Measure the exact bit count - int c = NumericsHelpers.CbitHighZero((uint)h); + int c = BitOperations.LeadingZeroCount((uint)h); long b = (long)value._bits.Length * 32 - c; // Extract most significant bits @@ -1824,7 +1824,7 @@ public static explicit operator double(BigInteger value) ulong m = length > 1 ? bits[length - 2] : 0; ulong l = length > 2 ? bits[length - 3] : 0; - int z = NumericsHelpers.CbitHighZero((uint)h); + int z = BitOperations.LeadingZeroCount((uint)h); int exp = (length - 2) * 32 - z; ulong man = (h << 32 + z) | (m << z) | (l >> 32 - z); diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs index 72c177ecb6e3d..0926f69e2ba68 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/NumericsHelpers.cs @@ -46,7 +46,7 @@ public static double GetDoubleFromParts(int sign, int exp, ulong man) else { // Normalize so that 0x0010 0000 0000 0000 is the highest bit set. - int cbitShift = CbitHighZero(man) - 11; + int cbitShift = BitOperations.LeadingZeroCount(man) - 11; if (cbitShift < 0) man >>= -cbitShift; else @@ -126,43 +126,5 @@ public static uint Abs(int a) return ((uint)a ^ mask) - mask; } } - - public static int CbitHighZero(uint u) - { - if (u == 0) - return 32; - - int cbit = 0; - if ((u & 0xFFFF0000) == 0) - { - cbit += 16; - u <<= 16; - } - if ((u & 0xFF000000) == 0) - { - cbit += 8; - u <<= 8; - } - if ((u & 0xF0000000) == 0) - { - cbit += 4; - u <<= 4; - } - if ((u & 0xC0000000) == 0) - { - cbit += 2; - u <<= 2; - } - if ((u & 0x80000000) == 0) - cbit += 1; - return cbit; - } - - public static int CbitHighZero(ulong uu) - { - if ((uu & 0xFFFFFFFF00000000) == 0) - return 32 + CbitHighZero((uint)uu); - return CbitHighZero((uint)(uu >> 32)); - } } } From 4bfe2fffa95b1465a5ed5c4a431972990c5368fb Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 9 Jun 2021 20:38:59 +0800 Subject: [PATCH 4/9] Remove another LeadingZeros --- .../Numerics/BigIntegerCalculator.DivRem.cs | 36 +------------------ .../Numerics/BigIntegerCalculator.GcdInv.cs | 2 +- 2 files changed, 2 insertions(+), 36 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs index bb75e77a228d0..de1001d7f4f38 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs @@ -173,7 +173,7 @@ private static unsafe void Divide(uint* left, int leftLength, uint divLo = rightLength > 1 ? right[rightLength - 2] : 0; // We measure the leading zeros of the divisor - int shift = LeadingZeros(divHi); + int shift = BitOperations.LeadingZeroCount(divHi); int backShift = 32 - shift; // And, we make sure the most significant bit is set @@ -316,39 +316,5 @@ private static bool DivideGuessTooBig(ulong q, ulong valHi, uint valLo, return false; } - - private static int LeadingZeros(uint value) - { - if (value == 0) - return 32; - - int count = 0; - if ((value & 0xFFFF0000) == 0) - { - count += 16; - value = value << 16; - } - if ((value & 0xFF000000) == 0) - { - count += 8; - value = value << 8; - } - if ((value & 0xF0000000) == 0) - { - count += 4; - value = value << 4; - } - if ((value & 0xC0000000) == 0) - { - count += 2; - value = value << 2; - } - if ((value & 0x80000000) == 0) - { - count += 1; - } - - return count; - } } } diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.GcdInv.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.GcdInv.cs index d984912ec6246..9368891c0428e 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.GcdInv.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.GcdInv.cs @@ -238,7 +238,7 @@ private static void ExtractDigits(ref BitsBuffer xBuffer, } // Use all the bits but one, see [hac] 14.58 (ii) - int z = LeadingZeros((uint)xh); + int z = BitOperations.LeadingZeroCount((uint)xh); x = ((xh << 32 + z) | (xm << z) | (xl >> 32 - z)) >> 1; y = ((yh << 32 + z) | (ym << z) | (yl >> 32 - z)) >> 1; From 86b08e54b83b0585d7f8d3ff0a8fcaf7a54c4301 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 10 Jun 2021 15:37:32 +0800 Subject: [PATCH 5/9] More cleanup in BigInteger --- .../src/System/Numerics/BigInteger.cs | 43 +++++++++---------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 9af0a60685513..f5162429676a1 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -263,7 +263,7 @@ public BigInteger(ReadOnlySpan value, bool isUnsigned = false, bool isBigE bool isNegative; if (byteCount > 0) { - byte mostSignificantByte = isBigEndian ? value[0] : value[byteCount - 1]; + byte mostSignificantByte = isBigEndian ? value[0] : value[^1]; isNegative = (mostSignificantByte & 0x80) != 0 && !isUnsigned; if (mostSignificantByte == 0) @@ -639,12 +639,12 @@ public bool IsPowerOfTwo AssertValid(); if (_bits == null) - return (_sign & (_sign - 1)) == 0 && _sign != 0; + return BitOperations.IsPow2(_sign); if (_sign != 1) return false; int iu = _bits.Length - 1; - if ((_bits[iu] & (_bits[iu] - 1)) != 0) + if (!BitOperations.IsPow2(_bits[iu])) return false; while (--iu >= 0) { @@ -818,9 +818,9 @@ public static double Log(BigInteger value, double baseValue) if (value._bits == null) return Math.Log(value._sign, baseValue); - ulong h = value._bits[value._bits.Length - 1]; - ulong m = value._bits.Length > 1 ? value._bits[value._bits.Length - 2] : 0; - ulong l = value._bits.Length > 2 ? value._bits[value._bits.Length - 3] : 0; + ulong h = value._bits[^1]; + ulong m = value._bits.Length > 1 ? value._bits[^2] : 0; + ulong l = value._bits.Length > 2 ? value._bits[^3] : 0; // Measure the exact bit count int c = BitOperations.LeadingZeroCount((uint)h); @@ -1000,10 +1000,7 @@ public override int GetHashCode() public override bool Equals([NotNullWhen(true)] object? obj) { AssertValid(); - - if (!(obj is BigInteger)) - return false; - return Equals((BigInteger)obj); + return obj is BigInteger bigInt && Equals(bigInt); } public bool Equals(long other) @@ -1126,9 +1123,9 @@ public int CompareTo(object? obj) { if (obj == null) return 1; - if (!(obj is BigInteger)) + if (obj is not BigInteger bigInt) throw new ArgumentException(SR.Argument_MustBeBigInt, nameof(obj)); - return CompareTo((BigInteger)obj); + return CompareTo(bigInt); } /// @@ -1296,13 +1293,13 @@ private enum GetBytesMode { AllocateArray, Count, Span } // because a bits array of all zeros would represent 0, and this case // would be encoded as _bits = null and _sign = 0. Debug.Assert(bits.Length > 0); - Debug.Assert(bits[bits.Length - 1] != 0); + Debug.Assert(bits[^1] != 0); while (bits[nonZeroDwordIndex] == 0U) { nonZeroDwordIndex++; } - highDword = ~bits[bits.Length - 1]; + highDword = ~bits[^1]; if (bits.Length - 1 == nonZeroDwordIndex) { // This will not overflow because highDword is less than or equal to uint.MaxValue - 1. @@ -1314,7 +1311,7 @@ private enum GetBytesMode { AllocateArray, Count, Span } { Debug.Assert(sign == 1); highByte = 0x00; - highDword = bits[bits.Length - 1]; + highDword = bits[^1]; } byte msb; @@ -1473,14 +1470,14 @@ private ReadOnlySpan ToUInt32Span(Span scratch) // Find highest significant byte and ensure high bit is 0 if positive, 1 if negative int msb; - for (msb = dwords.Length - 1; msb > 0 && dwords[msb] == highDWord; msb--); + for (msb = dwords.Length - 1; msb > 0 && dwords[msb] == highDWord; msb--) ; bool needExtraByte = (dwords[msb] & 0x80000000) != (highDWord & 0x80000000); int length = msb + 1 + (needExtraByte ? 1 : 0); bool copyDwordsToScratch = true; if (length <= scratch.Length) { - scratch = scratch.Slice(0, length); + scratch = scratch[..length]; copyDwordsToScratch = !dwordsIsScratch; } else @@ -1490,7 +1487,7 @@ private ReadOnlySpan ToUInt32Span(Span scratch) if (copyDwordsToScratch) { - dwords.Slice(0, msb + 1).CopyTo(scratch); + dwords[..(msb + 1)].CopyTo(scratch); } if (needExtraByte) @@ -1820,9 +1817,9 @@ public static explicit operator double(BigInteger value) return double.NegativeInfinity; } - ulong h = bits[length - 1]; - ulong m = length > 1 ? bits[length - 2] : 0; - ulong l = length > 2 ? bits[length - 3] : 0; + ulong h = bits[^1]; + ulong m = length > 1 ? bits[^2] : 0; + ulong l = length > 2 ? bits[^3] : 0; int z = BitOperations.LeadingZeroCount((uint)h); @@ -2408,7 +2405,7 @@ public long GetBitLength() else { bitsArrayLength = bits.Length; - highValue = bits[bitsArrayLength - 1]; + highValue = bits[^1]; } long bitLength = bitsArrayLength * 32L - BitOperations.LeadingZeroCount(highValue); @@ -2485,7 +2482,7 @@ private void AssertValid() // Wasted space: _bits[0] could have been packed into _sign Debug.Assert(_bits.Length > 1 || _bits[0] >= kuMaskHighBit); // Wasted space: leading zeros could have been truncated - Debug.Assert(_bits[_bits.Length - 1] != 0); + Debug.Assert(_bits[^1] != 0); } else { From 7e37a19b1c99acf335b29f9d6a883671f2d3eb97 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 10 Jun 2021 15:43:01 +0800 Subject: [PATCH 6/9] Cleanup complex --- .../src/System/Numerics/Complex.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs index 9325be0d806c2..5ad072fe4204e 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs @@ -370,8 +370,7 @@ public static Complex Reciprocal(Complex value) public override bool Equals([NotNullWhen(true)] object? obj) { - if (!(obj is Complex)) return false; - return Equals((Complex)obj); + return obj is Complex c && Equals(c); } public bool Equals(Complex value) @@ -379,14 +378,7 @@ public bool Equals(Complex value) return m_real.Equals(value.m_real) && m_imaginary.Equals(value.m_imaginary); } - public override int GetHashCode() - { - int n1 = 99999997; - int realHash = m_real.GetHashCode() % n1; - int imaginaryHash = m_imaginary.GetHashCode(); - int finalHash = realHash ^ imaginaryHash; - return finalHash; - } + public override int GetHashCode() => HashCode.Combine(m_real, m_imaginary); public override string ToString() => $"({m_real}, {m_imaginary})"; From 3022198dd65e2a45ee06c9ad5112df3bb5474639 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 10 Jun 2021 23:56:20 +0800 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: Tanner Gooding --- .../System.Runtime.Numerics/src/System/Numerics/BigInteger.cs | 2 +- .../System.Runtime.Numerics/src/System/Numerics/Complex.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index f5162429676a1..becae20e21277 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -1000,7 +1000,7 @@ public override int GetHashCode() public override bool Equals([NotNullWhen(true)] object? obj) { AssertValid(); - return obj is BigInteger bigInt && Equals(bigInt); + return obj is BigInteger other && Equals(other); } public bool Equals(long other) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs index 5ad072fe4204e..8ec356e6dc215 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/Complex.cs @@ -370,7 +370,7 @@ public static Complex Reciprocal(Complex value) public override bool Equals([NotNullWhen(true)] object? obj) { - return obj is Complex c && Equals(c); + return obj is Complex other && Equals(other); } public bool Equals(Complex value) From b450c7a9738ce657d78a6f48fa7e7037b81bf66f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Fri, 11 Jun 2021 00:12:01 +0800 Subject: [PATCH 8/9] Convert body-less for to while --- .../src/System/Numerics/BigInteger.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index becae20e21277..27f7c746c843a 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -1469,8 +1469,11 @@ private ReadOnlySpan ToUInt32Span(Span scratch) } // Find highest significant byte and ensure high bit is 0 if positive, 1 if negative - int msb; - for (msb = dwords.Length - 1; msb > 0 && dwords[msb] == highDWord; msb--) ; + int msb = dwords.Length - 1; + while (msb > 0 && dwords[msb] == highDWord) + { + msb--; + } bool needExtraByte = (dwords[msb] & 0x80000000) != (highDWord & 0x80000000); int length = msb + 1 + (needExtraByte ? 1 : 0); From c2864abe90466efc9ab768100023900e0e196db9 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 16 Jun 2021 21:40:46 +0800 Subject: [PATCH 9/9] Use HashCode.AddBytes --- .../System.Runtime.Numerics/src/System/Numerics/BigInteger.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs index 27f7c746c843a..17bd834a13681 100644 --- a/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs +++ b/src/libraries/System.Runtime.Numerics/src/System/Numerics/BigInteger.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Runtime.InteropServices; namespace System.Numerics { @@ -992,8 +993,7 @@ public override int GetHashCode() HashCode hash = default; hash.Add(_sign); - for (int iv = _bits.Length; --iv >= 0;) - hash.Add(_bits[iv]); + hash.AddBytes(MemoryMarshal.AsBytes(_bits.AsSpan())); return hash.ToHashCode(); }