JIT: Wrong result with double comparison

[jakobbotsch]

// Generated by Fuzzlyn v2.4 on 2025-02-23 17:30:17
// Run on X64 Windows
// Seed: 18098778274409643257-vectort,vector128,vector256,x86aes,x86avx,x86avx2,x86avx512bw,x86avx512bwvl,x86avx512cd,x86avx512cdvl,x86avx512dq,x86avx512dqvl,x86avx512f,x86avx512fvl,x86avx512fx64,x86bmi1,x86bmi1x64,x86bmi2,x86bmi2x64,x86fma,x86lzcnt,x86lzcntx64,x86pclmulqdq,x86popcnt,x86popcntx64,x86sse,x86ssex64,x86sse2,x86sse2x64,x86sse3,x86sse41,x86sse41x64,x86sse42,x86sse42x64,x86ssse3,x86x86base
// Reduced from 23.4 KiB to 0.4 KiB in 00:01:16
// Debug: Prints 0 line(s)
// Release: Prints 1 line(s)
using System;
using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.X86;

public class Program
{
    public static short s_1;
    public static void Main()
    {
        ushort vr0 = default(ushort);
        s_1 = -1;
        if (((double)0 <= (uint)s_1))
        {
            System.Console.WriteLine(vr0);
        }
    }
}

cc @dotnet/jit-contrib

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[ShreyasJejurkar]

I checked in SharpLab, I don't see that happening. (Debug/Release switch is on top corner RHS)

https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEUCuA7NAExAGoAfAAQCYAGAWACgKBmAAmvYEYB2Rgb0ash7NhU5JWAZwAW0DFID6nANyDh1KmqECGwvaxwy5rAG5QarALytCMAGYBDHABsMACkOyoGAJSrd+kKSSlascCpa+gCWdqxu8YQQOMDOMD4WADzWHlF4vsGcPj6RejqBgWIAdGIAnG5mNH4lwgC+kW0MLUA==

[hez2010]

Can repro on godbolt (the one on the right) with

DOTNET_TieredCompilation=0
DOTNET_JITMinOpts=1
DOTNET_JITMinOptsName=*

https://godbolt.org/z/er1G16Yx8

Interestingly, the issue will disappear if you turn on tiered compilation (the one on the left).

[saucecontrol]

This is another one exposed by #111595

Without AVX-512, we don't allow uint->floating casts directly -- they go through long:

               [000008] ----G+-----                            \--*  CAST      double <- long
               [000015] ----G+---U-                               \--*  CAST      long <- uint
               [000007] n---G+-----                                  \--*  IND       short 
               [000006] H----+-----                                     \--*  CNS_INT(h) long   0x7ffcea03b220 static Fseq[s_1]

With it, we don't force the intermediate cast:

               [000008] ----G----U-                            \--*  CAST      double <- uint
               [000007] n---G------                               \--*  IND       short 
               [000006] H----------                                  \--*  CNS_INT(h) long   0x7ffcea03b220 static Fseq[s_1]

In lowering, uint->floating casts where the base type is small get an intermediate uint->int cast inserted:

runtime/src/coreclr/jit/lowerxarch.cpp

Lines 1167 to 1173 in 509c12b

	// Case of src is a small type and dst is a floating point type.
	if (varTypeIsSmall(srcType) && varTypeIsFloating(castToType))
	{
	// These conversions can never be overflow detecting ones.
	noway_assert(!tree->gtOverflow());
	tmpType = TYP_INT;
	}

So we get this transform:

lowering Node before:
N001 (???,???) [000006] Hc---+-----                    t6 =    CNS_INT(h) long   0x7ffcea03b220 static Fseq[s_1]
                                                            /--*  t6     long   
N002 (???,???) [000007] n---G+-----                    t7 = *  IND       short 
                                                            /--*  t7     short  
N003 (???,???) [000008] ----G+---U-                    t8 = *  CAST      double <- uint

lowering Node after:
N001 (???,???) [000006] Hc---+-----                    t6 =    CNS_INT(h) long   0x7ffcea03b220 static Fseq[s_1]
                                                            /--*  t6     long   
N002 (???,???) [000007] n---G+-----                    t7 = *  IND       short 
                                                            /--*  t7     short  
               [000022] ----G----U-                   t22 = *  CAST      int <- uint
                                                            /--*  t22    int    
N003 (???,???) [000008] ----G+-----                    t8 = *  CAST      double <- int

And we end up emitting the signed int conversion instruction instead of unsigned.

[jakobbotsch]

In lowering, uint->floating casts where the base type is small get an intermediate uint->int cast inserted:

runtime/src/coreclr/jit/lowerxarch.cpp

Lines 1167 to 1173 in 509c12b

// Case of src is a small type and dst is a floating point type.
if (varTypeIsSmall(srcType) && varTypeIsFloating(castToType))
{
// These conversions can never be overflow detecting ones.
noway_assert(!tree->gtOverflow());
tmpType = TYP_INT;
}

That code just looks bogus, using the non-actual type of an operand to make any semantic changes in codegen is just wrong.

[saucecontrol]

Agreed, I can't figure out why that was there, because uint->floating casts shouldn't have made it past the front end in the first place before #111595.

[saucecontrol]

That code just looks bogus

It was indeed bogus. One of the casts it was trying to insert (float->small -> float->int->small) was already handled in morph. The one for the other direction was unnecessary as long as ContainCheckCast and genIntToFloatCast do the right thing.

Opened #112892 with the fix