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.

Sign up for GitHub

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] Remove dependence on svar/dvar when computing call offset #81017

Merged
merged 5 commits into from
Jan 24, 2023
Merged

[mono][interp] Remove dependence on svar/dvar when computing call offset #81017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e4085d
[mono][interp] Rename field to better reflect its content
BrzVlad Jan 21, 2023
4ca4fe0
[mono][interp] Fix stack alignment
BrzVlad Jan 21, 2023
08f345d
[mono][interp] Move assertion to catch misalignment sooner
BrzVlad Jan 23, 2023
5d66502
[mono][interp] Remove dependence on svar/dvar when computing call offset
BrzVlad Jan 23, 2023
a7522ca
Enable assertion temporarily for CI
BrzVlad Jan 23, 2023
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
2 changes: 1 addition & 1 deletion src/mono/mono/mini/interp/interp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4213,6 +4213,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
reinit_frame (child_frame, frame, cmethod, locals + return_offset, locals + call_args_offset);
frame = child_frame;
}
g_assert (((gsize)frame->stack % MINT_STACK_ALIGNMENT) == 0);

MonoException *call_ex;
if (method_entry (context, frame,
Expand All @@ -4226,7 +4227,6 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
}

context->stack_pointer = (guchar*)frame->stack + cmethod->alloca_size;
g_assert_checked (((gsize)context->stack_pointer % MINT_STACK_ALIGNMENT) == 0);

if (G_UNLIKELY (context->stack_pointer >= context->stack_end)) {
context->stack_end = context->stack_real_end;
Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/mini/interp/mintops.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ OPDEF(MINT_STRLEN, "strlen", 3, 1, 1, MintOpNoArgs)
OPDEF(MINT_ARRAY_RANK, "array_rank", 3, 1, 1, MintOpNoArgs)
OPDEF(MINT_ARRAY_ELEMENT_SIZE, "array_element_size", 3, 1, 1, MintOpNoArgs)

OPDEF(MINT_CALL_ALIGN_STACK, "call_align_stack", 3, 1, 0, MintOpShortInt)
OPDEF(MINT_CALL_ALIGN_STACK, "call_align_stack", 3, 0, 0, MintOpTwoShorts)

/* Calls */
OPDEF(MINT_CALL, "call", 4, 1, 1, MintOpMethodToken)
Expand Down
102 changes: 48 additions & 54 deletions src/mono/mono/mini/interp/transform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1383,9 +1383,7 @@ interp_generate_mae_throw (TransformData *td, MonoMethod *method, MonoMethod *ta
call_args [2] = -1;
td->last_ins->info.call_info->call_args = call_args;
} else {
BrzVlad marked this conversation as resolved.
Show resolved Hide resolved
// Unoptimized code needs every call to have a dreg for offset allocation,
// even if call is void
td->last_ins->dreg = td->sp [0].local;
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
}
}

Expand All @@ -1402,6 +1400,8 @@ interp_generate_void_throw (TransformData *td, MonoJitICallId icall_id)
push_simple_type (td, STACK_TYPE_I4);
td->sp--;
td->last_ins->dreg = td->sp [0].local;
} else {
BrzVlad marked this conversation as resolved.
Show resolved Hide resolved
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
}
}

Expand Down Expand Up @@ -1429,9 +1429,7 @@ interp_generate_ipe_throw_with_msg (TransformData *td, MonoError *error_msg)
call_args [1] = -1;
td->last_ins->info.call_info->call_args = call_args;
} else {
BrzVlad marked this conversation as resolved.
Show resolved Hide resolved
// Unoptimized code needs every call to have a dreg for offset allocation,
// even if call is void
td->last_ins->dreg = td->sp [0].local;
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
}
}

Expand Down Expand Up @@ -2007,6 +2005,8 @@ interp_emit_ldelema (TransformData *td, MonoClass *array_class, MonoClass *check
td->last_ins->data [1] = GINT_TO_UINT16 (size);
init_last_ins_call (td);
td->last_ins->info.call_info->call_args = call_args;
if (!td->optimized)
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
}
} else {
interp_add_ins (td, MINT_LDELEMA_TC);
Expand All @@ -2019,6 +2019,8 @@ interp_emit_ldelema (TransformData *td, MonoClass *array_class, MonoClass *check
td->last_ins->data [0] = get_data_item_index (td, check_class);
init_last_ins_call (td);
td->last_ins->info.call_info->call_args = call_args;
if (!td->optimized)
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
}

push_simple_type (td, STACK_TYPE_MP);
Expand Down Expand Up @@ -3463,10 +3465,8 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
int *call_args = create_call_args (td, num_args);
td->last_ins->info.call_info->call_args = call_args;
} else {
BrzVlad marked this conversation as resolved.
Show resolved Hide resolved
// Dummy dreg
push_simple_type (td, STACK_TYPE_I4);
interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
td->sp--;
// Execution stack is empty once the args are pop'ed
td->last_ins->info.call_info->call_offset = 0;
}

