Linux/Arm: BBJ_ALWAYS block remains unvisited during dominance computation

[kunalspathak]

On arm, we prevent having retless calls by adding BBJ_ALWAYS block to the enter blocks list.

runtime/src/coreclr/jit/fgopt.cpp

Lines 318 to 321 in 4d72c6b

	// TODO-ARM-Cleanup: The ARM code here to prevent creating retless calls by adding the BBJ_ALWAYS
	// to the enter blocks is a bit of a compromise, because sometimes the blocks are already reachable,
	// and it messes up DFS ordering to have them marked as enter block. We should prevent the
	// creation of retless calls some other way.

Due to this, we also skip iterating over these blocks while computing the dominance and we hit an assert because that block in below scenario is never processed while doing inverted post order traversal of blocks.

The code is dated so it might be that we can have simply have a condition now to not create retless calls if we are processing the BBJ_ALWAYS block. I will check if this approach is possible and then remove BBJ_ALWAYS from entry block list. If the change is bigger, we will have to temporarily ignore the assert in these situations.

using System.Runtime.CompilerServices;
public class TestClass
{
    public struct S2
    {
        public struct S2_D1_F1
        {
            public double double_1;
        }
    }
    static int s_int_6 = -2;
    static S2 s_s2_16 = new S2();
    int int_6 = -2;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public int LeafMethod6()
    {
        return 1;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public S2 Method0(out short p_short_1)
    {
        long long_7 = -1;
        p_short_1 = 0;
        switch (long_7)
        {
            case -5:
                {
                    do
                    {
                        try
                        {
                            int_6 ^= int_6;
                        }
                        finally
                        {
                            // The expression doesn't matter, it just has to be long enough
                            // to have few extra blocks which we don't walk when doing inverse
                            // post order while computing dominance information.
                            long_7 &= long_7;
                            int_6 &= (int_6 /= (int_6 -= LeafMethod6() - int_6) + 69) / ((int_6 << (int_6 - int_6)) + (int_6 |= LeafMethod6()) + (LeafMethod6() >> s_int_6) + 62);
                        }
                    }
                    while (long_7 == 8);
                    break;
                }
            default:
                {
                    break;
                }
        }
        return s_s2_16;
    }

    public static void Main(string[] args)
    {
        new TestClass().Method0(out short s);
    }
}

On linux/arm we get the following assertion:

Assert failure(PID 29776 [0x00007450], Thread: 45856 [0xb320]): Assertion failed 'postIndex == fgBBcount + 1' in 'TestClass:Method1(ushort,byref,byref,byref):S2:this' during 'Compute blocks reachability' (IL size 403)
    File: D:\git\runtime\src\coreclr\jit\fgopt.cpp Line: 672
    Image: D:\git\runtime\artifacts\tests\coreclr\windows.x64.Checked\tests\Core_Root\CoreRun.exe

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

Issue Details

---

On arm, we prevent having retless calls by adding BBJ_ALWAYS block to the enter blocks list.

runtime/src/coreclr/jit/fgopt.cpp

Lines 318 to 321 in 4d72c6b

	// TODO-ARM-Cleanup: The ARM code here to prevent creating retless calls by adding the BBJ_ALWAYS
	// to the enter blocks is a bit of a compromise, because sometimes the blocks are already reachable,
	// and it messes up DFS ordering to have them marked as enter block. We should prevent the
	// creation of retless calls some other way.

Due to this, we also skip iterating over these blocks while computing the dominance and we hit an assert because that block in below scenario is never processed while doing inverted post order traversal of blocks.

The code is dated so it might be that we can have simply have a condition now to not create retless calls if we are processing the BBJ_ALWAYS block. I will check if this approach is possible and then remove BBJ_ALWAYS from entry block list. If the change is bigger, we will have to temporarily ignore the assert in these situations.

using System.Runtime.CompilerServices;
public class TestClass
{
    public struct S2
    {
        public struct S2_D1_F1
        {
            public double double_1;
        }
    }
    static int s_int_6 = -2;
    static S2 s_s2_16 = new S2();
    int int_6 = -2;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public int LeafMethod6()
    {
        return 1;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    public S2 Method0(out short p_short_1)
    {
        long long_7 = -1;
        p_short_1 = 0;
        switch (long_7)
        {
            case -5:
                {
                    do
                    {
                        try
                        {
                            int_6 ^= int_6;
                        }
                        finally
                        {
                            // The expression doesn't matter, it just has to be long enough
                            // to have few extra blocks which we don't walk when doing inverse
                            // post order while computing dominance information.
                            long_7 &= long_7;
                            int_6 &= (int_6 /= (int_6 -= LeafMethod6() - int_6) + 69) / ((int_6 << (int_6 - int_6)) + (int_6 |= LeafMethod6()) + (LeafMethod6() >> s_int_6) + 62);
                        }
                    }
                    while (long_7 == 8);
                    break;
                }
            default:
                {
                    break;
                }
        }
        return s_s2_16;
    }

    public static void Main(string[] args)
    {
        new TestClass().Method0(out short s);
    }
}

On linux/arm we get the following assertion:

Assert failure(PID 29776 [0x00007450], Thread: 45856 [0xb320]): Assertion failed 'postIndex == fgBBcount + 1' in 'TestClass:Method1(ushort,byref,byref,byref):S2:this' during 'Compute blocks reachability' (IL size 403)
    File: D:\git\runtime\src\coreclr\jit\fgopt.cpp Line: 672
    Image: D:\git\runtime\artifacts\tests\coreclr\windows.x64.Checked\tests\Core_Root\CoreRun.exe

Author:	kunalspathak
Assignees:	-
Labels:	arch-arm32, os-linux, area-CodeGen-coreclr
Milestone:	-

[kunalspathak]

@dotnet/jit-contrib

[kunalspathak]

We have this code for a while, so definitely this is not .NET 6 only issue. Marking it for .NET 7