Skip to content

Use native ulong shifts when custom shift is not needed #50

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
May 15, 2025
Merged

Use native ulong shifts when custom shift is not needed #50

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d7429af
Use native shifts when possible
Hyodar May 9, 2025
14738e0
Fix custom shift not being used in relevant places
Hyodar May 9, 2025
f08fba3
Throw division by zero through separate method
Hyodar May 12, 2025
2def59d
Add debug shift amount asserts to native shift methods
Hyodar May 12, 2025
9740891
Remove message from ThrowDivideByZeroException
Hyodar May 14, 2025
3af9cb0
Merge branch 'main' into shift-optimization
Hyodar May 14, 2025
cdcfab9
Fix merge issue
Hyodar May 14, 2025
8e4c0a9
Specify behavior on Udivrem if d is zero
Hyodar May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 49 additions & 32 deletions src/Nethermind.Int256/UInt256.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,20 @@ internal static ulong Rsh(ulong a, int n)
return (a >> n1) >> n2;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static ulong NativeLsh(ulong a, int n)
{
return a << n;
Copy link
Contributor

@Scooletz Scooletz May 12, 2025

Choose a reason for hiding this comment

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

Could we Debug.Assert the requirement when this method can be called (n > 64).

Copy link
Contributor Author

@Hyodar Hyodar May 12, 2025

Choose a reason for hiding this comment

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

Sure! Done in 2def59d

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static ulong NativeRsh(ulong a, int n)
{
return a >> n;
Copy link
Contributor

@Scooletz Scooletz May 12, 2025

Choose a reason for hiding this comment

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

Could we Debug.Assert the requirement when this method can be called?

Copy link
Contributor Author

@Hyodar Hyodar May 12, 2025

Choose a reason for hiding this comment

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

Sure! Done in 2def59d

}

// Udivrem divides u by d and produces both quotient and remainder.
// It assumes d is not zero.
// The quotient is stored in provided quot - len(u)-len(d)+1 words.
// It loosely follows the Knuth's division algorithm (sometimes referenced as "schoolbook" division) using 64-bit words.
// See Knuth, Volume 2, section 4.3.1, Algorithm D.
Expand Down Expand Up @@ -689,6 +702,10 @@ private static void Udivrem(ref ulong quot, ref ulong u, int length, in UInt256
dLen = 1;
shift = LeadingZeros(d.u0);
}
else
{
throw new DivideByZeroException();
Copy link
Member

@benaadams benaadams May 12, 2025

Choose a reason for hiding this comment

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

Is this behaviour change?

Copy link
Member

@benaadams benaadams May 12, 2025

Choose a reason for hiding this comment

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

Looks like in Evm we check for 0 before calling divide, so should be ok (behaviour)

Copy link
Contributor

@Scooletz Scooletz May 12, 2025

Choose a reason for hiding this comment

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

Should this be a separate method, non inlinable?

Copy link
Contributor Author

@Hyodar Hyodar May 12, 2025

Choose a reason for hiding this comment

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

Done in f08fba3

Copy link
Contributor Author

@Hyodar Hyodar May 12, 2025

Choose a reason for hiding this comment

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

I used the existing method for throwing the exception - just followed the same as ThrowOverflowException and am now passing the message as argument

}

int uLen = 0;
for (int i = length - 1; i >= 0; i--)
Expand All @@ -704,36 +721,36 @@ private static void Udivrem(ref ulong quot, ref ulong u, int length, in UInt256
un[uLen] = Rsh(Unsafe.Add(ref u, uLen - 1), 64 - shift);
for (int i = uLen - 1; i > 0; i--)
{
un[i] = Lsh(Unsafe.Add(ref u, i), shift) | Rsh(Unsafe.Add(ref u, i - 1), 64 - shift);
un[i] = NativeLsh(Unsafe.Add(ref u, i), shift) | Rsh(Unsafe.Add(ref u, i - 1), 64 - shift);
}

un[0] = Lsh(u, shift);
un[0] = NativeLsh(u, shift);

// TODO: Skip the highest word of numerator if not significant.

if (dLen == 1)
{
ulong dnn0 = Lsh(d.u0, shift);
ulong dnn0 = NativeLsh(d.u0, shift);
ulong r = UdivremBy1(ref quot, un, dnn0);
r = Rsh(r, shift);
r = NativeRsh(r, shift);
rem = (UInt256)r;
return;
}

ulong dn0 = Lsh(d.u0, shift);
ulong dn0 = NativeLsh(d.u0, shift);
ulong dn1 = 0;
ulong dn2 = 0;
ulong dn3 = 0;
switch (dLen)
{
case 4:
dn3 = Lsh(d.u3, shift) | Rsh(d.u2, 64 - shift);
dn3 = NativeLsh(d.u3, shift) | Rsh(d.u2, 64 - shift);
goto case 3;
case 3:
dn2 = Lsh(d.u2, shift) | Rsh(d.u1, 64 - shift);
dn2 = NativeLsh(d.u2, shift) | Rsh(d.u1, 64 - shift);
goto case 2;
case 2:
dn1 = Lsh(d.u1, shift) | Rsh(d.u0, 64 - shift);
dn1 = NativeLsh(d.u1, shift) | Rsh(d.u0, 64 - shift);
break;
}
Span<ulong> dnS = stackalloc ulong[4] { dn0, dn1, dn2, dn3 };
Expand All @@ -745,25 +762,25 @@ private static void Udivrem(ref ulong quot, ref ulong u, int length, in UInt256
switch (dLen)
{
case 1:
rem0 = Rsh(un[dLen - 1], shift);
rem0 = NativeRsh(un[dLen - 1], shift);
goto r0;
case 2:
rem1 = Rsh(un[dLen - 1], shift);
rem1 = NativeRsh(un[dLen - 1], shift);
goto r1;
case 3:
rem2 = Rsh(un[dLen - 1], shift);
rem2 = NativeRsh(un[dLen - 1], shift);
goto r2;
case 4:
rem3 = Rsh(un[dLen - 1], shift);
rem3 = NativeRsh(un[dLen - 1], shift);
goto r3;
}

r3:
rem2 = Rsh(un[2], shift) | Lsh(un[3], 64 - shift);
rem2 = NativeRsh(un[2], shift) | Lsh(un[3], 64 - shift);
r2:
rem1 = Rsh(un[1], shift) | Lsh(un[2], 64 - shift);
rem1 = NativeRsh(un[1], shift) | Lsh(un[2], 64 - shift);
r1:
rem0 = Rsh(un[0], shift) | Lsh(un[1], 64 - shift);
rem0 = NativeRsh(un[0], shift) | Lsh(un[1], 64 - shift);
r0:

rem = new UInt256(rem0, rem1, rem2, rem3);
Expand Down Expand Up @@ -1426,8 +1443,8 @@ internal static (ulong quo, ulong rem) Div64(ulong hi, ulong lo, ulong y)

ulong yn1 = y >> 32;
ulong yn0 = y & mask32;
ulong un32 = Lsh(hi, s) | Rsh(lo, (64 - s));
ulong un10 = Lsh(lo, s);
ulong un32 = NativeLsh(hi, s) | Rsh(lo, (64 - s));
ulong un10 = NativeLsh(lo, s);
ulong un1 = un10 >> 32;
ulong un0 = un10 & mask32;
ulong q1 = un32 / yn1;
Expand Down Expand Up @@ -1457,7 +1474,7 @@ internal static (ulong quo, ulong rem) Div64(ulong hi, ulong lo, ulong y)
}
}

return (q1 * two32 + q0, Rsh((un21 * two32 + un0 - q0 * y), s));
return (q1 * two32 + q0, NativeRsh((un21 * two32 + un0 - q0 * y), s));
}

