JIT: Assertion failed '!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()'

[jakobbotsch]

Run with optimizations on:

// Generated by Fuzzlyn v1.3 on 2021-08-19 15:47:06
// Run on .NET 6.0.0-dev on X64 Windows
// Seed: 16489483397161801783
// Reduced from 251.9 KiB to 1.7 KiB in 02:46:36
// 
// Assert failure(PID 27840 [0x00006cc0], Thread: 1464 [0x05b8]): Assertion failed '!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()' in 'Program:M51(System.Boolean[],System.UInt16[],System.Boolean[],long,System.Int32[],byref)' during 'Generate code' (IL size 140)
// 
//     File: D:\dev\dotnet\runtime\src\coreclr\jit\codegencommon.cpp Line: 11990
//     Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe
// 
// 
public class Program
{
    internal static I s_rt;
    internal static long s_10;
    internal static bool[] s_27;
    internal static short s_28;
    internal static bool s_33;
    internal static bool s_53;
    internal static int[][] s_56;
    public static void Main()
    {
        s_rt = new C();
        var vr9 = new ushort[]{0};
        var vr10 = new bool[]{true};
        var vr11 = new int[]{0};
        M51(s_27, vr9, vr10, 0, vr11, ref s_28);
    }

    internal static void M51(bool[] arg0, ushort[] arg5, bool[] arg9, ulong arg10, int[] arg11, ref short arg12)
    {
        if (arg0[0])
        {
            if (arg0[0])
            {
                arg11[0] = arg11[0];
            }
            else
            {
                try
                {
                }
                finally
                {
                    arg9[0] = arg9[0];
                }

                try
                {
                    arg0[0] = true;
                }
                finally
                {
                    for (int var4 = 0; var4 < 2; var4++)
                    {
                        s_53 = arg0[0];
                        arg10 = 0;
                        ushort var6 = arg5[0];
                        long var8 = 0;
                        s_rt.Write(s_56[0][0]);
                        s_rt.Write(var8);
                    }

                    s_33 = false;
                    try
                    {
                        s_10 = s_10;
                    }
                    finally
                    {
                        arg12 = ref arg12;
                    }
                }
            }

            bool vr1 = arg9[0];
        }
    }
}

public interface I
{
    void Write<T>(T val);
}

public class C : I
{
    public void Write<T>(T val)
    {
    }
}

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

Issue Details

---

Run with optimizations on:

// Generated by Fuzzlyn v1.3 on 2021-08-19 15:47:06
// Run on .NET 6.0.0-dev on X64 Windows
// Seed: 16489483397161801783
// Reduced from 251.9 KiB to 1.7 KiB in 02:46:36
// 
// Assert failure(PID 27840 [0x00006cc0], Thread: 1464 [0x05b8]): Assertion failed '!m_VariableLiveRanges->back().m_EndEmitLocation.Valid()' in 'Program:M51(System.Boolean[],System.UInt16[],System.Boolean[],long,System.Int32[],byref)' during 'Generate code' (IL size 140)
// 
//     File: D:\dev\dotnet\runtime\src\coreclr\jit\codegencommon.cpp Line: 11990
//     Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe
// 
// 
public class Program
{
    internal static I s_rt;
    internal static long s_10;
    internal static bool[] s_27;
    internal static short s_28;
    internal static bool s_33;
    internal static bool s_53;
    internal static int[][] s_56;
    public static void Main()
    {
        s_rt = new C();
        var vr9 = new ushort[]{0};
        var vr10 = new bool[]{true};
        var vr11 = new int[]{0};
        M51(s_27, vr9, vr10, 0, vr11, ref s_28);
    }