int in_offset = GPTRDIFF_TO_INT (td->ip - td->il_code);
Expand Down Expand Up @@ -3523,6 +3523,8 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
int num_args = csignature->param_count + !!csignature->hasthis;
td->sp -= num_args;
guint32 params_stack_size = get_stack_size (td->sp, num_args);
// Used only by unoptimized code
int param_offset = get_tos_offset (td);

int *call_args = create_call_args (td, num_args);

Expand Down Expand Up @@ -3676,15 +3678,19 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
if (td->last_ins->flags & INTERP_INST_FLAG_CALL) {
td->last_ins->info.call_info->call_args = call_args;
if (!td->optimized) {
int call_dreg = td->last_ins->dreg;
int call_offset = td->locals [call_dreg].stack_offset;
if ((call_offset % MINT_STACK_ALIGNMENT) != 0) {
InterpInst *align_ins = interp_insert_ins_bb (td, td->cbb, interp_prev_ins (td->last_ins), MINT_CALL_ALIGN_STACK);
interp_ins_set_dreg (align_ins, call_dreg);
align_ins->data [0] = params_stack_size;
if ((param_offset % MINT_STACK_ALIGNMENT) == 0) {
td->last_ins->info.call_info->call_offset = param_offset;
} else {
int new_param_offset = ALIGN_TO (param_offset, MINT_STACK_ALIGNMENT);
td->last_ins->info.call_info->call_offset = new_param_offset;
if (params_stack_size) {
InterpInst *align_ins = interp_insert_ins_bb (td, td->cbb, interp_prev_ins (td->last_ins), MINT_CALL_ALIGN_STACK);
align_ins->data [0] = param_offset;
align_ins->data [1] = params_stack_size;
}
if (calli) {
// fp_sreg is at the top of the stack, make sure it is not overwritten by MINT_CALL_ALIGN_STACK
int offset = ALIGN_TO (call_offset, MINT_STACK_ALIGNMENT) - call_offset;
int offset = new_param_offset - param_offset;
td->locals [fp_sreg].stack_offset += offset;
}
}
Expand Down Expand Up @@ -5961,10 +5967,12 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
interp_add_ins (td, MINT_NEWOBJ_ARRAY);
td->last_ins->data [0] = get_data_item_index (td, m->klass);
td->last_ins->data [1] = csignature->param_count;
init_last_ins_call (td);
if (!td->optimized)
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
push_type (td, stack_type [ret_mt], klass);
interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
interp_ins_set_sreg (td->last_ins, MINT_CALL_ARGS_SREG);
init_last_ins_call (td);
td->last_ins->info.call_info->call_args = call_args;
} else if (klass == mono_defaults.string_class) {
if (!td->optimized) {
Expand Down Expand Up @@ -6670,10 +6678,12 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
call_args [0] = td->sp [0].local;
call_args [1] = td->sp [1].local;
call_args [2] = -1;
init_last_ins_call (td);
if (!td->optimized)
td->last_ins->info.call_info->call_offset = get_tos_offset (td);
push_simple_type (td, STACK_TYPE_MP);
interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
td->last_ins->data [0] = get_data_item_index (td, klass);
init_last_ins_call (td);
td->last_ins->info.call_info->call_args = call_args;
interp_ins_set_sreg (td->last_ins, MINT_CALL_ARGS_SREG);
} else {
Expand Down Expand Up @@ -7312,15 +7322,12 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
for (int i = 0; i < info->sig->param_count; i++)
call_args [i] = td->sp [i].local;
call_args [info->sig->param_count] = -1;
int param_offset = get_tos_offset (td);

if (!MONO_TYPE_IS_VOID (info->sig->ret)) {
mt = mint_type (info->sig->ret);
push_simple_type (td, stack_type [mt]);
dreg = td->sp [-1].local;
} else if (!td->optimized) {
// Dummy dreg
push_simple_type (td, stack_type [STACK_TYPE_I4]);
dreg = td->sp [-1].local;
td->sp--;
}

if (jit_icall_id == MONO_JIT_ICALL_mono_threads_attach_coop) {
Expand All @@ -7342,6 +7349,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
td->last_ins->data [0] = get_data_item_index (td, (gpointer)info->func);
init_last_ins_call (td);
td->last_ins->info.call_info->call_args = call_args;
if (!td->optimized)
td->last_ins->info.call_info->call_offset = param_offset;
}
break;
}
Expand Down Expand Up @@ -8067,8 +8076,10 @@ compute_native_offset_estimates (TransformData *td)
}
}

if (!td->optimized)
if (!td->optimized) {
td->total_locals_size = ALIGN_TO (td->total_locals_size, MINT_STACK_ALIGNMENT);
td->param_area_offset = td->total_locals_size;
}
return noe;
}

