[interp] Remove some unused paths in newobj and cleanup a little. (#32685)

This is an alternative to https://github.com/mono/mono/pull/18974.
Mainly that it does not introduce new opcode to remove branch from newobj/newobj_string.
Both eliminate a bit of dead code in newobj (if turns out to be alive, can also be new opcodes).

Co-authored-by: Jay Krell <jay.krell@cornell.edu>

diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
index 05f6419..bb0df2d 100644 (file)
--- a/src/mono/mono/mini/interp/interp.c
+++ b/src/mono/mono/mini/interp/interp.c
@@ -5003,8 +5003,6 @@ call_newobj:
                                memmove (sp + 2, sp, param_count * sizeof (stackval));
                        }
 
-                       InterpMethod* const imethod = frame->imethod;
-
                        MonoClass * const newobj_class = cmethod->method->klass;
 
                        /*if (profiling_classes) {
@@ -5017,47 +5015,41 @@ call_newobj:
                         * First arg is the object.
                         * a constructor returns void, but we need to return the object we created
                         */
-                       if (m_class_is_valuetype (newobj_class)) {
-                               MonoType *t = m_class_get_byval_arg (newobj_class);
-                               if (!m_class_is_enumtype (newobj_class) && (t->type == MONO_TYPE_VALUETYPE || (t->type == MONO_TYPE_GENERICINST && mono_type_generic_inst_is_valuetype (t)))) {
-                                       sp [0].data.p = vt_sp; // return value
-                                       sp [1].data.p = vt_sp; // first parameter
-                               } else {
-                                       memset (sp, 0, sizeof (*sp));
-                                       sp [1].data.p = &sp [0].data; // first parameter is return value
-                               }
-                       } else {
-                               if (newobj_class != mono_defaults.string_class) {
-                                       MonoVTable *vtable = mono_class_vtable_checked (imethod->domain, newobj_class, error);
-                                       if (!is_ok (error) || !mono_runtime_class_init_full (vtable, error)) {
-                                               MonoException *exc = mono_error_convert_to_exception (error);
-                                               g_assert (exc);
-                                               THROW_EX (exc, ip);
-                                       }
-                                       error_init_reuse (error);
-                                       MonoObject* o = NULL; // See the comment about GC safety.
-                                       OBJREF (o) = mono_object_new_checked (imethod->domain, newobj_class, error);
-                                       mono_error_cleanup (error); // FIXME: do not swallow the error
-                                       error_init_reuse (error);
-                                       EXCEPTION_CHECKPOINT;
-                                       sp [0].data.o = o; // return value
-                                       sp [1].data.o = o; // first parameter
+
+                       g_assert (!m_class_is_valuetype (newobj_class));
+
+                       // This branch could be avoided. Move it to transform, and use a new opcode NEWOBJ_STRING.
+                       if (newobj_class == mono_defaults.string_class) {
+                               retval = sp;
+                               ++sp;
+                               sp->data.p = NULL; // first parameter
+                               is_void = TRUE;
+                               goto call;
+                       }
+
+                       MonoDomain* const domain = frame->imethod->domain;
+                       MonoVTable *vtable = mono_class_vtable_checked (domain, newobj_class, error);
+                       if (!is_ok (error) || !mono_runtime_class_init_full (vtable, error)) {
+                               MonoException *exc = mono_error_convert_to_exception (error);
+                               g_assert (exc);
+                               THROW_EX (exc, ip);
+                       }
+                       error_init_reuse (error);
+                       MonoObject* o = NULL; // See the comment about GC safety.
+                       OBJREF (o) = mono_object_new_checked (domain, newobj_class, error);
+                       mono_error_cleanup (error); // FIXME: do not swallow the error
+                       error_init_reuse (error);
+                       EXCEPTION_CHECKPOINT;
+                       sp [0].data.o = o; // return value
+                       sp [1].data.o = o; // first parameter
 #ifndef DISABLE_REMOTING
-                                       if (mono_object_is_transparent_proxy (o)) {
-                                               MonoMethod *remoting_invoke_method = mono_marshal_get_remoting_invoke_with_check (cmethod->method, error);
-                                               mono_error_assert_ok (error);
-                                               cmethod = mono_interp_get_imethod (imethod->domain, remoting_invoke_method, error);
-                                               mono_error_assert_ok (error);
-                                       }
-#endif
-                               } else {
-                                       retval = sp;
-                                       ++sp;
-                                       sp->data.p = NULL; // first parameter
-                                       is_void = TRUE;
-                                       goto call;
-                               }
+                       if (mono_object_is_transparent_proxy (o)) {
+                               MonoMethod *remoting_invoke_method = mono_marshal_get_remoting_invoke_with_check (cmethod->method, error);
+                               mono_error_assert_ok (error);
+                               cmethod = mono_interp_get_imethod (domain, remoting_invoke_method, error);
+                               mono_error_assert_ok (error);
                        }
+#endif
                        goto call_newobj;
                }
                MINT_IN_CASE(MINT_NEWOBJ_MAGIC) {

diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c
index f124007..da61c73 100644 (file)
--- a/src/mono/mono/mini/interp/transform.c
+++ b/src/mono/mono/mini/interp/transform.c
@@ -4550,23 +4550,22 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                                                // extra stack to hold this and return value, before call.
                                                simulate_runtime_stack_increase (td, 2);
 
-                                               if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT)
-                                                       interp_add_ins (td, MINT_NEWOBJ_VTST_FAST);
-                                               else
-                                                       interp_add_ins (td, MINT_NEWOBJ_VT_FAST);
+                                               const gboolean vt = mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT;
+
+                                               interp_add_ins (td, vt ? MINT_NEWOBJ_VTST_FAST : MINT_NEWOBJ_VT_FAST);
 
                                                td->last_ins->data [0] = get_data_item_index (td, mono_interp_get_imethod (domain, m, error));
                                                td->last_ins->data [1] = csignature->param_count;
 
-                                               if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT) {
+                                               if (vt)
                                                        td->last_ins->data [2] = mono_class_value_size (klass, NULL);
-                                               }
                                        }
                                } else {
                                        // Runtime (interp_exec_method_full in interp.c) inserts
                                        // extra stack to hold this and return value, before call.
                                        simulate_runtime_stack_increase (td, 2);
                                        interp_add_ins (td, MINT_NEWOBJ);
+                                       g_assert (!m_class_is_valuetype (klass));
                                        td->last_ins->data [0] = get_data_item_index (td, mono_interp_get_imethod (domain, m, error));
                                }
                                goto_if_nok (error, exit);
@@ -5026,7 +5025,10 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                                        mono_error_set_bad_image (error, image, "Cannot box IsByRefLike type '%s.%s'", m_class_get_name_space (klass), m_class_get_name (klass));
                                        goto exit;
                                }
-                               if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT) {
+
+                               const gboolean vt = mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT;
+
+                               if (vt) {
                                        size = mono_class_value_size (klass, NULL);
                                        size = ALIGN_TO (size, MINT_VT_ALIGNMENT);
                                        td->vt_sp -= size;
@@ -5036,10 +5038,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                                MonoVTable *vtable = mono_class_vtable_checked (domain, klass, error);
                                goto_if_nok (error, exit);
 
-                               if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT)
-                                       interp_add_ins (td, MINT_BOX_VT);
-                               else
-                                       interp_add_ins (td, MINT_BOX);
+                               interp_add_ins (td, vt ? MINT_BOX_VT : MINT_BOX);
                                td->last_ins->data [0] = get_data_item_index (td, vtable);
                                td->last_ins->data [1] = 0;
                                SET_TYPE(td->sp - 1, STACK_TYPE_O, klass);