Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono][interp] Attempt to devirtualize call if we have type information about this #85528

Merged
merged 11 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ private static EqualityComparer<T> CreateComparer()

/////////////////////////////////////////////////
// KEEP THIS IN SYNC WITH THE DEVIRT CODE
// IN METHOD-TO-IR.C
// IN mini_handle_call_res_devirt
/////////////////////////////////////////////////

if (t == typeof(byte))
Expand Down
12 changes: 5 additions & 7 deletions src/mono/mono/metadata/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2516,13 +2516,11 @@ mono_class_get_virtual_method (MonoClass *klass, MonoMethod *method, MonoError *
} else {
res = vtable [method->slot];
}
}

{
if (method->is_inflated) {
/* Have to inflate the result */
res = mono_class_inflate_generic_method_checked (res, &((MonoMethodInflated*)method)->context, error);
}
}
// res can be null if klass is abstract and doesn't implement method
if (res && method->is_inflated) {
/* Have to inflate the result */
res = mono_class_inflate_generic_method_checked (res, &((MonoMethodInflated*)method)->context, error);
}

return res;
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/mini/interp/mintops.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ typedef enum {
#define MINT_IS_SIMD_CREATE(op) ((op) >= MINT_SIMD_V128_I1_CREATE && (op) <= MINT_SIMD_V128_I8_CREATE)

// TODO Add more
#define MINT_NO_SIDE_EFFECTS(op) (MINT_IS_MOV (op) || MINT_IS_LDC_I4 (op) || MINT_IS_LDC_I8 (op) || op == MINT_LDPTR)
#define MINT_NO_SIDE_EFFECTS(op) (MINT_IS_MOV (op) || MINT_IS_LDC_I4 (op) || MINT_IS_LDC_I8 (op) || op == MINT_LDPTR || op == MINT_BOX)

#define MINT_CALL_ARGS 2
#define MINT_CALL_ARGS_SREG -2
Expand Down
119 changes: 104 additions & 15 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -489,14 +489,20 @@ push_var (TransformData *td, int var_index)
} while (0)

static void
set_simple_type_and_local (TransformData *td, StackInfo *sp, int type)
set_type_and_local (TransformData *td, StackInfo *sp, int type, MonoClass *klass)
{
SET_SIMPLE_TYPE (sp, type);
SET_TYPE (sp, type, klass);
create_interp_stack_local (td, sp, MINT_STACK_SLOT_SIZE);
if (!td->optimized)
td->locals [sp->local].stack_offset = sp->offset;
}

static void
set_simple_type_and_local (TransformData *td, StackInfo *sp, int type)
{
set_type_and_local (td, sp, type, NULL);
}

