Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor cleanups for System.Runtime.Numerics #53984

Merged
merged 10 commits into from
Aug 2, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,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];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this impact codegen at all? I could imagine this being equivalent to value.Length - 1, which might result in a secondary read from memory.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the size of this method, I think 1 length load should be OK

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably and for consistency, I'd just like to know if it is impactful at all, so we can more readily pin-point any changes in the perf issue triage.

isNegative = (mostSignificantByte & 0x80) != 0 && !isUnsigned;

if (mostSignificantByte == 0)
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -818,12 +818,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 +989,18 @@ 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();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

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--;
}
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2406,7 +2408,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 @@ -2483,7 +2485,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