public static void Lsh(in UInt256 x, int n, out UInt256 res)
Expand Down Expand Up @@ -1517,19 +1534,19 @@ public static void Lsh(in UInt256 x, int n, out UInt256 res)
}

// remaining shifts
a = Rsh(res.u0, 64 - n);
z0 = Lsh(res.u0, n);
a = NativeRsh(res.u0, 64 - n);
z0 = NativeLsh(res.u0, n);

sh64:
b = Rsh(res.u1, 64 - n);
z1 = Lsh(res.u1, n) | a;
b = NativeRsh(res.u1, 64 - n);
z1 = NativeLsh(res.u1, n) | a;

sh128:
a = Rsh(res.u2, 64 - n);
z2 = Lsh(res.u2, n) | b;
a = NativeRsh(res.u2, 64 - n);
z2 = NativeLsh(res.u2, n) | b;
ulong z3;
sh192:
z3 = Lsh(res.u3, n) | a;
z3 = NativeLsh(res.u3, n) | a;

res = new UInt256(z0, z1, z2, z3);
}
Expand Down Expand Up @@ -1630,19 +1647,19 @@ public static void Rsh(in UInt256 x, int n, out UInt256 res)
}

// remaining shifts
a = Lsh(res.u3, 64 - n);
z3 = Rsh(res.u3, n);
a = NativeLsh(res.u3, 64 - n);
z3 = NativeRsh(res.u3, n);

sh64:
b = Lsh(res.u2, 64 - n);
z2 = Rsh(res.u2, n) | a;
b = NativeLsh(res.u2, 64 - n);
z2 = NativeRsh(res.u2, n) | a;

sh128:
a = Lsh(res.u1, 64 - n);
z1 = Rsh(res.u1, n) | b;
a = NativeLsh(res.u1, 64 - n);
z1 = NativeRsh(res.u1, n) | b;

sh192:
z0 = Rsh(res.u0, n) | a;
z0 = NativeRsh(res.u0, n) | a;

res = new UInt256(z0, z1, z2, z3);
}
Expand Down
Loading