[interp] Optimize multidimensional array access (mono/mono#16822)

* [interp] Pass rank instead to LDELEMA

It feels more intuitive and uses less computations.

* [interp] Avoid unnecessary type check

Loading element address of array requires type checks only if the elements of the array are references.

* [interp] Avoid using MINT_CALLRUN for Get intrinsic

It is very slow. Use ldelema/ldobj pair instead.

* [interp] Optimize MINT_LDELEMA

* [interp] Rename ldelema_fast to ldelema1

ldelema_fast was handling arrays with a single dimension. Rename it to better suggest this.

* [interp] Avoid using MINT_CALLRUN for Set intrinsic

* [interp] Remove some duplicated and confusing code

* [interp] Fix stack type of MINT_NEWARR result

Commit migrated from https://github.com/mono/mono/commit/aaaa2bdd5e5795dfba689cde110999a9a9225fb2

diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
index bf56804..848f97a 100644 (file)
--- a/src/mono/mono/mini/interp/interp.c
+++ b/src/mono/mono/mini/interp/interp.c
@@ -1003,36 +1003,6 @@ ves_array_calculate_index (MonoArray *ao, stackval *sp, gboolean safe)
        return pos;
 }
 
-static MONO_NEVER_INLINE MonoException*
-ves_array_set (InterpFrame *frame, stackval *sp, MonoMethodSignature *sig)
-{
-       MonoObject *o = sp->data.o;
-       MonoArray *ao = (MonoArray *) o;
-       MonoClass *ac = o->vtable->klass;
-
-       g_assert (m_class_get_rank (ac) >= 1);
-
-       gint32 pos = ves_array_calculate_index (ao, sp + 1, TRUE);
-       if (pos == -1)
-               return mono_get_exception_index_out_of_range ();
-
-       int val_index = 1 + m_class_get_rank (ac);
-       if (sp [val_index].data.p && !m_class_is_valuetype (m_class_get_element_class (mono_object_class (o)))) {
-               ERROR_DECL (error);
-               MonoObject *isinst = mono_object_isinst_checked (sp [val_index].data.o, m_class_get_element_class (mono_object_class (o)), error);
-               mono_error_cleanup (error);
-               if (!isinst)
-                       return mono_get_exception_array_type_mismatch ();
-       }
-
-       gint32 esize = mono_array_element_size (ac);
-       gpointer ea = mono_array_addr_with_size_fast (ao, esize, pos);
-
-       MonoType *mt = sig->params [m_class_get_rank (ac)];
-       stackval_to_data (mt, &sp [val_index], ea, FALSE);
-       return NULL;
-}
-
 static MonoException*
 ves_array_get (InterpFrame *frame, stackval *sp, stackval *retval, MonoMethodSignature *sig, gboolean safe)
 {
@@ -1519,14 +1489,6 @@ ves_imethod (InterpFrame *frame, MonoMethod *method, MonoMethodSignature *sig, s
                }
                if (!strcmp (name, "UnsafeLoad"))
                        return ves_array_get (frame, sp, retval, sig, FALSE);
-       } else if (mini_class_is_system_array (method->klass)) {
-               MonoObject *obj = (MonoObject*) sp->data.p;
-               if (!obj)
-                       return mono_get_exception_null_reference ();
-               if (*name == 'S' && (strcmp (name, "Set") == 0))
-                       return ves_array_set (frame, sp, sig);
-               if (*name == 'G' && (strcmp (name, "Get") == 0))
-                       return ves_array_get (frame, sp, retval, sig, TRUE);
        }
        
        g_error ("Don't know how to exec runtime method %s.%s::%s", 
@@ -5372,7 +5334,7 @@ common_vcall:
                        ip++;
                        MINT_IN_BREAK;
                }
