UInt64 to Double conversions are incorrect

[tannergooding]

Conversions from UInt64 to Double for values greater than long.MaxValue are currently incorrect. The same issue may exist for UInt32 to Double for values greater than int.MaxValue but I have not confirmed.

The simple repro case is detailed here: dotnet/csharplang#4074

using System;
namespace App
{
    internal static class Program
    {
        internal static void Main()
        {
            /// Displays 'True'.
           Console.WriteLine(10648738977740919977d != 10648738977740919977);
        }
    }
}

You can see approximately what the runtime generates by looking at: https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWzQBMQBqAHwAEAmARgFgAoSgZgAIa2BhNgbybaCO7IhACuwADYw2AWQAUYyRAB2AczYIAlHwFD9lAOyaA3HsEBfJhaA===

public double M(ulong x) {
    return x;
}

Which results in:

    L0000: vzeroupper
    L0003: vxorps xmm0, xmm0, xmm0
    L0007: vcvtsi2sd xmm0, xmm0, rdx
    L000c: test rdx, rdx
    L000f: jge short L0019
    L0011: vaddsd xmm0, xmm0, [C.M(UInt64)]
    L0019: ret

The constant used by L0011 appears to be 1.8446744073709552E+19 (0x43f0000000000000)

category:correctness
theme:optimization
skill-level:beginner
cost:small
impact:small

[tannergooding]

This looks to be a "known" issue and was done for "back compat": https://github.com/dotnet/runtime/blob/master/src/coreclr/src/jit/codegenxarch.cpp#L6435-L6468

Clang vs MSVC generate different code with the MSVC variant being similar to our own, but which produces a correct result: https://godbolt.org/z/P7j6n8

[tannergooding]

Given this is .NET Core and given this is a correctness issue; I would say we take the breaking change and ensure we generate the correct IEEE compliant conversion.

[briansull]

I will investigate

[briansull]

This is the backwartds compat comment in the JIT source code:

        // The instruction sequence below is less accurate than what clang
        // and gcc generate. However, we keep the current sequence for backward compatibility.
        // If we change the instructions below, FloatingPointUtils::convertUInt64ToDobule
        // should be also updated for consistent conversion result.

[JulieLeeMSFT]

@briansull to discuss with @davidwrighton

[JulieLeeMSFT]

CC @dotnet/jit-contrib

[JulieLeeMSFT]

@davidwrighton recommends to fix this issue.
#47478 issues on Double to Int conversion will be handled separately.

[JulieLeeMSFT]

Moved to .NET 7 as priority 1. For Vectors handling, consider a way to make it work without control flow.

CC @dotnet/jit-contrib

[JulieLeeMSFT]

Moving to .NET 8.

[JulieLeeMSFT]

@tannergooding, assigning this to you for now. Please feel free to move out to Future.

[tannergooding]

This got moved to early .NET 9 so we can handle all the "breaking changes" at once and early in the release