From e9f91c7758f7087567d96972a26abe7ebb3efe03 Mon Sep 17 00:00:00 2001 From: Brezae Vlad Date: Wed, 29 Sep 2021 17:13:51 +0300 Subject: [PATCH 1/3] [interp] Fix opcode dump --- src/mono/mono/mini/interp/transform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 979f7a1b7ffe3e..da6735de97d78a 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -1424,6 +1424,7 @@ dump_interp_ins_data (InterpInst *ins, gint32 ins_offset, const guint16 *data, g target = ins_offset + *(gint16*)(data + 1); g_string_append_printf (str, " %u, IR_%04x", *(guint16*)data, target); } + break; case MintOpPair2: g_string_append_printf (str, " %u <- %u, %u <- %u", data [0], data [1], data [2], data [3]); break; From 3f9b2c9744ec5ca33c2e3b901437fcf7e6075628 Mon Sep 17 00:00:00 2001 From: Brezae Vlad Date: Thu, 30 Sep 2021 12:18:45 +0300 Subject: [PATCH 2/3] [interp] Implement tailcalls Before this commit we supported tailcalls only for calls to the same method. Tailcalls are implemented by copying the call arguments to the start of the stack space, followed by replacing of the current executing method. --- src/mono/mono/mini/interp/interp.c | 27 +++++++++++- src/mono/mono/mini/interp/mintops.def | 2 + src/mono/mono/mini/interp/transform.c | 61 ++++++++++++++++++++------- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index abd3e25e65a15c..72376b9c5e0456 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -3414,8 +3414,33 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs LOCAL_VAR (ip [1], gint64) = READ64 (ip + 2); /* note union usage */ ip += 6; MINT_IN_BREAK; + MINT_IN_CASE(MINT_TAILCALL) + MINT_IN_CASE(MINT_TAILCALL_VIRT) MINT_IN_CASE(MINT_JMP) { - InterpMethod *new_method = (InterpMethod*)frame->imethod->data_items [ip [1]]; + gboolean is_tailcall = *ip != MINT_JMP; + InterpMethod *new_method; + + if (is_tailcall) { + guint16 params_offset = ip [1]; + guint16 params_size = ip [3]; + + // Copy the params to their location at the start of the frame + memmove (frame->stack, (guchar*)frame->stack + params_offset, params_size); + new_method = (InterpMethod*)frame->imethod->data_items [ip [2]]; + + if (*ip == MINT_TAILCALL_VIRT) { + gint16 slot = (gint16)ip [4]; + MonoObject *this_arg = LOCAL_VAR (0, MonoObject*); + new_method = get_virtual_method_fast (new_method, this_arg->vtable, slot); + if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (new_method->method->klass)) { + /* unbox */ + gpointer unboxed = mono_object_unbox_internal (this_arg); + LOCAL_VAR (0, gpointer) = unboxed; + } + } + } else { + new_method = (InterpMethod*)frame->imethod->data_items [ip [1]]; + } if (frame->imethod->prof_flags & MONO_PROFILER_CALL_INSTRUMENTATION_TAIL_CALL) MONO_PROFILER_RAISE (method_tail_call, (frame->imethod->method, new_method->method)); diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 3f55869659543a..68aa59dead4270 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -670,6 +670,8 @@ OPDEF(MINT_CALLI_NAT_DYNAMIC, "calli.nat.dynamic", 5, 1, 2, MintOpMethodToken) OPDEF(MINT_CALLI_NAT_FAST, "calli.nat.fast", 7, 1, 2, MintOpMethodToken) OPDEF(MINT_CALL_VARARG, "call.vararg", 6, 1, 1, MintOpMethodToken) OPDEF(MINT_CALLRUN, "callrun", 5, 1, 1, MintOpNoArgs) +OPDEF(MINT_TAILCALL, "tailcall", 4, 0, 1, MintOpMethodToken) +OPDEF(MINT_TAILCALL_VIRT, "tailcall.virt", 5, 0, 1, MintOpMethodToken) OPDEF(MINT_ICALL_V_V, "mono_icall_v_v", 3, 0, 1, MintOpShortInt) OPDEF(MINT_ICALL_V_P, "mono_icall_v_p", 4, 1, 1, MintOpShortInt) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index da6735de97d78a..f43bf1536861a7 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3136,6 +3136,24 @@ interp_emit_arg_conv (TransformData *td, MonoMethodSignature *csignature) emit_convert (td, &arg_start [i], csignature->params [i]); } +static gint16 +get_virt_method_slot (MonoMethod *method) +{ + if (mono_class_is_interface (method->klass)) + return (gint16)(-2 * MONO_IMT_SIZE + mono_method_get_imt_slot (method)); + else + return (gint16)mono_method_get_vtable_slot (method); +} + +static int* +create_call_args (TransformData *td, int num_args) +{ + int *call_args = (int*) mono_mempool_alloc (td->mempool, (num_args + 1) * sizeof (int)); + for (int i = 0; i < num_args; i++) + call_args [i] = td->sp [i].local; + call_args [num_args] = -1; + return call_args; +} /* Return FALSE if error, including inline failure */ static gboolean interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target_method, MonoGenericContext *generic_context, MonoClass *constrained_class, gboolean readonly, MonoError *error, gboolean check_visibility, gboolean save_last_error, gboolean tailcall) @@ -3144,7 +3162,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target MonoMethodSignature *csignature; int is_virtual = *td->ip == CEE_CALLVIRT; int calli = *td->ip == CEE_CALLI || *td->ip == CEE_MONO_CALLI_EXTRA_ARG; - int i; guint32 res_size = 0; int op = -1; int native = 0; @@ -3295,26 +3312,44 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target } CHECK_STACK (td, csignature->param_count + csignature->hasthis); - if (tailcall && !td->gen_sdb_seq_points && !calli && op == -1 && (!is_virtual || (target_method->flags & METHOD_ATTRIBUTE_VIRTUAL) == 0) && + if (tailcall && !td->gen_sdb_seq_points && !calli && op == -1 && (target_method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) == 0 && (target_method->iflags & METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL) == 0 && !(target_method->iflags & METHOD_IMPL_ATTRIBUTE_NOINLINING)) { (void)mono_class_vtable_checked (target_method->klass, error); return_val_if_nok (error, FALSE); - if (method == target_method && *(td->ip + 5) == CEE_RET && !(csignature->hasthis && m_class_is_valuetype (target_method->klass))) { + if (*(td->ip + 5) == CEE_RET) { if (td->inlined_method) return FALSE; if (td->verbose_level) g_print ("Optimize tail call of %s.%s\n", m_class_get_name (target_method->klass), target_method->name); - for (i = csignature->param_count - 1 + !!csignature->hasthis; i >= 0; --i) - store_arg (td, i); + int num_args = csignature->param_count + !!csignature->hasthis; + td->sp -= num_args; + guint32 params_stack_size = get_stack_size (td->sp, num_args); + + int *call_args = create_call_args (td, num_args); + + if (is_virtual) { + interp_add_ins (td, MINT_CKNULL); + interp_ins_set_sreg (td->last_ins, td->sp->local); + set_simple_type_and_local (td, td->sp, td->sp->type); + interp_ins_set_dreg (td->last_ins, td->sp->local); + + interp_add_ins (td, MINT_TAILCALL_VIRT); + td->last_ins->data [2] = get_virt_method_slot (target_method); + } else { + interp_add_ins (td, MINT_TAILCALL); + } + interp_ins_set_sreg (td->last_ins, MINT_CALL_ARGS_SREG); + td->last_ins->data [0] = get_data_item_index (td, mono_interp_get_imethod (target_method, error)); + return_val_if_nok (error, FALSE); + td->last_ins->data [1] = params_stack_size; + td->last_ins->flags |= INTERP_INST_FLAG_CALL; + td->last_ins->info.call_args = call_args; - interp_add_ins (td, MINT_BR); - // We are branching to the beginning of the method - td->last_ins->info.target_bb = td->entry_bb; int in_offset = td->ip - td->il_code; if (interp_ip_in_cbb (td, in_offset + 5)) ++td->ip; /* gobble the CEE_RET if it isn't branched to */ @@ -3370,10 +3405,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target td->sp -= num_args; guint32 params_stack_size = get_stack_size (td->sp, num_args); - int *call_args = (int*) mono_mempool_alloc (td->mempool, (num_args + 1) * sizeof (int)); - for (int i = 0; i < num_args; i++) - call_args [i] = td->sp [i].local; - call_args [num_args] = -1; + int *call_args = create_call_args (td, num_args); // We overwrite it with the return local, save it for future use if (csignature->param_count || csignature->hasthis) @@ -3512,10 +3544,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target td->last_ins->data [2] = params_stack_size; } else if (is_virtual) { interp_add_ins (td, MINT_CALLVIRT_FAST); - if (mono_class_is_interface (target_method->klass)) - td->last_ins->data [1] = -2 * MONO_IMT_SIZE + mono_method_get_imt_slot (target_method); - else - td->last_ins->data [1] = mono_method_get_vtable_slot (target_method); + td->last_ins->data [1] = get_virt_method_slot (target_method); } else if (is_virtual) { interp_add_ins (td, MINT_CALLVIRT); } else { From 78bab105217956073da2ee8bc69b04aa59236a72 Mon Sep 17 00:00:00 2001 From: Brezae Vlad Date: Thu, 30 Sep 2021 12:25:40 +0300 Subject: [PATCH 3/3] Re-enable test --- src/tests/issues.targets | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 320be9692e23ce..4339c5a3e06c6f 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1820,9 +1820,6 @@ https://github.com/dotnet/runtime/issues/54375 - - https://github.com/dotnet/runtime/issues/54374 - needs triage