-               MINT_IN_CASE(MINT_LDELEMA_FAST) {
+               MINT_IN_CASE(MINT_LDELEMA1) {
                        /* No bounds, one direction */
                        MonoArray *ao = (MonoArray*)sp [-2].data.o;
                        NULL_CHECK (ao);
@@ -5386,23 +5348,42 @@ common_vcall:
 
                        MINT_IN_BREAK;
                }
-               MINT_IN_CASE(MINT_LDELEMA)
+               MINT_IN_CASE(MINT_LDELEMA) {
+                       guint16 rank = ip [1];
+                       gint32 const esize = READ32 (ip + 2);
+                       ip += 4;
+                       sp -= rank;
+
+                       MonoArray* const ao = (MonoArray*) sp [-1].data.o;
+                       NULL_CHECK (ao);
+
+                       g_assert (ao->bounds);
+                       guint32 pos = 0;
+                       for (int i = 0; i < rank; i++) {
+                               guint32 idx = sp [i].data.i;
+                               guint32 lower = ao->bounds [i].lower_bound;
+                               guint32 len = ao->bounds [i].length;
+                               if (idx < lower || (idx - lower) >= len)
+                                       THROW_EX (mono_get_exception_index_out_of_range (), ip);
+                               pos = (pos * len) + idx - lower;
+                       }
+
+                       sp [-1].data.p = mono_array_addr_with_size_fast (ao, esize, pos);
+                       MINT_IN_BREAK;
+               }
                MINT_IN_CASE(MINT_LDELEMA_TC) {
-                       
-                       guint16 numargs = ip [2];
+                       guint16 rank = ip [1];
                        ip += 3;
-                       sp -= numargs;
+                       sp -= rank;
 
-                       MonoObject* const o = sp [0].data.o;
+                       MonoObject* const o = sp [-1].data.o;
                        NULL_CHECK (o);
 
-                       MonoClass *klass = (MonoClass*)frame->imethod->data_items [ip [-3 + 1]];
+                       MonoClass *klass = (MonoClass*)frame->imethod->data_items [ip [-3 + 2]];
                        const gboolean needs_typecheck = ip [-3] == MINT_LDELEMA_TC;
-                       MonoException *ex = ves_array_element_address (frame, klass, (MonoArray *) o, &sp [1], needs_typecheck);
+                       MonoException *ex = ves_array_element_address (frame, klass, (MonoArray *) o, sp, needs_typecheck);
                        if (ex)
                                THROW_EX (ex, ip);
-                       ++sp;
-
                        MINT_IN_BREAK;
                }
                MINT_IN_CASE(MINT_LDELEM_I1) /* fall through */

diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def
index 6e5844a..0dcf893 100644 (file)
--- a/src/mono/mono/mini/interp/mintops.def
+++ b/src/mono/mono/mini/interp/mintops.def
@@ -400,9 +400,9 @@ OPDEF(MINT_LDELEM_R8, "ldelem.r8", 1, Pop2, Push1, MintOpNoArgs)
 OPDEF(MINT_LDELEM_REF, "ldelem.ref", 1, Pop2, Push1, MintOpNoArgs)
 OPDEF(MINT_LDELEM_VT, "ldelem.vt", 3, Pop2, Push1, MintOpInt)
 
-OPDEF(MINT_LDELEMA, "ldelema", 3, VarPop, Push1, MintOpTwoShorts)
+OPDEF(MINT_LDELEMA1, "ldelema1", 3, Pop2, Push1, MintOpInt)
+OPDEF(MINT_LDELEMA, "ldelema", 4, VarPop, Push1, MintOpTwoShorts)
 OPDEF(MINT_LDELEMA_TC, "ldelema.tc", 3, VarPop, Push1, MintOpTwoShorts)
-OPDEF(MINT_LDELEMA_FAST, "ldelema.fast", 3, Pop2, Push1, MintOpInt)
 
 OPDEF(MINT_STELEM_I, "stelem.i", 1, Pop3, Push0, MintOpNoArgs)
 OPDEF(MINT_STELEM_I1, "stelem.i1", 1, Pop3, Push0, MintOpNoArgs)

diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c
index 2dcc70f..974dba9 100644 (file)
--- a/src/mono/mono/mini/interp/transform.c
+++ b/src/mono/mono/mini/interp/transform.c
@@ -1069,6 +1069,96 @@ interp_get_ldind_for_mt (int mt)
        return -1;
 }
 
