Bug fixes from dead code elimination (#69897)

* Track if unreachable blocks were detected

squash: Update test case name

* Unreachable one-by-one

unreachable

to squash

* Add test cases

* add test description

* Check if there were any unreachable blocks

* review feedback

* jit format

* Fix a case for isBBCallAlwaysPairTail for Arm

src/coreclr/jit/fgopt.cpp

@@ -654,6 +654,7 @@ bool Compiler::fgRemoveDeadBlocks()
     JITDUMP("\n*************** In fgRemoveDeadBlocks()");
 
     jitstd::list<BasicBlock*> worklist(jitstd::allocator<void>(getAllocator(CMK_Reachability)));
+
     worklist.push_back(fgFirstBB);
 
     // Do not remove handler blocks
@@ -713,16 +714,43 @@ bool Compiler::fgRemoveDeadBlocks()
         }
     }
 
+    // Track if there is any unreachable block. Even if it is marked with
+    // BBF_DONT_REMOVE, fgRemoveUnreachableBlocks() still removes the code
+    // inside the block. So this variable tracks if we ever found such blocks
+    // or not.
+    bool hasUnreachableBlock = false;
+
     // A block is unreachable if no path was found from
     // any of the fgFirstBB, handler, filter or BBJ_ALWAYS (Arm) blocks.
     auto isBlockRemovable = [&](BasicBlock* block) -> bool {
-        return !BlockSetOps::IsMember(this, visitedBlocks, block->bbNum);
+        bool isVisited   = BlockSetOps::IsMember(this, visitedBlocks, block->bbNum);
+        bool isRemovable = (!isVisited || block->bbRefs == 0);
+
+#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
+        isRemovable &=
+            !block->isBBCallAlwaysPairTail(); // can't remove the BBJ_ALWAYS of a BBJ_CALLFINALLY / BBJ_ALWAYS pair
+#endif
+        hasUnreachableBlock |= isRemovable;
+
+        return isRemovable;
     };
 
-    bool changed = fgRemoveUnreachableBlocks(isBlockRemovable);
+    bool     changed        = false;
+    unsigned iterationCount = 1;
+    do
+    {
+        JITDUMP("\nRemoving unreachable blocks for fgRemoveDeadBlocks iteration #%u\n", iterationCount);
+
+        // Just to be paranoid, avoid infinite loops; fall back to minopts.
+        if (iterationCount++ > 10)
+        {
+            noway_assert(!"Too many unreachable block removal loops");
+        }
+        changed = fgRemoveUnreachableBlocks(isBlockRemovable);
+    } while (changed);
 
 #ifdef DEBUG
-    if (verbose && changed)
+    if (verbose && hasUnreachableBlock)
     {
         printf("\nAfter dead block removal:\n");
         fgDispBasicBlocks(verboseTrees);
@@ -732,7 +760,7 @@ bool Compiler::fgRemoveDeadBlocks()
     fgVerifyHandlerTab();
     fgDebugCheckBBlist(false);
 #endif // DEBUG
-    return changed;
+    return hasUnreachableBlock;
 }
 
 //-------------------------------------------------------------

src/tests/JIT/Regression/JitBlue/GitHub_69659/GitHub_69659_1.cs

@@ -0,0 +1,35 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+//
+// In this issue, although we were not removing an unreachable block, we were removing all the code
+// inside it and as such should update the liveness information. Since we were not updating the liveness
+// information for such scenarios, we were hitting an assert during register allocation.
+public class Program
+{
+    public static ulong[] s_14;
+    public static uint s_34;
+    public static int Main()
+    {
+        var vr2 = new ulong[][]{new ulong[]{0}};
+        M27(s_34, vr2);
+        return 100;
+    }
+
+    public static void M27(uint arg4, ulong[][] arg5)
+    {
+        arg5[0][0] = arg5[0][0];
+        for (int var7 = 0; var7 < 1; var7++)
+        {
+            return;
+        }
+
+        try
+        {
+            s_14 = arg5[0];
+        }
+        finally
+        {
+            arg4 = arg4;
+        }
+    }
+}

src/tests/JIT/Regression/JitBlue/GitHub_69659/GitHub_69659_1.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+	<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>

src/tests/JIT/Regression/JitBlue/GitHub_69659/GitHub_69659_2.cs

@@ -0,0 +1,46 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+//
+// In this issue, we were not removing all the unreachable blocks and that led us to expect that
+// there should be an IG label for one of the unreachable block, but we were not creating it leading
+// to an assert failure.
+public class _65659_2
+{
+    public static bool[][,] s_2;
+    public static short[,][] s_8;
+    public static bool[] s_10;
+    public static ushort[][] s_29;
+    public static int Main()
+    {
+        bool vr1 = M47();
+        return 100;
+    }
+
+    public static bool M47()
+    {
+        bool var0 = default(bool);
+        try
+        {
+            if (var0)
+            {
+                try
+                {
+                    if (s_10[0])
+                    {
+                        return s_2[0][0, 0];
+                    }
+                }
+                finally
+                {
+                    s_8[0, 0][0] = 0;
+                }
+            }
+        }
+        finally
+        {
+            s_29 = s_29;
+        }
+
+        return true;
+    }
+}

src/tests/JIT/Regression/JitBlue/GitHub_69659/GitHub_69659_2.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+	<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>