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.

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

Jump to bottom

Add some TODO #5213 comments #100954

Closed
wants to merge 8 commits into from
Closed

Add some TODO #5213 comments #100954

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
211230d
Add some TODO #5213 comments for
lilinus Apr 12, 2024
e0baac7
Merge branch 'main' into todo-5213-comments
lilinus Apr 12, 2024
7094cda
Merge branch 'main' into todo-5213-comments
lilinus Apr 16, 2024
5f1896b
Switch some TODOs to Math.DivRem
lilinus Apr 26, 2024
b70a69d
Merge branch 'todo-5213-comments' of https://github.com/lilinus/runti…
lilinus Apr 26, 2024
6eca043
Merge branch 'main' into todo-5213-comments
lilinus Apr 26, 2024
249cff3
Missed one switch to DivRem
lilinus Apr 26, 2024
87d3c95
Merge branch 'todo-5213-comments' of https://github.com/lilinus/runti…
lilinus Apr 26, 2024
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
3 changes: 3 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/DateTime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,7 @@ internal void GetDate(out int year, out int month, out int day)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void GetTime(out int hour, out int minute, out int second)
{
// TODO: https://github.com/dotnet/runtime/issues/5213
Copy link
Member

Choose a reason for hiding this comment

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

I'd honestly prefer these to instead just switch to using ulong.DivRem, that should produce the same code today and will automatically be updated in the future when any JIT optimization gets added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to DivRem except for in Decimal.DecCalc, where we the TODOs in this PR and the ones that already exists are heterogenous.

ulong seconds = UTicks / TicksPerSecond;
ulong minutes = seconds / 60;
second = (int)(seconds - (minutes * 60));
Expand All @@ -1416,6 +1417,7 @@ internal void GetTime(out int hour, out int minute, out int second)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void GetTime(out int hour, out int minute, out int second, out int millisecond)
{
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong milliseconds = UTicks / TicksPerMillisecond;
ulong seconds = milliseconds / 1000;
millisecond = (int)(milliseconds - (seconds * 1000));
Expand All @@ -1429,6 +1431,7 @@ internal void GetTime(out int hour, out int minute, out int second, out int mill
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void GetTimePrecise(out int hour, out int minute, out int second, out int tick)
{
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong ticks = UTicks;
ulong seconds = ticks / TicksPerSecond;
tick = (int)(ticks - (seconds * TicksPerSecond));
Expand Down
7 changes: 7 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Decimal.DecCalc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ private static uint Div96By32(ref Buf12 bufNum, uint den)
private static bool Div96ByConst(ref ulong high64, ref uint low, uint pow)
{
#if TARGET_64BIT
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong div64 = high64 / pow;
uint div = (uint)((((high64 - div64 * pow) << 32) + low) / pow);
if (low == div * pow)
Expand Down Expand Up @@ -675,6 +676,7 @@ private static unsafe uint DivByConst(uint* result, uint hiRes, out uint quotien
for (uint i = hiRes - 1; (int)i >= 0; i--)
{
#if TARGET_64BIT
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong num = result[i] + ((ulong)remainder << 32);
remainder = (uint)num - (result[i] = (uint)(num / power)) * power;
#else
Expand Down Expand Up @@ -710,6 +712,7 @@ private static int OverflowUnscale(ref Buf12 bufQuo, int scale, bool sticky)
Debug.Assert(bufQuo.U2 == 0);

// We have overflown, so load the high bit with a one.
// TODO: https://github.com/dotnet/runtime/issues/5213
const ulong highbit = 1UL << 32;
bufQuo.U2 = (uint)(highbit / 10);
ulong tmp = ((highbit % 10) << 32) + bufQuo.U1;
Expand Down Expand Up @@ -1090,6 +1093,7 @@ internal static unsafe void DecAddSub(ref DecCalc d1, ref DecCalc d2, bool sign)
Number.ThrowOverflowException(SR.Overflow_Decimal);
flags -= 1 << ScaleShift;

// TODO: https://github.com/dotnet/runtime/issues/5213
const uint den = 10;
ulong num = high + (1UL << 32);
high = (uint)(num / den);
Expand Down Expand Up @@ -1776,6 +1780,7 @@ internal static void VarDecFromR8(double input, out DecCalc result)
if (lmax > 14)
lmax = 14;

// TODO: https://github.com/dotnet/runtime/issues/5213
if ((byte)mant == 0 && lmax >= 8)
{
const uint den = 100000000;
Expand Down Expand Up @@ -2349,6 +2354,7 @@ internal static void InternalRound(ref DecCalc d, uint scale, MidpointRounding m
{
scale -= MaxInt32Scale;

// TODO: https://github.com/dotnet/runtime/issues/5213
const uint divisor = TenToPowerNine;
uint n = d.uhi;
if (n == 0)
Expand Down Expand Up @@ -2463,6 +2469,7 @@ internal static uint DecDivMod1E9(ref DecCalc value)
value.uhi = (uint)(div64 >> 32);
value.umid = (uint)div64;

// TODO: https://github.com/dotnet/runtime/issues/5213
ulong num = ((high64 - (uint)div64 * TenToPowerNine) << 32) + value.ulo;
uint div = (uint)(num / TenToPowerNine);
value.ulo = div;
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime.Numerics/src/System/Numerics/BigIntegerCalculator.DivRem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public static void Divide(ReadOnlySpan<uint> left, uint right, Span<uint> quotie

for (int i = left.Length - 1; i >= 0; i--)
{
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong value = (carry << 32) | left[i];
ulong digit = value / right;
quotient[i] = (uint)digit;
Expand All @@ -39,6 +40,7 @@ public static void Divide(ReadOnlySpan<uint> left, uint right, Span<uint> quotie
ulong carry = 0UL;
for (int i = left.Length - 1; i >= 0; i--)
{
// TODO: https://github.com/dotnet/runtime/issues/5213
ulong value = (carry << 32) | left[i];
ulong digit = value / right;
quotient[i] = (uint)digit;
Expand Down
Loading