JIT: Handle GT_RETFILT in fgInsertStmtNearEnd (#85420)

Also add some logging for read backs in physical promotion.

Fix #85088

Author: jakobbotsch

src/coreclr/jit/fgstmt.cpp

@@ -171,7 +171,7 @@ Statement* Compiler::fgNewStmtAtEnd(BasicBlock* block, GenTree* tree, const Debu
 
 //------------------------------------------------------------------------
 // fgInsertStmtNearEnd: Insert the given statement at the end of the given basic block,
-//   but before the GT_JTRUE, if present.
+//   but before the terminating node, if present.
 //
 // Arguments:
 //   block - the block into which 'stmt' will be inserted;
@@ -182,7 +182,7 @@ void Compiler::fgInsertStmtNearEnd(BasicBlock* block, Statement* stmt)
     // This routine can only be used when in tree order.
     assert(fgOrder == FGOrderTree);
 
-    if (block->KindIs(BBJ_COND, BBJ_SWITCH, BBJ_RETURN))
+    if (block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET, BBJ_COND, BBJ_SWITCH, BBJ_RETURN))
     {
         Statement* firstStmt = block->firstStmt();
         noway_assert(firstStmt != nullptr);
@@ -191,24 +191,28 @@ void Compiler::fgInsertStmtNearEnd(BasicBlock* block, Statement* stmt)
         Statement* insertionPoint = lastStmt->GetPrevStmt();
 
 #if DEBUG
-        if (block->bbJumpKind == BBJ_COND)
+        if (block->KindIs(BBJ_COND))
         {
-            assert(lastStmt->GetRootNode()->gtOper == GT_JTRUE);
+            assert(lastStmt->GetRootNode()->OperIs(GT_JTRUE));
         }
-        else if (block->bbJumpKind == BBJ_RETURN)
+        else if (block->KindIs(BBJ_RETURN))
         {
-            assert((lastStmt->GetRootNode()->gtOper == GT_RETURN) || (lastStmt->GetRootNode()->gtOper == GT_JMP) ||
+            assert((lastStmt->GetRootNode()->OperIs(GT_RETURN, GT_JMP)) ||
                    // BBJ_RETURN blocks in functions returning void do not get a GT_RETURN node if they
                    // have a .tail prefix (even if canTailCall returns false for these calls)
                    // code:Compiler::impImportBlockCode (search for the RET: label)
                    // Ditto for real tail calls (all code after them has been removed)
-                   ((lastStmt->GetRootNode()->gtOper == GT_CALL) &&
+                   (lastStmt->GetRootNode()->OperIs(GT_CALL) &&
                     ((info.compRetType == TYP_VOID) || lastStmt->GetRootNode()->AsCall()->IsTailCall())));
         }
+        else if (block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET))
+        {
+            assert(lastStmt->GetRootNode()->OperIs(GT_RETFILT));
+        }
         else
         {
-            assert(block->bbJumpKind == BBJ_SWITCH);
-            assert(lastStmt->GetRootNode()->gtOper == GT_SWITCH);
+            assert(block->KindIs(BBJ_SWITCH));
+            assert(lastStmt->GetRootNode()->OperIs(GT_SWITCH));
         }
 #endif // DEBUG
 

src/coreclr/jit/promotion.cpp

@@ -1770,11 +1770,13 @@ PhaseStatus Promotion::Run()
                 assert(!rep.NeedsReadBack || !rep.NeedsWriteBack);
                 if (rep.NeedsReadBack)
                 {
-                    JITDUMP("Reading back replacement V%02u.[%03u..%03u) -> V%02u at the end of " FMT_BB "\n", i,
+                    JITDUMP("Reading back replacement V%02u.[%03u..%03u) -> V%02u near the end of " FMT_BB ":\n", i,
                             rep.Offset, rep.Offset + genTypeSize(rep.AccessType), rep.LclNum, bb->bbNum);
 
-                    GenTree* readBack = replacer.CreateReadBack(i, rep);
-                    m_compiler->fgInsertStmtNearEnd(bb, m_compiler->fgNewStmtFromTree(readBack));
+                    GenTree*   readBack = replacer.CreateReadBack(i, rep);
+                    Statement* stmt     = m_compiler->fgNewStmtFromTree(readBack);
+                    DISPSTMT(stmt);
+                    m_compiler->fgInsertStmtNearEnd(bb, stmt);
                     rep.NeedsReadBack = false;
                 }
 

src/tests/JIT/Regression/JitBlue/Runtime_85088/Runtime_85088.cs

@@ -0,0 +1,50 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Runtime_85088
+{
+    [Fact]
+    public static int Test()
+    {
+        Foo f = new();
+        try
+        {
+            try
+            {
+                throw new Exception();
+            }
+            finally
+            {
+                f.X = 15;
+                f.Y = 20;
+                f.X += f.Y;
+                f.Y *= f.X;
+
+                // f will be physically promoted and will require a read back after this call.
+                // Since this is a finally, some platforms will have a GT_RETFILT that we were
+                // inserting IR after instead of before.
+                f = Call(f);
+            }
+        }
+        catch
+        {
+        }
+
+        return f.X + f.Y;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static Foo Call(Foo f)
+    {
+        return new Foo { X = 75, Y = 25 };
+    }
+
+    private struct Foo
+    {
+        public short X, Y;
+    }
+}

src/tests/JIT/Regression/JitBlue/Runtime_85088/Runtime_85088.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+    <CLRTestEnvironmentVariable Include="DOTNET_JitStressModeNames" Value="STRESS_PHYSICAL_PROMOTION STRESS_PHYSICAL_PROMOTION_COST STRESS_NO_OLD_PROMOTION" />
+  </ItemGroup>
+</Project>