Skip to content

Commit

Permalink
[interp] Remove some unused paths in newobj and cleanup a little. (#3…
Browse files Browse the repository at this point in the history 
…2685)

This is an alternative to mono/mono#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 <[email protected]>
  • Loading branch information
@monojenkins @jaykrell
monojenkins and jaykrell authored Feb 23, 2020
1 parent 19b7774 commit fd181c0
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 52 deletions.
74 changes: 33 additions & 41 deletions src/mono/mono/mini/interp/interp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5003,8 +5003,6 @@ call:;
memmove (sp + 2, sp, param_count * sizeof (stackval));
}

InterpMethod* const imethod = frame->imethod;

MonoClass * const newobj_class = cmethod->method->klass;

/*if (profiling_classes) {
Expand All @@ -5017,47 +5015,41 @@ call:;
* 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) {
Expand Down
21 changes: 10 additions & 11 deletions src/mono/mono/mini/interp/transform.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down

0 comments on commit fd181c0

Please sign in to comment.