    internal static void M51(bool[] arg0, ushort[] arg5, bool[] arg9, ulong arg10, int[] arg11, ref short arg12)
    {
        if (arg0[0])
        {
            if (arg0[0])
            {
                arg11[0] = arg11[0];
            }
            else
            {
                try
                {
                }
                finally
                {
                    arg9[0] = arg9[0];
                }

                try
                {
                    arg0[0] = true;
                }
                finally
                {
                    for (int var4 = 0; var4 < 2; var4++)
                    {
                        s_53 = arg0[0];
                        arg10 = 0;
                        ushort var6 = arg5[0];
                        long var8 = 0;
                        s_rt.Write(s_56[0][0]);
                        s_rt.Write(var8);
                    }

                    s_33 = false;
                    try
                    {
                        s_10 = s_10;
                    }
                    finally
                    {
                        arg12 = ref arg12;
                    }
                }
            }

            bool vr1 = arg9[0];
        }
    }
}

public interface I
{
    void Write<T>(T val);
}

public class C : I
{
    public void Write<T>(T val)
    {
    }
}

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

[JulieLeeMSFT]

CC @dotnet/jit-contrib

[jakobbotsch]

This one might be related, it fails on a different assert in that same function:

// Generated by Fuzzlyn v1.3 on 2021-08-23 02:35:43
// Run on .NET 6.0.0-dev on X64 Windows
// Seed: 13788434105727734599
// Reduced from 365.1 KiB to 0.9 KiB in 02:25:35
// Exits with error:
// 
// Assert failure(PID 11828 [0x00002e34], Thread: 24792 [0x60d8]): Assertion failed 'm_VariableLiveRanges != nullptr && !m_VariableLiveRanges->empty()' in 'Program:M53(byref,byref)' during 'Generate code' (IL size 177)
// 
//     File: D:\dev\dotnet\runtime\src\coreclr\jit\codegencommon.cpp Line: 11987
//     Image: D:\dev\Fuzzlyn\Fuzzlyn\publish\windows-x64\Fuzzlyn.exe
// 
// 
public class Program
{
    internal static ulong s_46;
    public static void Main()
    {
        M53(ref s_46, ref s_46);
    }

    internal static void M53(ref ulong arg0, ref ulong arg1)
    {
        int var11 = default(int);
        for (int var0 = 0; var0 < 1; var0++)
        {
            return;
        }

        try
        {
            bool vr0 = arg1 >= 0;
            bool vr6 = vr0;
        }
        finally
        {
            for (int var2 = 0; var2 < -1; var2++)
            {
            }

            try
            {
            }
            finally
            {
                System.Console.WriteLine(var11);
            }

            var vr7 = new uint[][]{new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}, new uint[]{0}};
        }

        System.Console.WriteLine(arg0);
    }
}

[AndyAyersMS]

I saw a similar assert when I tried to enable all this in .NET 5. Some of the analysis I did then may or may not be useful.

#38894 (comment)

[davidwrighton]

