JIT: Evaluate GDV call args early (#75634)

The normal evaluation order for a callvirt is the following:
1. The 'this' arg is evaluated
2. The arguments are evaluated
3. 'this' is null-checked
4. The call is performed

Step 1 and 2 happen as part of the IL instructions that load the
arguments, while step 3 and 4 happen as part of the callvirt IL
instruction.

For GDV the guards needs to dereference 'this'. We were doing this too
early, causing step 3 to happen before step 2. The fix is to spill all
side-effecting arguments for GDVs to temps.

Fix #75607

src/coreclr/jit/indirectcalltransformer.cpp

@@ -540,28 +540,44 @@ class IndirectCallTransformer
             checkBlock             = currBlock;
             checkBlock->bbJumpKind = BBJ_COND;
 
-            CallArg* thisArg  = origCall->gtArgs.GetThisArg();
-            GenTree* thisTree = thisArg->GetNode();
-
-            // Create temp for this if the tree is costly.
-            if (thisTree->IsLocal())
+            // Find last arg with a side effect. All args with any effect
+            // before that will need to be spilled.
+            CallArg* lastSideEffArg = nullptr;
+            for (CallArg& arg : origCall->gtArgs.Args())
             {
-                thisTree = compiler->gtCloneExpr(thisTree);
+                if ((arg.GetNode()->gtFlags & GTF_SIDE_EFFECT) != 0)
+                {
+                    lastSideEffArg = &arg;
+                }
             }
-            else
+
+            if (lastSideEffArg != nullptr)
             {
-                const unsigned thisTempNum = compiler->lvaGrabTemp(true DEBUGARG("guarded devirt this temp"));
-                GenTree*       asgTree     = compiler->gtNewTempAssign(thisTempNum, thisTree);
-                Statement*     asgStmt     = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
-                compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);
+                for (CallArg& arg : origCall->gtArgs.Args())
+                {
+                    GenTree* argNode = arg.GetNode();
+                    if (((argNode->gtFlags & GTF_ALL_EFFECT) != 0) || compiler->gtHasLocalsWithAddrOp(argNode))
+                    {
+                        SpillArgToTempBeforeGuard(&arg);
+                    }
 
-                thisTree = compiler->gtNewLclvNode(thisTempNum, TYP_REF);
+                    if (&arg == lastSideEffArg)
+                    {
+                        break;
+                    }
+                }
+            }
 
-                // Propagate the new this to the call. Must be a new expr as the call
-                // will live on in the else block and thisTree is used below.
-                thisArg->SetEarlyNode(compiler->gtNewLclvNode(thisTempNum, TYP_REF));
+            CallArg* thisArg = origCall->gtArgs.GetThisArg();
+            // We spill 'this' if it is complex, regardless of side effects. It
+            // is going to be used multiple times due to the guard.
+            if (!thisArg->GetNode()->IsLocal())
+            {
+                SpillArgToTempBeforeGuard(thisArg);
             }
 
+            GenTree* thisTree = compiler->gtCloneExpr(thisArg->GetNode());
+
             // Remember the current last statement. If we're doing a chained GDV, we'll clone/copy
             // all the code in the check block up to and including this statement.
             //
@@ -663,6 +679,22 @@ class IndirectCallTransformer
             compiler->fgInsertStmtAtEnd(checkBlock, jmpStmt);
         }
 
+        //------------------------------------------------------------------------
+        // SpillArgToTempBeforeGuard: spill an argument into a temp in the guard/check block.
+        //
+        // Parameters
+        //   arg - The arg to create a temp and assignment for.
+        //
+        void SpillArgToTempBeforeGuard(CallArg* arg)
+        {
+            unsigned   tmpNum  = compiler->lvaGrabTemp(true DEBUGARG("guarded devirt arg temp"));
+            GenTree*   asgTree = compiler->gtNewTempAssign(tmpNum, arg->GetNode());
+            Statement* asgStmt = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
+            compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);
+
+            arg->SetEarlyNode(compiler->gtNewLclvNode(tmpNum, genActualType(arg->GetNode())));
+        }
+
         //------------------------------------------------------------------------
         // FixupRetExpr: set up to repair return value placeholder from call
         //

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

@@ -0,0 +1,63 @@
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading;
+
+public class Program
+{
+    private static int s_result;
+    public static int Main()
+    {
+        C c = new();
+        for (int i = 0; i < 100; i++)
+        {
+            Foo(c);
+            Thread.Sleep(15);
+        }
+
+        s_result = -1;
+        try
+        {
+            Foo(null);
+            Console.WriteLine("FAIL: No exception thrown");
+            return -2;
+        }
+        catch (NullReferenceException)
+        {
+            if (s_result == 100)
+            {
+                Console.WriteLine("PASS");
+            }
+            else
+            {
+                Console.WriteLine("FAIL: Result is {0}", s_result);
+            }
+
+            return s_result;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Foo(Base b)
+    {
+        b.Test(SideEffect(10));
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static long SideEffect(long i)
+    {
+        s_result = 100;
+        return i;
+    }
+}
+
+public interface Base
+{
+    void Test(long arg);
+}
+
+public class C : Base
+{
+    public void Test(long arg)
+    {
+    }
+}

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

@@ -0,0 +1,24 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_TieredPGO=1
+set COMPlus_TC_QuickJitForLoops=1
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_TieredPGO=1
+export COMPlus_TC_QuickJitForLoops=1
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>