Skip to content

Commit

Permalink
Minor cleanups for System.Runtime.Numerics (dotnet#53984)
Browse files Browse the repository at this point in the history
* Replace union with BitConverter

* Replace CombineHash with HashCode

* Replace CbitHighZero with LeadingZeroCount

* Remove another LeadingZeros

* More cleanup in BigInteger

* Cleanup complex

* Apply suggestions from code review

Co-authored-by: Tanner Gooding <tagoo@outlook.com>

* Convert body-less for to while

* Use HashCode.AddBytes

Co-authored-by: Tanner Gooding <tagoo@outlook.com>
  • Loading branch information
huoyaoyuan and tannergooding authored Aug 2, 2021
1 parent bb7e7f9 commit 65439de
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.InteropServices;

namespace System.Numerics
{
Expand Down Expand Up @@ -263,7 +264,7 @@ public BigInteger(ReadOnlySpan<byte> 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)
Expand Down Expand Up @@ -639,12 +640,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)
{
Expand Down Expand Up @@ -818,12 +819,12 @@ 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 = NumericsHelpers.CbitHighZero((uint)h);
int c = BitOperations.LeadingZeroCount((uint)h);
long b = (long)value._bits.Length * 32 - c;

// Extract most significant bits
Expand Down Expand Up @@ -989,19 +990,17 @@ public override int GetHashCode()

if (_bits == null)
return _sign;
int hash = _sign;
for (int iv = _bits.Length; --iv >= 0;)
hash = NumericsHelpers.CombineHash(hash, unchecked((int)_bits[iv]));
return hash;

HashCode hash = default;
hash.Add(_sign);
hash.AddBytes(MemoryMarshal.AsBytes(_bits.AsSpan()));
return hash.ToHashCode();
}

public override bool Equals([NotNullWhen(true)] object? obj)
{
AssertValid();

if (!(obj is BigInteger))
return false;
return Equals((BigInteger)obj);
return obj is BigInteger other && Equals(other);
}

public bool Equals(long other)
Expand Down Expand Up @@ -1124,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);
}

/// <summary>
Expand Down Expand Up @@ -1294,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.
Expand All @@ -1312,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;
Expand Down Expand Up @@ -1470,15 +1469,18 @@ private ReadOnlySpan<uint> ToUInt32Span(Span<uint> 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);
bool copyDwordsToScratch = true;
if (length <= scratch.Length)
{
scratch = scratch.Slice(0, length);
scratch = scratch[..length];
copyDwordsToScratch = !dwordsIsScratch;
}
else
Expand All @@ -1488,7 +1490,7 @@ private ReadOnlySpan<uint> ToUInt32Span(Span<uint> scratch)

if (copyDwordsToScratch)
{
dwords.Slice(0, msb + 1).CopyTo(scratch);
dwords[..(msb + 1)].CopyTo(scratch);
}

if (needExtraByte)
Expand Down Expand Up @@ -1818,11 +1820,11 @@ 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 = 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);
Expand Down Expand Up @@ -2416,7 +2418,7 @@ public long GetBitLength()
else
{
bitsArrayLength = bits.Length;
highValue = bits[bitsArrayLength - 1];
highValue = bits[^1];
}

long bitLength = bitsArrayLength * 32L - BitOperations.LeadingZeroCount(highValue);
Expand Down Expand Up @@ -2493,7 +2495,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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,23 +370,15 @@ 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 other && Equals(other);
}

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})";

Expand Down
Loading

0 comments on commit 65439de

Please sign in to comment.