[mono][interp] Add unbox when calling valuetype method through delegate (#79445)

* [mono][interp] Add unbox when calling valuetype method through delegate

If we are calling an open instance delegate, where the target method is on a valuetype, we will need to unbox this pointer.

* [mono][interp] Remove redundant check

If we are calling a method of a valuetype, then we already know `this` pointer is an instantiation of a valuetype.

* [mono][interp] Remove dead opcode

* [tests] Add regression test

* [tests] Disable test on fullaot

diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
index 22e9522de36cb5db5bcb6de900400ab40ce01583..0976c9bf5e466b21efe8be5f8aece35a1219f606 100644 (file)
--- a/src/mono/mono/mini/interp/interp.c
+++ b/src/mono/mono/mini/interp/interp.c
@@ -3975,7 +3975,12 @@ main_loop:
                                        } else if ((m_method_is_virtual (del_imethod->method) && !m_method_is_static (del_imethod->method)) && !del->target && !m_class_is_valuetype (del_imethod->method->klass)) {
                                                // 'this' is passed dynamically, we need to recompute the target method
                                                // with each call
-                                               del_imethod = get_virtual_method (del_imethod, LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*)->vtable);
+                                               MonoObject *obj = LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*);
+                                               del_imethod = get_virtual_method (del_imethod, obj->vtable);
+                                               if (m_class_is_valuetype (del_imethod->method->klass)) {
+                                                       // We are calling into a value type method, `this` needs to be unboxed
+                                                       LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, gpointer) = mono_object_unbox_internal (obj);
+                                               }
                                        } else {
                                                del->interp_invoke_impl = del_imethod;
                                        }
@@ -3998,7 +4003,7 @@ main_loop:
                                        MonoObject *this_arg = del->target;
 
                                        // replace the MonoDelegate* on the stack with 'this' pointer
-                                       if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
+                                       if (m_class_is_valuetype (cmethod->method->klass)) {
                                                gpointer unboxed = mono_object_unbox_internal (this_arg);
                                                LOCAL_VAR (call_args_offset, gpointer) = unboxed;
                                        } else {
@@ -4098,7 +4103,7 @@ main_loop:
                        ip += 5;
                        // FIXME push/pop LMF
                        cmethod = get_virtual_method_fast (cmethod, this_arg->vtable, slot);
-                       if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
+                       if (m_class_is_valuetype (cmethod->method->klass)) {
                                /* unbox */
                                gpointer unboxed = mono_object_unbox_internal (this_arg);
                                LOCAL_VAR (call_args_offset, gpointer) = unboxed;
@@ -4148,29 +4153,6 @@ main_loop:
                        goto call;
                }
 
-               MINT_IN_CASE(MINT_CALLVIRT) {
-                       // FIXME CALLVIRT opcodes are not used on netcore. We should kill them.
-                       cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
-                       return_offset = ip [1];
-                       call_args_offset = ip [2];
-
-                       MonoObject *this_arg = LOCAL_VAR (call_args_offset, MonoObject*);
-
-                       // FIXME push/pop LMF
-                       cmethod = get_virtual_method (cmethod, this_arg->vtable);
-                       if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
-                               /* unbox */
-                               gpointer unboxed = mono_object_unbox_internal (this_arg);
-                               LOCAL_VAR (call_args_offset, gpointer) = unboxed;
-                       }
-
-#ifdef ENABLE_EXPERIMENT_TIERED
-                       ip += 5;
-#else
-                       ip += 4;
-#endif
-                       goto call;
-               }
                MINT_IN_CASE(MINT_CALL) {
                        cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
                        return_offset = ip [1];

diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def
index da0a71251a79d3178758cd2e7e03d46c2c7b1095..56080ef6ca50d9da725f984027fdc2cb5f6ba95b 100644 (file)
--- a/src/mono/mono/mini/interp/mintops.def
+++ b/src/mono/mono/mini/interp/mintops.def
@@ -670,7 +670,6 @@ OPDEF(MINT_ARRAY_ELEMENT_SIZE, "array_element_size", 3, 1, 1, MintOpNoArgs)
 
 /* Calls */
 OPDEF(MINT_CALL, "call", 4, 1, 1, MintOpMethodToken)
-OPDEF(MINT_CALLVIRT, "callvirt", 4, 1, 1, MintOpMethodToken)
 OPDEF(MINT_CALLVIRT_FAST, "callvirt.fast", 5, 1, 1, MintOpMethodToken)
 OPDEF(MINT_CALL_DELEGATE, "call.delegate", 5, 1, 1, MintOpTwoShorts)
 OPDEF(MINT_CALLI, "calli", 4, 1, 2, MintOpNoArgs)

diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c
index d523d73fabe4012da48b667bc19cb182cf50f3a6..831268c14b3d2eef087beb54078009c9bd4da1d6 100644 (file)
--- a/src/mono/mono/mini/interp/transform.c
+++ b/src/mono/mono/mini/interp/transform.c
@@ -3541,8 +3541,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
                        } else if (is_virtual) {
                                interp_add_ins (td, MINT_CALLVIRT_FAST);
                                td->last_ins->data [1] = get_virt_method_slot (target_method);
-                       } else if (is_virtual) {
-                               interp_add_ins (td, MINT_CALLVIRT);
                        } else {
                                interp_add_ins (td, MINT_CALL);
                        }

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.cs b/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.cs
new file mode 100644 (file)
index 0000000..8114d96
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.cs
@@ -0,0 +1,45 @@
+// 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.Reflection;
+
+public interface IGetContents {
+    (string, int, string) GetContents();
+}
+
+public struct MyStruct : IGetContents {
+    public string s1;
+    public int a;
+    public string s2;
+
+    public (string, int, string) GetContents()
+    {
+        return (s1, a, s2);
+    }
+}
+
+public class Program {
+
+    public delegate (string, int, string) MyDelegate(IGetContents arg);
+
+    public static int Main(string[] args)
+    {
+        MyStruct str = new MyStruct();
+        str.s1 = "test1";
+        str.a = 42;
+        str.s2 = "test2";
+
+        MethodInfo mi = typeof(IGetContents).GetMethod("GetContents");
+        MyDelegate func = (MyDelegate)mi.CreateDelegate(typeof(MyDelegate));
+
+        (string c1, int c2, string c3) = func(str);
+        if (c1 != "test1")
+            return 1;
+        if (c2 != 42)
+            return 2;
+        if (c3 != "test2")
+            return 3;
+        return 100;
+    }
+}

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.csproj
new file mode 100644 (file)
index 0000000..75e7d24
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.csproj
@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>

diff --git a/src/tests/issues.targets b/src/tests/issues.targets
index b55ae0895666decb4002dbfd94b60c7bb7301233..e925f3ec64b82335c38592ffc3be8887a62aac02 100644 (file)
--- a/src/tests/issues.targets
+++ b/src/tests/issues.targets
@@ -2910,6 +2910,9 @@
         <ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/DevDiv_461649/DevDiv_461649/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
+        <ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354/**">
+            <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
+        </ExcludeList>
         <ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>