[mono][interp] Remove dependence on svar/dvar when computing call offset (#81017)
authorVlad Brezae <brezaevlad@gmail.com>
Tue, 24 Jan 2023 14:32:52 +0000 (16:32 +0200)
committerGitHub <noreply@github.com>
Tue, 24 Jan 2023 14:32:52 +0000 (16:32 +0200)
* [mono][interp] Rename field to better reflect its content

* [mono][interp] Fix stack alignment

* [mono][interp] Move assertion to catch misalignment sooner

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

On optimized code, we made sure we always had a svar set, even if the call had no args. We then resolved the offset of the call as the offset of the first arg.
On unoptimized code, we made sure we always had a dvar set, even if the call has void return. We then resolved the offset of the call as the offset of the dvar. As we are introducing alignment guarantees of param area and, in the future, the dvar (only if it is simd type), the offset of the dvar and svar can end up with quite different offsets.

This commit solves these complications by introducing a call_offset field in the call info data, which represents the offset of the call args. This field is set either by the var offset allocator or on the fly, during compilation, in unoptimized case. The dvar offset is allocated independently and when emitting compacted instruction for calls, we do it now in an unified way between optimized and unoptimized compilation.

* Enable assertion temporarily for CI

src/mono/mono/mini/interp/interp.c
src/mono/mono/mini/interp/mintops.def
src/mono/mono/mini/interp/transform.c
src/mono/mono/mini/interp/transform.h

index 4776b6f..61cb384 100644 (file)
@@ -4212,6 +4212,7 @@ call:
                                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,
@@ -4225,7 +4226,6 @@ call:
                        }
 
                        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;
index 0fc232f..b5fdcca 100644 (file)
@@ -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)
index 7f15748..572b6d7 100644 (file)
@@ -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 {
-               // 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);
        }
 }
 
@@ -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 {
+               td->last_ins->info.call_info->call_offset = get_tos_offset (td);
        }
 }
 
@@ -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 {
-               // 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);
        }
 }
 
@@ -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);
@@ -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);
@@ -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 {
-                               // 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);
@@ -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);
 
@@ -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;
                                }
                        }
@@ -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) {
@@ -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 {
@@ -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) {
@@ -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;
                                }
@@ -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;
 }
 
@@ -8300,6 +8311,11 @@ 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));
@@ -8307,16 +8323,7 @@ emit_compacted_instruction (TransformData *td, guint16* start_ip, InterpInst *in
                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]));
@@ -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)) {
@@ -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;
@@ -10744,7 +10738,7 @@ retry:
        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]));
 
index 052b000..207b1ad 100644 (file)
@@ -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;
        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;
        };
 };