Skip to content

Commit

Permalink
[mono][interp] Optimize out some ldloca uses (#76669)
Browse files Browse the repository at this point in the history
* [mono][interp] Remove redundant opcode

Also allowing general optimizations that are being applied to ldfld opcodes.

* [mono][interp] Remove ldloca when used with ldfld/stfld

Generate direct moves instead to the valuetype interior offset

* [mono][interp] Replace ldloca + ldind/stdind with simple var moves
  • Loading branch information
BrzVlad authored Oct 11, 2022
1 parent 9af8303 commit 94d7503
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 45 deletions.
12 changes: 2 additions & 10 deletions src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -6042,15 +6042,6 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
ip += 3;
MINT_IN_BREAK;
}
MINT_IN_CASE(MINT_LDLEN_SPAN) {
MonoObject *o = LOCAL_VAR (ip [2], MonoObject*);
NULL_CHECK (o);
// FIXME What's the point of this opcode ? It's just a LDFLD
gsize offset_length = (gsize)(gint16)ip [3];
LOCAL_VAR (ip [1], mono_u) = *(gint32 *) ((guint8 *) o + offset_length);
ip += 4;
MINT_IN_BREAK;
}
MINT_IN_CASE(MINT_GETCHR) {
MonoString *s = LOCAL_VAR (ip [2], MonoString*);
NULL_CHECK (s);
Expand Down Expand Up @@ -7099,7 +7090,8 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
ip += 3;
MINT_IN_BREAK;

MINT_IN_CASE(MINT_MOV_OFF)
MINT_IN_CASE(MINT_MOV_SRC_OFF)
MINT_IN_CASE(MINT_MOV_DST_OFF)
// This opcode is resolved to a normal MINT_MOV when emitting compacted instructions
g_assert_not_reached ();
MINT_IN_BREAK;
Expand Down
4 changes: 2 additions & 2 deletions src/mono/mono/mini/interp/mintops.def
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ OPDEF(MINT_STSFLD_W, "stsfld.w", 8, 0, 1, MintOpTwoInts)
OPDEF(MINT_LDSFLDA, "ldsflda", 4, 1, 0, MintOpTwoShorts)
OPDEF(MINT_LDTSFLDA, "ldtsflda", 4, 1, 0, MintOpInt)

OPDEF(MINT_MOV_OFF, "mov.off", 6, 1, 1, MintOpTwoShorts)
OPDEF(MINT_MOV_SRC_OFF, "mov.src.off", 6, 1, 1, MintOpTwoShorts)
OPDEF(MINT_MOV_DST_OFF, "mov.dst.off", 6, 1, 1, MintOpTwoShorts)

OPDEF(MINT_MOV_I1, "mov.i1", 3, 1, 1, MintOpNoArgs)
OPDEF(MINT_MOV_U1, "mov.u1", 3, 1, 1, MintOpNoArgs)
Expand Down Expand Up @@ -410,7 +411,6 @@ OPDEF(MINT_STELEM_REF, "stelem.ref", 4, 0, 3, MintOpNoArgs)
OPDEF(MINT_STELEM_VT, "stelem.vt", 6, 0, 3, MintOpTwoShorts)

OPDEF(MINT_LDLEN, "ldlen", 3, 1, 1, MintOpNoArgs)
OPDEF(MINT_LDLEN_SPAN, "ldlen.span", 4, 1, 1, MintOpShortInt)

OPDEF(MINT_GETITEM_SPAN, "getitem.span", 7, 1, 2, MintOpTwoShorts)

Expand Down
2 changes: 2 additions & 0 deletions src/mono/mono/mini/interp/mintops.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ typedef enum {
#define MINT_IS_LDFLD(op) ((op) >= MINT_LDFLD_I1 && (op) <= MINT_LDFLD_O)
#define MINT_IS_STFLD(op) ((op) >= MINT_STFLD_I1 && (op) <= MINT_STFLD_O)
#define MINT_IS_LDIND_INT(op) ((op) >= MINT_LDIND_I1 && (op) <= MINT_LDIND_I8)
#define MINT_IS_LDIND(op) ((op) >= MINT_LDIND_I1 && (op) <= MINT_LDIND_R8)
#define MINT_IS_STIND_INT(op) ((op) >= MINT_STIND_I1 && (op) <= MINT_STIND_I8)
#define MINT_IS_STIND(op) ((op) >= MINT_STIND_I1 && (op) <= MINT_STIND_REF)

#define MINT_CALL_ARGS 2
#define MINT_CALL_ARGS_SREG -2
Expand Down
190 changes: 157 additions & 33 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,33 @@ get_mov_for_type (int mt, gboolean needs_sext)
g_assert_not_reached ();
}

static int
get_mint_type_size (int mt)
{
switch (mt) {
case MINT_TYPE_I1:
case MINT_TYPE_U1:
return 1;
case MINT_TYPE_I2:
case MINT_TYPE_U2:
return 2;
case MINT_TYPE_I4:
case MINT_TYPE_R4:
return 4;
case MINT_TYPE_I8:
case MINT_TYPE_R8:
return 8;
case MINT_TYPE_O:
#if SIZEOF_VOID_P == 8
return 8;
#else
return 4;
#endif
}
g_assert_not_reached ();
}


// Should be called when td->cbb branches to newbb and newbb can have a stack state
static void
fixup_newbb_stack_locals (TransformData *td, InterpBasicBlock *newbb)
Expand Down Expand Up @@ -1753,6 +1780,24 @@ interp_get_ldind_for_mt (int mt)
return -1;
}