I can also reproduce this with building the X64 System.Private.CoreLib.dll using a crossgen2 compiler targetting arm64. (To reproduce, take the crossgen2 command for building System.Private.Corelib.dll and replace --targetarch:x64 with --targetarch:arm64.

[jakobbotsch]

In this case we have left a lot of unreachable blocks around which seems to contribute to confusing the JIT:

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1       [000..005)-> BB10 ( cond )                     i label idxlen LIR 
BB02 [0001]  1       BB01                  1       [005..014)-> BB09 (always)                     i idxlen LIR 
BB04 [0004]  0                             1       [014..017)                                     keep i LIR 
BB05 [0005]  1       BB04                  1       [017..01F)                                     keep i idxlen LIR 
BB06 [0007]  1  0    BB05                  1       [01F..025)                 T0      try { }     keep i try label idxlen LIR 
BB07 [0019]  1       BB06                  1       [???..???)-> BB11 (callf )                     i internal label LIR 
BB08 [0020]  1       BB14                  1       [???..???)-> BB09 (ALWAYS)                     i internal LIR KEEP 
BB09 [0015]  2       BB02,BB08             1       [07A..07E)                                     i label idxlen LIR 
BB10 [0016]  2       BB01,BB09             1       [07E..07F)        (return)                     i label LIR 
BB16 [0025]  0                             0       [???..???)        (throw )                     keep i internal rare label LIR 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ funclets follow
BB11 [0008]  2     0 BB07                  1       [025..029)                    H0 F finally {   keep i label flet idxlen LIR 
BB12 [0009]  2     0 BB11,BB12             8     0 [029..062)-> BB12 (always)    H0               i Loop label hascall gcsafe idxlen bwd bwd-target LIR 
BB14 [0012]  0     0                       1       [068..07A)        (finret)    H0   }           keep i LIR 
-----------------------------------------------------------------------------------------------------------------------------------------

It's unfortunate that we have both fgUpdateFlowGraph and fgComputeReachability that remove unreachable blocks, with the former not doing as good a job as the latter (but also doing other things). We should look into unifying them in .NET 7.

As for this issue: it seems we create assertions that say that the loop condition is both always true and always false (presumably, some side effect of all the code being unreachable):

GenTreeNode creates assertion:
N004 (  5,  5) [000029] ------------              ▌  JTRUE     void  
In BB12 New Global Constant Assertion: ($1ea,$40) Const_Loop_Bnd {LT($1e9, $42)} is not  {IntCns 0}, index = #13
GenTreeNode creates assertion:
N004 (  5,  5) [000029] ------------              ▌  JTRUE     void  
In BB12 New Global Constant Assertion: ($1ea,$40) Const_Loop_Bnd {LT($1e9, $42)} is  {IntCns 0}, index = #14

This makes us optimize the loop into an infinite loop:

VN relop based constant assertion prop in BB12:
Assertion index=#13: [000028] != 0
N003 (  3,  3) [000028] ------------              ▌  CNS_INT   int    1
Propagating #01 #02 #03 #04 #05 #06 #07 #08 #09 #10 #11 #12 #13 #14 for BB12, stmt STMT00006, tree [000029], tree -> #13
Re-morphing this stmt:
STMT00006 (IL 0x05E...0x060)
N004 (  5,  5) [000029] ------------              ▌  JTRUE     void  
N003 (  3,  3) [000028] ------------              └──▌  CNS_INT   int    1


removing useless STMT00006 (IL 0x05E...0x060)
N004 (  5,  5) [000029] ------------              ▌  JTRUE     void  
N003 (  3,  3) [000028] ------------              └──▌  CNS_INT   int    1
 from BB12

Conditional folded at BB12
BB12 becomes a BBJ_ALWAYS to BB12
optAssertionPropMain removed tree:
N004 (  5,  5) [000029] ------------              ▌  JTRUE     void  
N003 (  3,  3) [000028] ------------              └──▌  CNS_INT   int    1

Looks like this effects liveness: in particular, we have a callfinally+always pair in BB07+BB08, after arg0[0] = true; in the second try. We end up with the following liveness:

BB liveness after fgLiveVarAnalysis():

BB01 IN (3)={V00 V02 V04} + ByrefExposed + GcHeap
     OUT(2)={    V02 V04} + ByrefExposed + GcHeap

BB02 IN (2)={V02 V04} + ByrefExposed + GcHeap
     OUT(1)={V02    } + ByrefExposed + GcHeap

BB04 IN (2)={V00 V02} + ByrefExposed + GcHeap
     OUT(2)={V00 V02} + ByrefExposed + GcHeap

BB05 IN (2)={V00 V02} + ByrefExposed + GcHeap
     OUT(1)={V00    } + ByrefExposed + GcHeap

BB06 IN (1)={V00} + ByrefExposed + GcHeap
     OUT(1)={V00} + ByrefExposed + GcHeap

BB07 IN (1)={V00} + ByrefExposed + GcHeap
     OUT(1)={V00} + ByrefExposed + GcHeap       // V02 not live-out in callfinally

BB08 IN (1)={V02} + ByrefExposed + GcHeap
     OUT(1)={V02} + ByrefExposed + GcHeap       // V02 live-out in always

BB09 IN (1)={V02} + ByrefExposed + GcHeap
     OUT(0)={   }

BB10 IN (0)={}
     OUT(0)={}

BB16 IN (0)={}
     OUT(0)={}

BB11 IN (1)={    V00} + ByrefExposed + GcHeap
     OUT(2)={V06 V00} + ByrefExposed + GcHeap

BB12 IN (2)={V06 V00} + ByrefExposed + GcHeap
     OUT(2)={V06 V00} + ByrefExposed + GcHeap

BB14 IN (1)={V02} + ByrefExposed + GcHeap
     OUT(1)={V02} + ByrefExposed + GcHeap

V02 is live-out of the always but not live-out of the callfinally, presumably because the finally being called now has an infinite loop.
The final nail in the coffin comes from two things:

1. When we generate code for callfinally+always pairs, we do it in a special way. In particular we only handle liveness changes for the callfinally and skip over the always block:
   

   runtime/src/coreclr/jit/codegenlinear.cpp

   Lines 712 to 714 in 32df679

   	case BBJ_CALLFINALLY:
   	block = genCallFinally(block);
   	break;

   
   

   runtime/src/coreclr/jit/codegenxarch.cpp

   Lines 385 to 394 in 32df679

   	// The BBJ_ALWAYS is used because the BBJ_CALLFINALLY can't point to the
   	// jump target using bbJumpDest - that is already used to point
   	// to the finally block. So just skip past the BBJ_ALWAYS unless the
   	// block is RETLESS.
   	if (!(block->bbFlags & BBF_RETLESS_CALL))
   	{
   	assert(block->isBBCallAlwaysPair());
   	block = block->bbNext;
   	}
   	return block;

2. Later, when we visit BB09 we use the live-out vars of the previous block, i.e. always block of the callfinally+always pair, to check if we opened a live range:
   

   runtime/src/coreclr/jit/lsra.cpp

   Lines 1350 to 1357 in 32df679

   	#ifdef USING_VARIABLE_LIVE_RANGE
   	if (bb->bbPrev != nullptr && VarSetOps::IsMember(compiler, bb->bbPrev->bbLiveOut, varIndex))
   	{
   	// varDsc was alive on previous block end ("bb->bbPrev->bbLiveOut"), so it has an open
   	// "VariableLiveRange" which should change to be according "getInVarToRegMap"
   	compiler->codeGen->getVariableLiveKeeper()->siUpdateVariableLiveRange(varDsc, varNum);
   	}
   	#endif // USING_VARIABLE_LIVE_RANGE

   
   But this block was skipped over during codegen.

The initial assertion created seems weird, but even with that fixed either the check for the previous block or the fact that we skip liveness for the always block of a callfinally+always seems wrong.

[jakobbotsch]

@davidwrighton's issue seems to not be the same as the previous comment, but it does look like it's exactly the same as @AndyAyersMS's comments in #38894, in the same function.
It looks like we did add an IsSupported check in SseImpl in the meantime:

runtime/src/libraries/System.Private.CoreLib/src/System/Numerics/Matrix4x4.cs

Lines 1389 to 1395 in 32df679

	static unsafe bool SseImpl(Matrix4x4 matrix, out Matrix4x4 result)
	{
	if (!Sse.IsSupported)
	{
	// Redundant test so we won't prejit remainder of this method on platforms without SSE.
	throw new PlatformNotSupportedException();
	}

However, it looks like it does not unfold to 0 when using --targetarch:arm64, not sure if that's intended @davidwrighton.

@AndyAyersMS did we merge #38894 in a different PR, and did we do anything to resolve the problem you identified there?

[AndyAyersMS]

@AndyAyersMS did we merge #38894 in a different PR, and did we do anything to resolve the problem you identified there?

Yes (#50162), and not that I know of...