Assertion failed '!"Should not see phi nodes after rationalize"' during 'Lowering nodeinfo'

[jakobbotsch]

// Generated by Fuzzlyn v1.5 on 2022-11-04 01:59:14
// Run on Arm64 Linux
// Seed: 4397362897463137173
// Reduced from 88.3 KiB to 0.6 KiB in 00:01:58
// Hits JIT assert in Release:
// Assertion failed '!"Should not see phi nodes after rationalize"' in 'Program:M37(byte,bool,short)' during 'Lowering nodeinfo' (IL size 42; hash 0x747e58b5; FullOpts)
// 
//     File: /__w/1/s/src/coreclr/jit/lower.cpp Line: 6634
// 
public class Program
{
    public static long[] s_3;
    public static sbyte s_11;
    public static uint s_17;
    public static short s_18;
    public static void Main()
    {
        bool vr6 = default(bool);
        M37(s_11, vr6, s_18);
    }

    public static void M37(sbyte arg0, bool arg1, short arg2)
    {
        if (!arg1)
        {
            uint vr8 = s_17;
            arg1 = 0 <= s_3[0];
        }

        if (!arg1)
        {
            arg0 = s_11;
        }

        if (arg1)
        {
            arg2 = 0;
        }
    }
}

Fallout from #73472, cc @a74nh

[ghost]

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

Issue Details

---

// Generated by Fuzzlyn v1.5 on 2022-11-04 01:59:14
// Run on Arm64 Linux
// Seed: 4397362897463137173
// Reduced from 88.3 KiB to 0.6 KiB in 00:01:58
// Hits JIT assert in Release:
// Assertion failed '!"Should not see phi nodes after rationalize"' in 'Program:M37(byte,bool,short)' during 'Lowering nodeinfo' (IL size 42; hash 0x747e58b5; FullOpts)
// 
//     File: /__w/1/s/src/coreclr/jit/lower.cpp Line: 6634
// 
public class Program
{
    public static long[] s_3;
    public static sbyte s_11;
    public static uint s_17;
    public static short s_18;
    public static void Main()
    {
        bool vr6 = default(bool);
        M37(s_11, vr6, s_18);
    }

    public static void M37(sbyte arg0, bool arg1, short arg2)
    {
        if (!arg1)
        {
            uint vr8 = s_17;
            arg1 = 0 <= s_3[0];
        }

        if (!arg1)
        {
            arg0 = s_11;
        }

        if (arg1)
        {
            arg2 = 0;
        }
    }
}

Fallout from #73472, cc @a74nh

Author:	jakobbotsch
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[a74nh]

This just requires checking the block being if converted doesn't contain phi nodes.

This is already fixed in #77728, on this line:
https://github.com/dotnet/runtime/pull/77728/files#diff-e76b0400a810f7e0ccc103f6ad923eb60b5dcb2ea6d566ca27cf253fd6e8cb4eR4794

Happy to raise a quick PR with this change, or otherwise it will automatically get fixed by 77728 ?

Once this is fixed, no if conversion happens and the program exits with an exception. I'm assuming that's the expected behaviour.
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object. at Program.M37(SByte arg0, Boolean arg1, Int16 arg2) in /home/alahay01/dotnet/examples/phi/Program.cs:line 27 at Program.Main() in /home/alahay01/dotnet/examples/phi/Program.cs:line 19

[jakobbotsch]

Happy to raise a quick PR with this change, or otherwise it will automatically get fixed by 77728 ?

I think it would be good with a separate quick fix, not sure how far away #77728 is from being merged (looks like it has some build failures currently?).

[jakobbotsch]

Once this is fixed, no if conversion happens and the program exits with an exception. I'm assuming that's the expected behaviour.

Yeah, on asserts Fuzzlyn will typically end up reducing the program such that once the assert is fixed, the program output is some exception thrown.

[jakobbotsch]

Fixed by #77888