JIT: fix invocation of unboxed entry when method returns struct

src/coreclr/jit/importer.cpp

@@ -21115,7 +21115,17 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
                                 //
                                 if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr))
                                 {
-                                    call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs);
+                                    // If there's a ret buf, the context is the second arg.
+                                    //
+                                    if (call->HasRetBufArg())
+                                    {
+                                        GenTreeCall::Use* const beforeArg = call->gtCallArgs;
+                                        beforeArg->SetNext(gtNewCallArgs(methodTableArg));
+                                    }
+                                    else
+                                    {
+                                        call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs);
+                                    }
                                 }
                                 else
                                 {
@@ -21194,26 +21204,26 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
                     // locally, or was boxed locally but we were unable to remove the box for
                     // various reasons.
                     //
-                    // We may still be able to update the call to invoke the unboxed entry.
+                    // We can still to update the call to invoke the unboxed entry, if the
+                    // boxed value is simple.
                     //
                     if (requiresInstMethodTableArg)
                     {
-                        const DWORD derivedClassAttribs = info.compCompHnd->getClassAttribs(derivedClass);
+                        // Get the method table from the boxed object.
+                        //
+                        GenTree* const thisArg       = call->gtCallThisArg->GetNode();
+                        GenTree* const clonedThisArg = gtClone(thisArg);
 
-                        if ((derivedClassAttribs & CORINFO_FLG_SHAREDINST) != 0)
+                        if (clonedThisArg == nullptr)
                         {
                             JITDUMP(
-                                "unboxed entry needs MT arg, but the handle we have is shared. Deferring update.\n");
+                                "unboxed entry needs MT arg, but `this` was too complex to clone. Deferring update.\n");
                         }
                         else
                         {
-                            // See if the method table we have is a viable argument to the unboxed
-                            // entry point. It is, if it's not a shared MT.
-                            //
-                            GenTree* const thisArg        = call->gtCallThisArg->GetNode();
-                            GenTree* const methodTableArg = gtNewIconEmbClsHndNode(derivedClass);
+                            JITDUMP("revising call to invoke unboxed entry with additional method table arg\n");
 
-                            JITDUMP("revising call to invoke unboxed entry with additional known class handle arg\n");
+                            GenTree* const methodTableArg = gtNewMethodTableLookup(clonedThisArg);
 
                             // Update the 'this' pointer to refer to the box payload
                             //
@@ -21232,14 +21242,24 @@ void Compiler::impDevirtualizeCall(GenTreeCall*            call,
                             derivedMethod        = unboxedEntryMethod;
                             derivedMethodAttribs = unboxedMethodAttribs;
 
-                            // Add the method table argument...
+                            // Add the method table argument.
                             //
                             // Prepend for R2L arg passing or empty L2R passing
                             // Append for non-empty L2R
                             //
                             if ((Target::g_tgtArgOrder == Target::ARG_ORDER_R2L) || (call->gtCallArgs == nullptr))
                             {
-                                call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs);
+                                // If there's a ret buf, the context is the second arg.
+                                //
+                                if (call->HasRetBufArg())
+                                {
+                                    GenTreeCall::Use* const beforeArg = call->gtCallArgs;
+                                    beforeArg->SetNext(gtNewCallArgs(methodTableArg));
+                                }
+                                else
+                                {
+                                    call->gtCallArgs = gtPrependNewCallArg(methodTableArg, call->gtCallArgs);
+                                }
                             }
                             else
                             {

src/tests/JIT/opt/Devirtualization/structreturningstruct.cs

@@ -0,0 +1,63 @@
+// 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.Collections;
+using System.Collections.Generic;
+using System.Threading;
+
+// Runtime issue 52975.
+//
+// If we devirtualize an interface call on a struct,      ** and **
+// update the call site to invoke the unboxed entry,      ** and **
+// the method returns a struct via hidden buffer pointer, ** and **
+// the unboxed method requires a type context arg,
+//
+// we need to be careful to pass the type context argument
+// in the right spot in the arglist.
+//
+// The test below is set up to devirtualize under PGO.
+//
+// COMPlus_TieredPGO=1
+// COMPlus_TC_QuickJitForLoopsO=1
+//
+class X
+{
+    static int F(IDictionary i)
+    {
+        int r = 0;
+        IDictionaryEnumerator e = i.GetEnumerator();
+        while (e.MoveNext())
+        {
+            // This is the critical call.
+            //
+            DictionaryEntry entry = e.Entry;
+            r++;
+        }
+        return r;
+    }
+
+    public static int Main()
+    {
+        Dictionary<string, string> s = new Dictionary<string, string>();
+        s["hello"] = "world";
+        int r = 0;
+
+        for (int i = 0; i < 50; i++)
+        {
+            r += F(s);
+            Thread.Sleep(15);
+        }
+
+        int iter = 100;
+
+        for (int i = 0; i < iter; i++)
+        {
+            r += F(s);
+        }
+
+        int result = 2 * (r - iter);
+        Console.WriteLine($"Result={result}");
+        return result;
+    }
+}

src/tests/JIT/opt/Devirtualization/structreturningstruct.csproj

@@ -0,0 +1,27 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>PdbOnly</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <PropertyGroup>
+    <!-- This test requires tiered compilation and PGO -->
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set DOTNET_TieredCompilation=1
+set DOTNET_TieredPGO=1
+set DOTNET_TC_QuickJitForLoops=1
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export DOTNET_TieredCompilation=1
+export DOTNET_TieredPGO=1
+export DOTNET_TC_QuickJitForLoops=1
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="structreturningstruct.cs" />
+  </ItemGroup>
+</Project>