static void
push_type (TransformData *td, int type, MonoClass *k)
{
Expand Down Expand Up @@ -779,8 +785,16 @@ init_bb_stack_state (TransformData *td, InterpBasicBlock *bb)
{
// FIXME If already initialized, then we need to generate mov to the registers in the state.
// Check if already initialized
if (bb->stack_height >= 0)
if (bb->stack_height >= 0) {
// Discard type information if we have type conflicts for stack contents
for (int i = 0; i < bb->stack_height; i++) {
if (bb->stack_state [i].klass != td->stack [i].klass) {
bb->stack_state [i].klass = NULL;
td->stack [i].klass = NULL;
}
}
return;
}

bb->stack_height = GPTRDIFF_TO_INT (td->sp - td->stack);
if (bb->stack_height > 0) {
Expand Down Expand Up @@ -1211,6 +1225,7 @@ mono_interp_jit_call_supported (MonoMethod *method, MonoMethodSignature *sig)

if (mono_aot_only && m_class_get_image (method->klass)->aot_module && !(method->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED)) {
ERROR_DECL (error);
mono_class_init_internal (method->klass);
gpointer addr = mono_aot_get_method (method, error);
if (addr && is_ok (error)) {
MonoAotMethodFlags flags = mono_aot_get_method_flags (addr);
Expand Down Expand Up @@ -3291,6 +3306,38 @@ interp_realign_simd_params (TransformData *td, StackInfo *sp_params, int num_arg
}
}

static MonoMethod*
interp_try_devirt (MonoClass *this_klass, MonoMethod *target_method)
{
ERROR_DECL(error);
// No relevant information about the type
if (!this_klass || this_klass == mono_defaults.object_class)
return NULL;

if (mono_class_is_interface (this_klass))
return NULL;

// Make sure first it is valid to lookup method in the vtable
gboolean assignable;
mono_class_is_assignable_from_checked (target_method->klass, this_klass, &assignable, error);
if (!is_ok (error) || !assignable)
return NULL;

MonoMethod *new_target_method = mono_class_get_virtual_method (this_klass, target_method, error);
if (!is_ok (error) || !new_target_method)
return NULL;

// TODO We would need to emit unboxing in order to devirtualize call to valuetype method
if (m_class_is_valuetype (new_target_method->klass))
return NULL;

// final methods can still be overriden with explicit overrides
if (m_class_is_sealed (this_klass))
return new_target_method;

return NULL;
}

/* 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)
Expand Down Expand Up @@ -3528,6 +3575,19 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
}
}

// Attempt to devirtualize the call
if (is_virtual) {
MonoClass *this_klass = (td->sp - 1 - csignature->param_count)->klass;
MonoMethod *new_target_method = interp_try_devirt (this_klass, target_method);

if (new_target_method) {
if (td->verbose_level)
g_print ("DEVIRTUALIZE %s.%s to %s.%s\n", m_class_get_name (target_method->klass), target_method->name, m_class_get_name (new_target_method->klass), new_target_method->name);
target_method = new_target_method;
is_virtual = FALSE;
}
}

if (op == -1)
target_method = interp_transform_internal_calls (method, target_method, csignature, is_virtual);

Expand All @@ -3538,7 +3598,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
StackInfo *sp = td->sp - 1 - csignature->param_count;
interp_add_ins (td, MINT_CKNULL);
interp_ins_set_sreg (td->last_ins, sp->local);
set_simple_type_and_local (td, sp, sp->type);
set_type_and_local (td, sp, sp->type, sp->klass);
interp_ins_set_dreg (td->last_ins, sp->local);
}

Expand All @@ -3549,7 +3609,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target

if (interp_inline_method (td, target_method, mheader, error)) {
td->ip += 5;
return TRUE;
goto done;
}
}

Expand Down Expand Up @@ -3803,6 +3863,13 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
g_assert (call_offset == -1);
}


done:
if (csignature->ret->type != MONO_TYPE_VOID && target_method) {
MonoClass *ret_klass = mini_handle_call_res_devirt (target_method);
if (ret_klass)
td->sp [-1].klass = ret_klass;
}
return TRUE;
}

Expand Down Expand Up @@ -9024,7 +9091,7 @@ interp_fold_unop (TransformData *td, LocalValue *local_defs, InterpInst *ins)
return ins;
}

#define INTERP_FOLD_UNOP_BR(_opcode,_local_type,_cond) \
#define INTERP_FOLD_UNOP_BR(_opcode,_cond) \
case _opcode: \
if (_cond) { \
ins->opcode = MINT_BR; \
Expand All @@ -9046,18 +9113,30 @@ interp_fold_unop_cond_br (TransformData *td, InterpBasicBlock *cbb, LocalValue *
int sreg = ins->sregs [0];
LocalValue *val = &local_defs [sreg];

if (val->type != LOCAL_VALUE_I4 && val->type != LOCAL_VALUE_I8)
if (val->type != LOCAL_VALUE_I4 && val->type != LOCAL_VALUE_I8 && val->type != LOCAL_VALUE_NON_NULL)
return ins;

// Top of the stack is a constant
switch (ins->opcode) {
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I4, LOCAL_VALUE_I4, val->i == 0);
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I8, LOCAL_VALUE_I8, val->l == 0);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I4, LOCAL_VALUE_I4, val->i != 0);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I8, LOCAL_VALUE_I8, val->l != 0);
if (val->type == LOCAL_VALUE_NON_NULL) {
switch (ins->opcode) {
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I4, FALSE);
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I8, FALSE);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I4, TRUE);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I8, TRUE);

default:
return ins;
default:
return ins;
}
} else {
// Top of the stack is a constant
switch (ins->opcode) {
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I4, val->i == 0);
INTERP_FOLD_UNOP_BR (MINT_BRFALSE_I8, val->l == 0);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I4, val->i != 0);
INTERP_FOLD_UNOP_BR (MINT_BRTRUE_I8, val->l != 0);

default:
return ins;
}
}

if (td->verbose_level) {
Expand Down Expand Up @@ -9816,6 +9895,16 @@ interp_cprop (TransformData *td)
sregs [0] = local;
needs_retry = TRUE;
}
} else if (opcode == MINT_CKNULL) {
InterpInst *def = local_defs [sregs [0]].ins;
if (def && def->opcode == MINT_LDLOCA_S) {
// CKNULL on LDLOCA is a NOP
ins->opcode = MINT_MOV_P;
needs_retry = TRUE;
}
} else if (opcode == MINT_BOX) {
// TODO Add more relevant opcodes
local_defs [dreg].type = LOCAL_VALUE_NON_NULL;
}

ins_index++;
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/interp/transform.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ typedef struct
#define LOCAL_VALUE_LOCAL 1
#define LOCAL_VALUE_I4 2
#define LOCAL_VALUE_I8 3
#define LOCAL_VALUE_NON_NULL 4

// LocalValue contains data to construct an InterpInst that is equivalent with the contents
// of the stack slot / local / argument.
Expand Down
82 changes: 13 additions & 69 deletions src/mono/mono/mini/method-to-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,6 @@ static int inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSigna
static MonoInst*
convert_value (MonoCompile *cfg, MonoType *type, MonoInst *ins);

/* helper methods signatures */

/* type loading helpers */
static GENERATE_GET_CLASS_WITH_CACHE (iequatable, "System", "IEquatable`1")
static GENERATE_GET_CLASS_WITH_CACHE (geqcomparer, "System.Collections.Generic", "GenericEqualityComparer`1");

/*
* Instruction metadata
*/
Expand Down Expand Up @@ -5451,72 +5445,22 @@ emit_optimized_ldloca_ir (MonoCompile *cfg, guchar *ip, guchar *end, int local)
static MonoInst*
handle_call_res_devirt (MonoCompile *cfg, MonoMethod *cmethod, MonoInst *call_res)
{
/*
* Devirt EqualityComparer.Default.Equals () calls for some types.
* The corefx code excepts these calls to be devirtualized.
* This depends on the implementation of EqualityComparer.Default, which is
* in mcs/class/referencesource/mscorlib/system/collections/generic/equalitycomparer.cs
*/
if (m_class_get_image (cmethod->klass) == mono_defaults.corlib &&
!strcmp (m_class_get_name (cmethod->klass), "EqualityComparer`1") &&
!strcmp (cmethod->name, "get_Default")) {
MonoType *param_type = mono_class_get_generic_class (cmethod->klass)->context.class_inst->type_argv [0];
MonoClass *inst;
MonoGenericContext ctx;
ERROR_DECL (error);

memset (&ctx, 0, sizeof (ctx));

MonoType *args [ ] = { param_type };
ctx.class_inst = mono_metadata_get_generic_inst (1, args);

inst = mono_class_inflate_generic_class_checked (mono_class_get_iequatable_class (), &ctx, error);
mono_error_assert_ok (error);
MonoClass *ret_klass = mini_handle_call_res_devirt (cmethod);

/* EqualityComparer<T>.Default returns specific types depending on T */
// FIXME: Add more
// 1. Implements IEquatable<T>
// 2. Nullable<T>
/*
* Can't use this for string/byte as it might use a different comparer:
*
* // Specialize type byte for performance reasons
* if (t == typeof(byte)) {
* return (EqualityComparer<T>)(object)(new ByteEqualityComparer());
* }
* #if MOBILE
* // Breaks .net serialization compatibility
* if (t == typeof (string))
* return (EqualityComparer<T>)(object)new InternalStringComparer ();
* #endif
*/
if (mono_class_is_assignable_from_internal (inst, mono_class_from_mono_type_internal (param_type)) && param_type->type != MONO_TYPE_U1 && param_type->type != MONO_TYPE_STRING) {
MonoInst *typed_objref;
MonoClass *gcomparer_inst;

memset (&ctx, 0, sizeof (ctx));

args [0] = param_type;
ctx.class_inst = mono_metadata_get_generic_inst (1, args);

MonoClass *gcomparer = mono_class_get_geqcomparer_class ();
g_assert (gcomparer);
gcomparer_inst = mono_class_inflate_generic_class_checked (gcomparer, &ctx, error);
if (is_ok (error)) {
MONO_INST_NEW (cfg, typed_objref, OP_TYPED_OBJREF);
typed_objref->type = STACK_OBJ;
typed_objref->dreg = alloc_ireg_ref (cfg);
typed_objref->sreg1 = call_res->dreg;
typed_objref->klass = gcomparer_inst;
MONO_ADD_INS (cfg->cbb, typed_objref);
if (ret_klass) {
MonoInst *typed_objref;
MONO_INST_NEW (cfg, typed_objref, OP_TYPED_OBJREF);
typed_objref->type = STACK_OBJ;
typed_objref->dreg = alloc_ireg_ref (cfg);
typed_objref->sreg1 = call_res->dreg;
typed_objref->klass = ret_klass;
MONO_ADD_INS (cfg->cbb, typed_objref);

call_res = typed_objref;
call_res = typed_objref;

/* Force decompose */
cfg->flags |= MONO_CFG_NEEDS_DECOMPOSE;
cfg->cbb->needs_decompose = TRUE;
}
}
/* Force decompose */
cfg->flags |= MONO_CFG_NEEDS_DECOMPOSE;
cfg->cbb->needs_decompose = TRUE;
}

return call_res;
Expand Down
Loading