Expand Down Expand Up @@ -8300,23 +8311,19 @@ emit_compacted_instruction (TransformData *td, guint16* start_ip, InterpInst *in
int num_vars = mono_interp_oplen [opcode] - 1;
for (int i = 0; i < num_vars; i++)
*ip++ = GINT_TO_UINT16 (get_local_offset (td, ins->data [i]));
} else if (opcode == MINT_CALL_ALIGN_STACK) {
g_assert (!td->optimized);
// ins->data [0] represents the stack offset of the call args (within the execution stack)
*ip++ = GINT_TO_UINT16 (td->param_area_offset + ins->data [0]);
*ip++ = GINT_TO_UINT16 (ins->data [1]);
} else {
if (mono_interp_op_dregs [opcode])
*ip++ = GINT_TO_UINT16 (get_local_offset (td, ins->dreg));

if (mono_interp_op_sregs [opcode]) {
for (int i = 0; i < mono_interp_op_sregs [opcode]; i++) {
if (ins->sregs [i] == MINT_CALL_ARGS_SREG) {
int offset;
// In the unoptimized case the return and the start of the param area are always at the
// same offset. Use the dreg offset so we don't need to rely on existing call_args.
if (td->optimized)
offset = get_local_offset (td, ins->info.call_info->call_args [0]);
else if (opcode == MINT_NEWOBJ_ARRAY || opcode == MINT_LDELEMA_TC || opcode == MINT_LDELEMA)
// no alignment required since this is not a real call
offset = get_local_offset (td, ins->dreg);
else
offset = ALIGN_TO (get_local_offset (td, ins->dreg), MINT_STACK_ALIGNMENT);
int offset = td->param_area_offset + ins->info.call_info->call_offset;
*ip++ = GINT_TO_UINT16 (offset);
} else {
*ip++ = GINT_TO_UINT16 (get_local_offset (td, ins->sregs [i]));
Expand Down Expand Up @@ -10228,10 +10235,11 @@ end_active_call (TransformData *td, ActiveCalls *ac, InterpInst *call)
// Given we iterate over the list of deferred calls from the last to the first one to end, all deps of a call are guaranteed to have been processed at this point.
int base_offset = 0;
for (GSList *list = deferred_call->info.call_info->call_deps; list; list = list->next) {
int call_offset = ((InterpInst*)list->data)->info.call_info->call_offset;
if (call_offset > base_offset)
base_offset = call_offset;
int end_offset = ((InterpInst*)list->data)->info.call_info->call_end_offset;
if (end_offset > base_offset)
base_offset = end_offset;
}
deferred_call->info.call_info->call_offset = base_offset;
// Compute to offset of each call argument
int *call_args = deferred_call->info.call_info->call_args;
if (call_args && (*call_args != -1)) {
Expand All @@ -10241,22 +10249,8 @@ end_active_call (TransformData *td, ActiveCalls *ac, InterpInst *call)
call_args++;
var = *call_args;
}
} else {
// This call has no argument. Allocate a dummy one so when we resolve the
// offset for MINT_CALL_ARGS_SREG during compacted instruction emit, we can
// always use the offset of the first var in the call_args array
int new_var = create_interp_local (td, mono_get_int_type ());
td->locals [new_var].call = deferred_call;
td->locals [new_var].flags |= INTERP_LOCAL_FLAG_CALL_ARGS;
alloc_var_offset (td, new_var, &base_offset);

call_args = (int*)mono_mempool_alloc (td->mempool, 3 * sizeof (int));
call_args [0] = new_var;
call_args [1] = -1;

deferred_call->info.call_info->call_args = call_args;
}
deferred_call->info.call_info->call_offset = base_offset;
deferred_call->info.call_info->call_end_offset = ALIGN_TO (base_offset, MINT_STACK_ALIGNMENT);

if (ac->deferred_calls) {
deferred_call = (InterpInst*) ac->deferred_calls->data;
Expand Down Expand Up @@ -10744,7 +10738,7 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
g_assert (!td->optimized || !td->max_stack_size);
rtm->alloca_size = td->total_locals_size + td->max_stack_size;
g_assert ((rtm->alloca_size % MINT_STACK_ALIGNMENT) == 0);
rtm->locals_size = td->optimized ? td->param_area_offset : td->total_locals_size;
rtm->locals_size = td->param_area_offset;
rtm->data_items = (gpointer*)mono_mem_manager_alloc0 (td->mem_manager, td->n_data_items * sizeof (td->data_items [0]));
memcpy (rtm->data_items, td->data_items, td->n_data_items * sizeof (td->data_items [0]));

Expand Down
5 changes: 3 additions & 2 deletions src/mono/mono/mini/interp/transform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,12 @@ struct _InterpCallInfo {
// in the order they are pushed to the stack. This makes it easy to find
// all source vars for these types of opcodes. This is terminated with -1.
int *call_args;
int call_offset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call_end_offset has become temporary variable needed only during the offset allocation and it might be removed from the call_info struct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added the call_end_offset field so we can easily map between call InterpInst to allocated offset, without use of other data structures and lookups. Did you have an alternative for this ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without other data structures, no. It can be inferred from base_offset + call_args but it is probably inefficient.

union {
// Array of call dependencies that need to be resolved before
GSList *call_deps;
// Stack call offset with call arguments
int call_offset;
// Stack end offset of call arguments
int call_end_offset;
};
};

Expand Down