From 5d2bd68d97f23b61c8b969800f23c378015f1b4e Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 13 Jul 2020 17:18:42 +0300 Subject: [PATCH 1/4] [interp] Fix definition of instruction --- src/mono/mono/mini/interp/mintops.def | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 2367907f987bbd..bb072e94f09030 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -786,7 +786,7 @@ OPDEF(MINT_PROF_COVERAGE_STORE, "prof_coverage_store", 5, Pop0, Push0, MintOpLon OPDEF(MINT_INTRINS_ENUM_HASFLAG, "intrins_enum_hasflag", 2, Pop2, Push1, MintOpClassToken) OPDEF(MINT_INTRINS_GET_HASHCODE, "intrins_get_hashcode", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_GET_TYPE, "intrins_get_type", 1, Pop1, Push1, MintOpNoArgs) -OPDEF(MINT_INTRINS_BYREFERENCE_CTOR, "intrins_byreference_ctor", 1, Pop1, Push1, MintOpClassToken) +OPDEF(MINT_INTRINS_BYREFERENCE_CTOR, "intrins_byreference_ctor", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_BYREFERENCE_GET_VALUE, "intrins_byreference_get_value", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_UNSAFE_ADD_BYTE_OFFSET, "intrins_unsafe_add_byte_offset", 1, Pop2, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_UNSAFE_BYTE_OFFSET, "intrins_unsafe_byte_offset", 1, Pop2, Push1, MintOpNoArgs) From 0b226d5faa6147955948235198f31e0f36fc2ed9 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 13 Jul 2020 17:23:54 +0300 Subject: [PATCH 2/4] [interp] Intrinsify span ctor It is heavily used in bcl for static read only spans. In the future we should do copy propagation through its fields so we avoid creating the span in the first place and remove other unecessary opcodes. --- src/mono/mono/metadata/exception.h | 3 +-- src/mono/mono/mini/interp/interp.c | 17 +++++++++++++++++ src/mono/mono/mini/interp/mintops.def | 1 + src/mono/mono/mini/interp/transform.c | 23 ++++++++++++++++++++++- 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/exception.h b/src/mono/mono/metadata/exception.h index 721dd493ae0459..155c23c7e77476 100644 --- a/src/mono/mono/metadata/exception.h +++ b/src/mono/mono/metadata/exception.h @@ -106,8 +106,7 @@ mono_get_exception_argument_null (const char *arg); MONO_API MonoException * mono_get_exception_argument (const char *arg, const char *msg); -MONO_API MONO_RT_EXTERNAL_ONLY -MonoException * +MONO_API MonoException * mono_get_exception_argument_out_of_range (const char *arg); MONO_API MONO_RT_EXTERNAL_ONLY diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 8640163368d4d5..d107b8d1b541d9 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -5269,6 +5269,23 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs ip++; MINT_IN_BREAK; } + MINT_IN_CASE(MINT_INTRINS_SPAN_CTOR) { + gpointer ptr = sp [-2].data.p; + int len = sp [-1].data.i; + if (len < 0) + THROW_EX (mono_get_exception_argument_out_of_range ("length"), ip); + *(gpointer*)vt_sp = ptr; + *(gint32*)((gpointer*)vt_sp + 1) = len; + sp [-2].data.p = vt_sp; +#if SIZEOF_VOID_P == 8 + vt_sp += ALIGN_TO (12, MINT_VT_ALIGNMENT); +#else + vt_sp += ALIGN_TO (8, MINT_VT_ALIGNMENT); +#endif + sp--; + ip++; + MINT_IN_BREAK; + } MINT_IN_CASE(MINT_INTRINS_BYREFERENCE_GET_VALUE) { gpointer *byreference_this = (gpointer*)sp [-1].data.p; sp [-1].data.p = *byreference_this; diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index bb072e94f09030..488fc86dded31e 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -787,6 +787,7 @@ OPDEF(MINT_INTRINS_ENUM_HASFLAG, "intrins_enum_hasflag", 2, Pop2, Push1, MintOpC OPDEF(MINT_INTRINS_GET_HASHCODE, "intrins_get_hashcode", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_GET_TYPE, "intrins_get_type", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_BYREFERENCE_CTOR, "intrins_byreference_ctor", 1, Pop1, Push1, MintOpNoArgs) +OPDEF(MINT_INTRINS_SPAN_CTOR, "intrins_span_ctor", 1, Pop2, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_BYREFERENCE_GET_VALUE, "intrins_byreference_get_value", 1, Pop1, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_UNSAFE_ADD_BYTE_OFFSET, "intrins_unsafe_add_byte_offset", 1, Pop2, Push1, MintOpNoArgs) OPDEF(MINT_INTRINS_UNSAFE_BYTE_OFFSET, "intrins_unsafe_byte_offset", 1, Pop2, Push1, MintOpNoArgs) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index accc016e4ab968..c75c481bbee3cf 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3062,6 +3062,18 @@ mono_test_interp_method_compute_offsets (TransformData *td, InterpMethod *imetho interp_method_compute_offsets (td, imethod, signature, header, error); } +static gboolean +type_has_references (MonoType *type) +{ + if (MONO_TYPE_IS_REFERENCE (type)) + return TRUE; + if (MONO_TYPE_ISSTRUCT (type)) { + MonoClass *klass = mono_class_from_mono_type_internal (type); + return m_class_has_references (klass); + } + return FALSE; +} + /* Return false is failure to init basic blocks due to being in inline method */ static gboolean init_bb_start (TransformData *td, MonoMethodHeader *header, gboolean inlining) @@ -4743,6 +4755,14 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, /* public ByReference(ref T value) */ g_assert (csignature->hasthis && csignature->param_count == 1); interp_add_ins (td, MINT_INTRINS_BYREFERENCE_CTOR); + } else if (m_class_get_image (klass) == mono_defaults.corlib && + (!strcmp (m_class_get_name (m->klass), "Span`1") || + !strcmp (m_class_get_name (m->klass), "ReadOnlySpan`1")) && + csignature->param_count == 2 && + csignature->params [0]->type == MONO_TYPE_PTR && + !type_has_references (mono_method_get_context (m)->class_inst->type_argv [0])) { + /* ctor frequently used with ReadOnlySpan over static arrays */ + interp_add_ins (td, MINT_INTRINS_SPAN_CTOR); } else if (klass != mono_defaults.string_class && !mono_class_is_marshalbyref (klass) && !mono_class_has_finalizer (klass) && @@ -4827,7 +4847,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, } } if ((vt_stack_used != 0 || vt_res_size != 0) && - td->last_ins->opcode != MINT_INTRINS_BYREFERENCE_CTOR) { + td->last_ins->opcode != MINT_INTRINS_BYREFERENCE_CTOR && + td->last_ins->opcode != MINT_INTRINS_SPAN_CTOR) { /* FIXME Remove this once vtsp and sp are unified */ interp_add_ins (td, MINT_VTRESULT); td->last_ins->data [0] = vt_res_size; From 97688709ae594a2cec274f26d4d67a533246ce59 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 13 Jul 2020 17:26:34 +0300 Subject: [PATCH 3/4] [interp] Allow method inlining if constructor is intrinsified --- src/mono/mono/mini/interp/transform.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index c75c481bbee3cf..4510746222b304 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -4741,6 +4741,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, PUSH_TYPE (td, stack_type [mint_type (m_class_get_byval_arg (klass))], klass); } else { + gboolean can_inline = TRUE; if (m_class_get_parent (klass) == mono_defaults.array_class) { interp_add_ins (td, MINT_NEWOBJ_ARRAY); td->last_ins->data [0] = get_data_item_index (td, m->klass); @@ -4820,6 +4821,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, move_stack (td, (td->sp - td->stack) - csignature->param_count, -2); // Set the method to be executed as part of newobj instruction newobj_fast->data [0] = get_data_item_index (td, mono_interp_get_imethod (domain, m, error)); + can_inline = FALSE; } else { // Runtime (interp_exec_method_full in interp.c) inserts // extra stack to hold this and return value, before call. @@ -4829,8 +4831,10 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, td->last_ins->data [0] = get_data_item_index (td, mono_interp_get_imethod (domain, m, error)); } goto_if_nok (error, exit); - /* The constructor was not inlined, abort inlining of current method */ - INLINE_FAILURE; + if (!can_inline) { + /* The constructor was not inlined, abort inlining of current method */ + INLINE_FAILURE; + } td->sp -= csignature->param_count; if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT) { From d4f6b0c795bee1d80163030045432fd66d4030c9 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 14 Jul 2020 18:57:33 +0300 Subject: [PATCH 4/4] Fix case where has_references is not yet initialized --- src/mono/mono/mini/interp/transform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 4510746222b304..f0a333d04dcf79 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3069,6 +3069,8 @@ type_has_references (MonoType *type) return TRUE; if (MONO_TYPE_ISSTRUCT (type)) { MonoClass *klass = mono_class_from_mono_type_internal (type); + if (!m_class_is_inited (klass)) + mono_class_init_internal (klass); return m_class_has_references (klass); } return FALSE;