static int
interp_get_mt_for_ldind (int ldind_op)
{
switch (ldind_op) {
case MINT_LDIND_I1: return MINT_TYPE_I1;
case MINT_LDIND_U1: return MINT_TYPE_U1;
case MINT_LDIND_I2: return MINT_TYPE_I2;
case MINT_LDIND_U2: return MINT_TYPE_U2;
case MINT_LDIND_I4: return MINT_TYPE_I4;
case MINT_LDIND_I8: return MINT_TYPE_I8;
case MINT_LDIND_R4: return MINT_TYPE_R4;
case MINT_LDIND_R8: return MINT_TYPE_R8;
default:
g_assert_not_reached ();
}
return -1;
}

static int
interp_get_stind_for_mt (int mt)
{
Expand Down Expand Up @@ -2107,18 +2152,6 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
td->ip += 5;
return TRUE;
}
} else if (!strcmp (tm, "get_Length")) {
MonoClassField *length_field = mono_class_get_field_from_name_full (target_method->klass, "_length", NULL);
g_assert (length_field);
int offset_length = m_field_get_offset (length_field) - sizeof (MonoObject);
interp_add_ins (td, MINT_LDLEN_SPAN);
td->last_ins->data [0] = GINT_TO_UINT16 (offset_length);
td->sp--;
interp_ins_set_sreg (td->last_ins, td->sp [0].local);
push_simple_type (td, STACK_TYPE_I4);
interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
td->ip += 5;
return TRUE;
}
} else if (in_corlib && !strcmp (klass_name_space, "System.Runtime.CompilerServices") && !strcmp (klass_name, "Unsafe")) {
if (!strcmp (tm, "AddByteOffset"))
Expand Down Expand Up @@ -6054,7 +6087,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
size = 8;
}
#endif
interp_add_ins (td, MINT_MOV_OFF);
interp_add_ins (td, MINT_MOV_SRC_OFF);
g_assert (m_class_is_valuetype (klass));
td->sp--;
interp_ins_set_sreg (td->last_ins, td->sp [0].local);
Expand Down Expand Up @@ -7938,13 +7971,17 @@ emit_compacted_instruction (TransformData *td, guint16* start_ip, InterpInst *in
// IL_SEQ_POINT shouldn't exist in the emitted code, we undo the ip position
if (opcode == MINT_IL_SEQ_POINT)
return ip - 1;
} else if (opcode == MINT_MOV_OFF) {
} else if (opcode == MINT_MOV_SRC_OFF || opcode == MINT_MOV_DST_OFF) {
guint16 foff = ins->data [0];
guint16 mt = ins->data [1];
guint16 fsize = ins->data [2];

int dest_off = get_local_offset (td, ins->dreg);
int src_off = get_local_offset (td, ins->sregs [0]) + foff;
int src_off = get_local_offset (td, ins->sregs [0]);
if (opcode == MINT_MOV_SRC_OFF)
src_off += foff;
else
dest_off += foff;
if (mt == MINT_TYPE_VT || fsize)
opcode = MINT_MOV_VT;
else
Expand Down Expand Up @@ -8764,19 +8801,67 @@ interp_cprop (TransformData *td)
}
} else if (MINT_IS_BINOP_CONDITIONAL_BRANCH (opcode)) {
ins = interp_fold_binop_cond_br (td, bb, local_defs, ins);
} else if (MINT_IS_LDFLD (opcode) && ins->data [0] == 0) {
} else if (MINT_IS_LDIND (opcode)) {
InterpInst *ldloca = local_defs [sregs [0]].ins;
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S) {
int local = ldloca->sregs [0];
int mt = td->locals [local].mt;
if (mt != MINT_TYPE_VT) {
// LDIND cannot simply be replaced with a mov because it might also include a
// necessary conversion (especially when we do cprop and do moves between vars of
// different types).
int ldind_mt = interp_get_mt_for_ldind (opcode);
switch (ldind_mt) {
case MINT_TYPE_I1: ins->opcode = MINT_CONV_I1_I4; break;
case MINT_TYPE_U1: ins->opcode = MINT_CONV_U1_I4; break;
case MINT_TYPE_I2: ins->opcode = MINT_CONV_I2_I4; break;
case MINT_TYPE_U2: ins->opcode = MINT_CONV_U2_I4; break;
default:
ins->opcode = GINT_TO_OPCODE (get_mov_for_type (ldind_mt, FALSE));
break;
}
local_ref_count [sregs [0]]--;
interp_ins_set_sreg (ins, local);

if (td->verbose_level) {
g_print ("Replace ldloca/ldind pair :\n\t");
dump_interp_inst (ins);
}
needs_retry = TRUE;
}
}
} else if (MINT_IS_LDFLD (opcode)) {
InterpInst *ldloca = local_defs [sregs [0]].ins;
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S &&
td->locals [ldloca->sregs [0]].mt == (ins->opcode - MINT_LDFLD_I1)) {
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S) {
int mt = ins->opcode - MINT_LDFLD_I1;
int local = ldloca->sregs [0];
// Replace LDLOCA + LDFLD with LDLOC, when the loading field represents
// the entire local. This is the case with loading the only field of an
// IntPtr. We don't handle value type loads.
ins->opcode = GINT_TO_OPCODE (get_mov_for_type (mt, TRUE));
// The dreg of the MOV is the same as the dreg of the LDFLD
// Allow ldloca instruction to be killed
local_ref_count [sregs [0]]--;
sregs [0] = local;
if (td->locals [local].mt == (ins->opcode - MINT_LDFLD_I1) && ins->data [0] == 0) {
// Replace LDLOCA + LDFLD with LDLOC, when the loading field represents
// the entire local. This is the case with loading the only field of an
// IntPtr. We don't handle value type loads.
ins->opcode = GINT_TO_OPCODE (get_mov_for_type (mt, TRUE));
// The dreg of the MOV is the same as the dreg of the LDFLD
sregs [0] = local;
} else {
// Add mov.src.off to load directly from the local var space without use of ldloca.
int foffset = ins->data [0];
guint16 ldsize = 0;
if (mt == MINT_TYPE_VT)
ldsize = ins->data [1];

// This loads just a part of the local valuetype
ins = interp_insert_ins (td, ins, MINT_MOV_SRC_OFF);
interp_ins_set_dreg (ins, ins->prev->dreg);
interp_ins_set_sreg (ins, local);
ins->data [0] = foffset;
ins->data [1] = mt;
if (mt == MINT_TYPE_VT)
ins->data [2] = ldsize;

interp_clear_ins (ins->prev);
}

if (td->verbose_level) {
g_print ("Replace ldloca/ldfld pair :\n\t");
Expand Down Expand Up @@ -8819,7 +8904,7 @@ interp_cprop (TransformData *td)
needs_retry = TRUE;
} else {
// This loads just a part of the local valuetype
ins = interp_insert_ins (td, ins, MINT_MOV_OFF);
ins = interp_insert_ins (td, ins, MINT_MOV_SRC_OFF);
interp_ins_set_dreg (ins, ins->prev->dreg);
interp_ins_set_sreg (ins, local);
ins->data [0] = 0;
Expand All @@ -8833,19 +8918,58 @@ interp_cprop (TransformData *td)
dump_interp_inst (ins);
}
}
} else if (MINT_IS_STFLD (opcode) && ins->data [0] == 0) {
} else if (MINT_IS_STIND (opcode)) {
InterpInst *ldloca = local_defs [sregs [0]].ins;
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S &&
td->locals [ldloca->sregs [0]].mt == (ins->opcode - MINT_STFLD_I1)) {
int mt = ins->opcode - MINT_STFLD_I1;
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S) {
int local = ldloca->sregs [0];
int mt = td->locals [local].mt;
if (mt != MINT_TYPE_VT) {
// We have an 8 byte local, just replace the stind with a mov
local_ref_count [sregs [0]]--;
// We make the assumption that the STIND matches the local type
ins->opcode = GINT_TO_OPCODE (get_mov_for_type (mt, TRUE));
interp_ins_set_dreg (ins, local);
interp_ins_set_sreg (ins, sregs [1]);

ins->opcode = GINT_TO_OPCODE (get_mov_for_type (mt, FALSE));
// The sreg of the MOV is the same as the second sreg of the STFLD
if (td->verbose_level) {
g_print ("Replace ldloca/stind pair :\n\t");
dump_interp_inst (ins);
}
needs_retry = TRUE;
}
}
} else if (MINT_IS_STFLD (opcode)) {
InterpInst *ldloca = local_defs [sregs [0]].ins;
if (ldloca != NULL && ldloca->opcode == MINT_LDLOCA_S) {
int mt = ins->opcode - MINT_STFLD_I1;
int local = ldloca->sregs [0];
local_ref_count [sregs [0]]--;
ins->dreg = local;
sregs [0] = sregs [1];
// Allow ldloca instruction to be killed
if (td->locals [local].mt == (ins->opcode - MINT_STFLD_I1) && ins->data [0] == 0) {
ins->opcode = GINT_TO_OPCODE (get_mov_for_type (mt, FALSE));
// The sreg of the MOV is the same as the second sreg of the STFLD
ins->dreg = local;
sregs [0] = sregs [1];
} else {
// Add mov.dst.off to store directly int the local var space without use of ldloca.
int foffset = ins->data [0];
guint16 vtsize = 0;
if (mt == MINT_TYPE_VT)
vtsize = ins->data [1];
else
vtsize = get_mint_type_size (mt);

// This stores just to part of the dest valuetype
ins = interp_insert_ins (td, ins, MINT_MOV_DST_OFF);
interp_ins_set_dreg (ins, local);
interp_ins_set_sreg (ins, sregs [1]);
ins->data [0] = foffset;
// Always use MINT_TYPE_VT so we end up doing a memmove with the type size
ins->data [1] = MINT_TYPE_VT;
ins->data [2] = vtsize;

interp_clear_ins (ins->prev);
}
if (td->verbose_level) {
g_print ("Replace ldloca/stfld pair (off %p) :\n\t", (void *)(uintptr_t) ldloca->il_offset);
dump_interp_inst (ins);
Expand Down

0 comments on commit 94d7503

Please sign in to comment.