+static void
+interp_emit_ldobj (TransformData *td, MonoClass *klass)
+{
+       int mt = mint_type (m_class_get_byval_arg (klass));
+       int size;
+
+       if (mt == MINT_TYPE_VT) {
+               interp_add_ins (td, MINT_LDOBJ_VT);
+               size = mono_class_value_size (klass, NULL);
+               WRITE32_INS (td->last_ins, 0, &size);
+               PUSH_VT (td, size);
+       } else {
+               int opcode = interp_get_ldind_for_mt (mt);
+               interp_add_ins (td, opcode);
+       }
+
+       SET_TYPE (td->sp - 1, stack_type [mt], klass);
+}
+
+static void
+interp_emit_stobj (TransformData *td, MonoClass *klass)
+{
+       int mt = mint_type (m_class_get_byval_arg (klass));
+
+       if (mt == MINT_TYPE_VT) {
+               int size;
+               interp_add_ins (td, MINT_STOBJ_VT);
+               td->last_ins->data [0] = get_data_item_index(td, klass);
+               size = mono_class_value_size (klass, NULL);
+               POP_VT (td, size);
+       } else {
+               int opcode;
+               switch (mt) {
+                       case MINT_TYPE_I1:
+                       case MINT_TYPE_U1:
+                               opcode = MINT_STIND_I1;
+                               break;
+                       case MINT_TYPE_I2:
+                       case MINT_TYPE_U2:
+                               opcode = MINT_STIND_I2;
+                               break;
+                       case MINT_TYPE_I4:
+                               opcode = MINT_STIND_I4;
+                               break;
+                       case MINT_TYPE_I8:
+                               opcode = MINT_STIND_I8;
+                               break;
+                       case MINT_TYPE_R4:
+                               opcode = MINT_STIND_R4;
+                               break;
+                       case MINT_TYPE_R8:
+                               opcode = MINT_STIND_R8;
+                               break;
+                       case MINT_TYPE_O:
+                               opcode = MINT_STIND_REF;
+                               break;
+                       default: g_assert_not_reached (); break;
+               }
+               interp_add_ins (td, opcode);
+       }
+       td->sp -= 2;
+}
+
+static void
+interp_emit_ldelema (TransformData *td, MonoClass *array_class, MonoClass *check_class)
+{
+       MonoClass *element_class = m_class_get_element_class (array_class);
+       int rank = m_class_get_rank (array_class);
+       int size = mono_class_array_element_size (element_class);
+
+       // We only need type checks when writing to array of references
+       if (!check_class || m_class_is_valuetype (element_class)) {
+               if (rank == 1) {
+                       interp_add_ins (td, MINT_LDELEMA1);
+                       WRITE32_INS (td->last_ins, 0, &size);
+               } else {
+                       interp_add_ins (td, MINT_LDELEMA);
+                       td->last_ins->data [0] = rank;
+                       WRITE32_INS (td->last_ins, 1, &size);
+               }
+       } else {
+               interp_add_ins (td, MINT_LDELEMA_TC);
+               td->last_ins->data [0] = rank;
+               td->last_ins->data [1] = get_data_item_index (td, check_class);
+       }
+
+       td->sp -= rank;
+       SET_SIMPLE_TYPE (td->sp - 1, STACK_TYPE_MP);
+}
+
 /* Return TRUE if call transformation is finished */
 static gboolean
 interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClass *constrained_class, MonoMethodSignature *csignature, gboolean readonly, int *op)
@@ -1293,9 +1383,34 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
                } else if (!strcmp (tm, "get_Length")) {
                        *op = MINT_LDLEN;
                } else if (!strcmp (tm, "Address")) {
-                       *op = readonly ? MINT_LDELEMA : MINT_LDELEMA_TC;
-               } else if (!strcmp (tm, "UnsafeMov") || !strcmp (tm, "UnsafeLoad") || !strcmp (tm, "Set") || !strcmp (tm, "Get")) {
+                       MonoClass *check_class = readonly ? NULL : m_class_get_element_class (target_method->klass);
+                       interp_emit_ldelema (td, target_method->klass, check_class);
+                       td->ip += 5;
+                       return TRUE;
+               } else if (!strcmp (tm, "UnsafeMov") || !strcmp (tm, "UnsafeLoad")) {
                        *op = MINT_CALLRUN;
+               } else if (!strcmp (tm, "Get")) {
+                       interp_emit_ldelema (td, target_method->klass, NULL);
+                       interp_emit_ldobj (td, m_class_get_element_class (target_method->klass));
+                       td->ip += 5;
+                       return TRUE;
+               } else if (!strcmp (tm, "Set")) {
+                       MonoClass *element_class = m_class_get_element_class (target_method->klass);
+                       MonoType *local_type = m_class_get_byval_arg (element_class);
+                       MonoClass *value_class = td->sp [-1].klass;
+                       // If value_class is NULL it means the top of stack is a simple type (valuetype)
+                       // which doesn't require type checks, or that we have no type information because
+                       // the code is unsafe (like in some wrappers). In that case we assume the type
+                       // of the array and don't do any checks.
+
+                       int local = create_interp_local (td, local_type);
+
+                       store_local_general (td, local, local_type);
+                       interp_emit_ldelema (td, target_method->klass, value_class);
+                       load_local_general (td, local, local_type);
+                       interp_emit_stobj (td, element_class);
+                       td->ip += 5;
+                       return TRUE;
                } else if (!strcmp (tm, "UnsafeStore")) {
                        g_error ("TODO ArrayClass::UnsafeStore");
                }
@@ -2183,10 +2298,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
                        SET_SIMPLE_TYPE (td->sp - 1, STACK_TYPE_I4);
 #endif
                }
