From b9dd31ee843b86bc1abda7192105264440f5f711 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 16 Mar 2023 13:35:33 -0700 Subject: [PATCH 1/4] Allow a single call when inlining methods, this makes it theoretically possible to inline List.get_Item --- src/mono/mono/mini/interp/transform.c | 18 +++++++++++++++--- src/mono/mono/mini/interp/transform.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index db02d71bf0171..b2855f6e0daf2 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3440,9 +3440,21 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target } } - /* Don't inline methods that do calls */ - if (op == -1 && td->inlined_method && !td->aggressive_inlining) - return FALSE; + /* + * When inlining a method, only allow it to perform one call. + * We previously prohibited calls entirely, this is a conservative rule that allows + * Things like ThrowHelper.ThrowIfNull to theoretically inline, along with a hypothetical + * X.get_Item that just calls Y.get_Item (profitable to inline) + */ + if (op == -1 && td->inlined_method && !td->aggressive_inlining) { + if (td->has_inlined_one_call) { + g_print("Prohibiting second inlined call in %s (target %s)\n", td->method->name, target_method->name); + return FALSE; + } else { + g_print("Allowing single inlined call in %s (target %s)\n", td->method->name, target_method->name); + td->has_inlined_one_call = TRUE; + } + } /* We need to convert delegate invoke to a indirect call on the interp_invoke_impl field */ if (target_method && m_class_get_parent (target_method->klass) == mono_defaults.multicastdelegate_class) { diff --git a/src/mono/mono/mini/interp/transform.h b/src/mono/mono/mini/interp/transform.h index 362f8c9afafb0..7b3e8cf1f2d76 100644 --- a/src/mono/mono/mini/interp/transform.h +++ b/src/mono/mono/mini/interp/transform.h @@ -265,6 +265,7 @@ typedef struct int aggressive_inlining : 1; int optimized : 1; int has_invalid_code : 1; + int has_inlined_one_call : 1; } TransformData; #define STACK_TYPE_I4 0 From 5a820d82e4cfbc6d89a57b17520c001983dcc4ce Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 16 Mar 2023 13:37:15 -0700 Subject: [PATCH 2/4] Disable logging by default --- src/mono/mono/mini/interp/transform.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index b2855f6e0daf2..763fab980c75f 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3448,10 +3448,12 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target */ if (op == -1 && td->inlined_method && !td->aggressive_inlining) { if (td->has_inlined_one_call) { - g_print("Prohibiting second inlined call in %s (target %s)\n", td->method->name, target_method->name); + if (td->verbose_level > 0) + g_print("Prohibiting second inlined call in %s (target %s)\n", td->method->name, target_method->name); return FALSE; } else { - g_print("Allowing single inlined call in %s (target %s)\n", td->method->name, target_method->name); + if (td->verbose_level > 2) + g_print("Allowing single inlined call in %s (target %s)\n", td->method->name, target_method->name); td->has_inlined_one_call = TRUE; } } From 446cb8165e8f6beff2ec15c25789f77a69b8e5dc Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 16 Mar 2023 14:34:27 -0700 Subject: [PATCH 3/4] Properly save/restore the 'has inlined a call' flag when inlining methods Don't allow doesnotreturn method calls to disable inlining, since we know they are unlikely to be called --- src/mono/mono/mini/interp/transform.c | 31 +++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 763fab980c75f..6b83da65c7ac0 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -59,6 +59,7 @@ static int stack_type [] = { }; static GENERATE_TRY_GET_CLASS_WITH_CACHE (intrinsic_klass, "System.Runtime.CompilerServices", "IntrinsicAttribute") +static GENERATE_TRY_GET_CLASS_WITH_CACHE (doesnotreturn_klass, "System.Diagnostics.CodeAnalysis", "DoesNotReturnAttribute") static gboolean generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, MonoGenericContext *generic_context, MonoError *error); @@ -77,6 +78,21 @@ has_intrinsic_attribute (MonoMethod *method) return result; } +static gboolean +has_doesnotreturn_attribute (MonoMethod *method) +{ + gboolean result = FALSE; + ERROR_DECL (aerror); + MonoClass *doesnotreturn_klass = mono_class_try_get_doesnotreturn_klass_class (); + MonoCustomAttrInfo *ainfo = mono_custom_attrs_from_method_checked (method, aerror); + mono_error_cleanup (aerror); /* FIXME don't swallow the error? */ + if (ainfo) { + result = doesnotreturn_klass && mono_custom_attrs_has_attr (ainfo, doesnotreturn_klass); + mono_custom_attrs_free (ainfo); + } + return result; +} + static InterpInst* interp_new_ins (TransformData *td, int opcode, int len) { @@ -2863,6 +2879,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe int i; int prev_sp_offset; int prev_aggressive_inlining; + int prev_has_inlined_one_call; MonoGenericContext *generic_context = NULL; StackInfo *prev_param_area; InterpBasicBlock **prev_offset_to_bb; @@ -2892,6 +2909,8 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe prev_entry_bb = td->entry_bb; prev_aggressive_inlining = td->aggressive_inlining; prev_imethod_items = td->imethod_items; + prev_has_inlined_one_call = td->has_inlined_one_call; + td->has_inlined_one_call = FALSE; td->inlined_method = target_method; prev_max_stack_height = td->max_stack_height; @@ -2973,6 +2992,7 @@ interp_inline_method (TransformData *td, MonoMethod *target_method, MonoMethodHe td->code_size = prev_code_size; td->entry_bb = prev_entry_bb; td->aggressive_inlining = prev_aggressive_inlining; + td->has_inlined_one_call = prev_has_inlined_one_call; g_free (td->in_offsets); td->in_offsets = prev_in_offsets; @@ -3447,8 +3467,15 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target * X.get_Item that just calls Y.get_Item (profitable to inline) */ if (op == -1 && td->inlined_method && !td->aggressive_inlining) { - if (td->has_inlined_one_call) { - if (td->verbose_level > 0) + if (has_doesnotreturn_attribute(target_method)) { + /* + * Since the method does not return, it's probably a throw helper and will not be called. + * As such we don't want it to prevent inlining of the method that calls it. + */ + if (td->verbose_level > 2) + g_print("Overlooking inlined doesnotreturn call in %s (target %s)\n", td->method->name, target_method->name); + } else if (td->has_inlined_one_call) { + if (td->verbose_level > 1) g_print("Prohibiting second inlined call in %s (target %s)\n", td->method->name, target_method->name); return FALSE; } else { From 726c38343132fe680e49d27abef00a3b42d00584 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 16 Mar 2023 16:27:32 -0700 Subject: [PATCH 4/4] Fix crash in inlining due to attribute check --- src/mono/mono/mini/interp/transform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 6b83da65c7ac0..5d768d76dbbeb 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3467,7 +3467,11 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target * X.get_Item that just calls Y.get_Item (profitable to inline) */ if (op == -1 && td->inlined_method && !td->aggressive_inlining) { - if (has_doesnotreturn_attribute(target_method)) { + if (!target_method) { + if (td->verbose_level > 1) + g_print("Disabling inlining because we have no target_method for call in %s\n", td->method->name); + return FALSE; + } else if (has_doesnotreturn_attribute(target_method)) { /* * Since the method does not return, it's probably a throw helper and will not be called. * As such we don't want it to prevent inlining of the method that calls it.