From 90f401348360d607892a39e188559312fd031e74 Mon Sep 17 00:00:00 2001 From: Jay Krell Date: Tue, 18 Feb 2020 16:17:14 -0800 Subject: [PATCH] [interp] Make newobj_vt[st]_fast non-recursive. (#18902) * [interp] Make newobj_vt_fast non-recursive. * PR feedback: Attempt to provide similer primitive increase_max_stack_height that is not so roundabout as move_stack (n) + move_stack (-n). * [interp] Make newobj_vtst_fast not recursive. This is builds upon https://github.com/mono/mono/pull/18875 and is easiest reviewed after it is merged. Or, perhaps, this replaces it. * comment-smithing --- mono/mini/interp/interp.c | 86 ++++++++++++-------------------------------- mono/mini/interp/transform.c | 12 +++++++ 2 files changed, 35 insertions(+), 63 deletions(-) diff --git a/mono/mini/interp/interp.c b/mono/mini/interp/interp.c index ed6d780bc58..d48451ff1be 100644 --- a/mono/mini/interp/interp.c +++ b/mono/mini/interp/interp.c @@ -301,14 +301,6 @@ static gboolean interp_init_done = FALSE; static void interp_exec_method_full (InterpFrame *frame, ThreadContext *context, FrameClauseArgs *clause_args, MonoError *error); -static MONO_NEVER_INLINE void -interp_exec_method_newobj_vtst_fast (InterpFrame *frame, ThreadContext *context, MonoError *error) -// This function makes WebAsssembly stacks clearer, so you can see which recursion -// is occuring, in the absence of line numbers in the debugger. -{ - interp_exec_method_full (frame, context, NULL, error); -} - static InterpMethod* lookup_method_pointer (gpointer addr); typedef void (*ICallMethod) (InterpFrame *frame); @@ -3223,36 +3215,6 @@ mono_interp_leave (InterpFrame* child_frame) return (MonoException*)tmp_sp.data.p; } -static MONO_NEVER_INLINE void -mono_interp_newobj_vt ( - // Parameters are sorted by name and parameter list is minimized - // to reduce stack use in caller, on e.g. NT/AMD64 (up to 4 parameters - // use no stack in caller). - InterpFrame* child_frame, - ThreadContext* context, - MonoError* error) -{ - stackval* const sp = child_frame->stack_args; - - stackval valuetype_this; - - memset (&valuetype_this, 0, sizeof (stackval)); - sp->data.p = &valuetype_this; - - // FIXME remove recursion - // - // FIXME It is unfortunate to outline a recursive case as it - // increases its stack usage. We do this however as it conserves - // stack for all the other recursive cases. - interp_exec_method (child_frame, context, error); - - CHECK_RESUME_STATE (context); - - *sp = valuetype_this; -resume: - ; -} - static MONO_NEVER_INLINE MonoException* mono_interp_newobj ( // Parameters are sorted by name and parameter list is minimized @@ -5079,51 +5041,49 @@ call:; } else { cmethod = (InterpMethod*)frame->imethod->data_items [imethod_index]; frame->ip = ip - 4; - is_void = TRUE; - retval = NULL; - ++sp; // Point sp at this, after return value. - goto call; + goto call_newobj; } MINT_IN_BREAK; } + MINT_IN_CASE(MINT_NEWOBJ_VT_FAST) MINT_IN_CASE(MINT_NEWOBJ_VTST_FAST) { - int dummy; - // This is split up to: - // - conserve stack - // - keep exception handling and resume mostly in the main function frame->ip = ip; - child_frame = alloc_frame (context, &dummy, frame, (InterpMethod*) frame->imethod->data_items [ip [1]], NULL, NULL); + cmethod = (InterpMethod*)frame->imethod->data_items [ip [1]]; guint16 const param_count = ip [2]; + // Make room for extra parameter and result. if (param_count) { sp -= param_count; - memmove (sp + 1, sp, param_count * sizeof (stackval)); + memmove (sp + 2, sp, param_count * sizeof (stackval)); } - child_frame->stack_args = sp; + gboolean const vtst = *ip == MINT_NEWOBJ_VTST_FAST; if (vtst) { memset (vt_sp, 0, ip [3]); - sp->data.p = vt_sp; ip += 4; - - // FIXME remove recursion - interp_exec_method_newobj_vtst_fast (child_frame, context, error); - - CHECK_RESUME_STATE (context); - sp->data.p = vt_sp; - + // Put extra parameter and result on stack, before other parameters, + // and point stack to extra parameter, after result. + // This pattern occurs for newobj_vt_fast and newobj_fast. + sp [1].data.p = vt_sp; + sp [0].data.p = vt_sp; } else { ip += 3; - // FIXME remove recursion - mono_interp_newobj_vt (child_frame, context, error); - CHECK_RESUME_STATE (context); + // Like newobj_fast, add valuetype_this parameter + // and result and point stack to this after result. + memset (sp, 0, sizeof (*sp)); + sp [1].data.p = &sp [0].data; // valuetype_this == result } - ++sp; - MINT_IN_BREAK; - } + } + // call_newobj captures the pattern where the return value is placed + // on the stack before the call, instead of the call forming it. +call_newobj: + ++sp; // Point sp at added extra param, after return value. + is_void = TRUE; + retval = NULL; + goto call; MINT_IN_CASE(MINT_NEWOBJ) { int dummy; diff --git a/mono/mini/interp/transform.c b/mono/mini/interp/transform.c index 5f2075d9b47..75fd22ff3ee 100644 --- a/mono/mini/interp/transform.c +++ b/mono/mini/interp/transform.c @@ -537,6 +537,15 @@ move_stack (TransformData *td, int start, int amount) memmove (td->stack + start + amount, td->stack + start, to_move * sizeof (StackInfo)); } +static void +simulate_runtime_stack_increase (TransformData *td, int amount) +{ + const int sp_height = td->sp - td->stack + amount; + + if (sp_height > td->max_stack_height) + td->max_stack_height = sp_height; +} + #define PUSH_VT(td, size) \ do { \ (td)->vt_sp += ALIGN_TO ((size), MINT_VT_ALIGNMENT); \ @@ -4537,6 +4546,9 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, // Set the method to be executed as part of newobj instruction newobj_fast->data [0] = get_data_item_index (td, mono_interp_get_imethod (domain, m, error)); } else { + // Runtime (interp_exec_method_full in interp.c) inserts extra stack to hold return value, before call. + simulate_runtime_stack_increase (td, 1); + if (mint_type (m_class_get_byval_arg (klass)) == MINT_TYPE_VT) interp_add_ins (td, MINT_NEWOBJ_VTST_FAST); else -- 2.11.4.GIT