-               if (op == MINT_LDELEMA || op == MINT_LDELEMA_TC) {
-                       td->last_ins->data [0] = get_data_item_index (td, m_class_get_element_class (target_method->klass));
-                       td->last_ins->data [1] = 1 + m_class_get_rank (target_method->klass);
-               }
 
                if (op == MINT_CALLRUN) {
                        td->last_ins->data [0] = get_data_item_index (td, target_method);
@@ -2796,69 +2907,6 @@ interp_handle_isinst (TransformData *td, MonoClass *klass, gboolean isinst_instr
 }
 
 static void
-interp_emit_ldobj (TransformData *td, MonoClass *klass)
-{
-       int mt = mint_type (m_class_get_byval_arg (klass));
-       int size;
-
-       if (mt == MINT_TYPE_VT) {
-               interp_add_ins (td, MINT_LDOBJ_VT);
-               size = mono_class_value_size (klass, NULL);
-               WRITE32_INS (td->last_ins, 0, &size);
-               PUSH_VT (td, size);
-       } else {
-               int opcode = interp_get_ldind_for_mt (mt);
-               interp_add_ins (td, opcode);
-       }
-
-       SET_TYPE (td->sp - 1, stack_type [mt], klass);
-}
-
-static void
-interp_emit_stobj (TransformData *td, MonoClass *klass)
-{
-       int mt = mint_type (m_class_get_byval_arg (klass));
-
-       if (mt == MINT_TYPE_VT) {
-               int size;
-               interp_add_ins (td, MINT_STOBJ_VT);
-               td->last_ins->data [0] = get_data_item_index(td, klass);
-               size = mono_class_value_size (klass, NULL);
-               POP_VT (td, size);
-       } else {
-               int opcode;
-               switch (mt) {
-                       case MINT_TYPE_I1:
-                       case MINT_TYPE_U1:
-                               opcode = MINT_STIND_I1;
-                               break;
-                       case MINT_TYPE_I2:
-                       case MINT_TYPE_U2:
-                               opcode = MINT_STIND_I2;
-                               break;
-                       case MINT_TYPE_I4:
-                               opcode = MINT_STIND_I4;
-                               break;
-                       case MINT_TYPE_I8:
-                               opcode = MINT_STIND_I8;
-                               break;
-                       case MINT_TYPE_R4:
-                               opcode = MINT_STIND_R4;
-                               break;
-                       case MINT_TYPE_R8:
-                               opcode = MINT_STIND_R8;
-                               break;
-                       case MINT_TYPE_O:
-                               opcode = MINT_STIND_REF;
-                               break;
-                       default: g_assert_not_reached (); break;
-               }
-               interp_add_ins (td, opcode);
-       }
-       td->sp -= 2;
-}
-
-static void
 interp_emit_ldsflda (TransformData *td, MonoClassField *field, MonoError *error)
 {
        MonoDomain *domain = td->rtm->domain;
@@ -4787,7 +4835,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                        SET_SIMPLE_TYPE (td->sp - 1, STACK_TYPE_I4);
                        interp_add_ins (td, MINT_NEWARR);
                        td->last_ins->data [0] = get_data_item_index (td, vtable);
-                       SET_TYPE (td->sp - 1, STACK_TYPE_O, klass);
+                       SET_TYPE (td->sp - 1, STACK_TYPE_O, array_class);
                        td->ip += 5;
                        break;
                }
@@ -4821,10 +4869,10 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                                mono_class_setup_vtable (klass);
                                CHECK_TYPELOAD (klass);
                                interp_add_ins (td, MINT_LDELEMA_TC);
-                               td->last_ins->data [0] = get_data_item_index (td, klass);
-                               td->last_ins->data [1] = 2;
+                               td->last_ins->data [0] = 1;
+                               td->last_ins->data [1] = get_data_item_index (td, klass);
                        } else {
-                               interp_add_ins (td, MINT_LDELEMA_FAST);
+                               interp_add_ins (td, MINT_LDELEMA1);
                                mono_class_init_internal (klass);
                                size = mono_class_array_element_size (klass);
                                WRITE32_INS (td->last_ins, 0, &size);
@@ -6049,9 +6097,12 @@ get_inst_stack_usage (TransformData *td, InterpInst *ins, int *pop, int *push)
                case MINT_NEWOBJ_ARRAY:
                case MINT_NEWOBJ_VT_FAST:
                case MINT_NEWOBJ_VTST_FAST:
+                       *pop = ins->data [1];
+                       *push = 1;
+                       break;
                case MINT_LDELEMA:
                case MINT_LDELEMA_TC:
-                       *pop = ins->data [1];
+                       *pop = ins->data [0] + 1;
                        *push = 1;
                        break;
                case MINT_NEWOBJ: {