From 52f454eaa418832c4383743c0e095412a709ead7 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Thu, 22 Sep 2022 15:22:07 -0400 Subject: [PATCH] [mono][aot] Prefer specific instances instead of gshared for methods containing static virtual calls. These calls cannot be resolved at compile time in gshared methods, so they cannot be inlined etc. They are used in perf sensitive BCL code like SpanHelpers. To fix this, modify the AOT compiler so in addition to the gshared versions, it emits specific instances of these methods if possible. This only affects a small subset of gshared methods so it doesn't lead to a noticable code size increase. Fixes https://github.com/dotnet/runtime/issues/75801. --- src/mono/mono/mini/aot-compiler.c | 62 +++++++++++++++++++++++++++++-- src/mono/mono/mini/method-to-ir.c | 10 +++-- src/mono/mono/mini/mini.h | 1 + 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 39d73d02e46835..c79ba81ec77ae9 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -396,6 +396,10 @@ typedef struct MonoAotCompile { GList *profile_data; GHashTable *profile_methods; GHashTable *blob_hash; + /* Maps MonoMethod*->GPtrArray* */ + GHashTable *gshared_instances; + /* Hash of gshared methods where specific instances are preferred */ + GHashTable *prefer_instances; #ifdef EMIT_WIN32_UNWIND_INFO GList *unwind_info_section_cache; #endif @@ -4302,8 +4306,23 @@ mono_dedup_cache_method (MonoAotCompile *acfg, MonoMethod *method) g_hash_table_insert (acfg->dedup_stats, stats_name, GUINT_TO_POINTER (count)); } +static gboolean +is_open_method (MonoMethod *method) +{ + MonoGenericContext *context; + + if (!method->is_inflated) + return FALSE; + context = mono_method_get_context (method); + if (context->class_inst && context->class_inst->is_open) + return TRUE; + if (context->method_inst && context->method_inst->is_open) + return TRUE; + return FALSE; +} + static void -add_extra_method_with_depth (MonoAotCompile *acfg, MonoMethod *method, int depth) +add_extra_method_full (MonoAotCompile *acfg, MonoMethod *method, gboolean prefer_gshared, int depth) { ERROR_DECL (error); @@ -4315,7 +4334,7 @@ add_extra_method_with_depth (MonoAotCompile *acfg, MonoMethod *method, int depth return; } - if (mono_method_is_generic_sharable_full (method, TRUE, TRUE, FALSE)) { + if (prefer_gshared && mono_method_is_generic_sharable_full (method, TRUE, TRUE, FALSE)) { MonoMethod *orig = method; method = mini_get_shared_method_full (method, SHARE_MODE_NONE, error); @@ -4325,10 +4344,23 @@ add_extra_method_with_depth (MonoAotCompile *acfg, MonoMethod *method, int depth return; } + if (g_hash_table_lookup (acfg->prefer_instances, method)) + /* Compile an instance as well */ + add_extra_method_full (acfg, orig, FALSE, depth); + /* Add it to profile_methods so its not skipped later */ if (acfg->aot_opts.profile_only && g_hash_table_lookup (acfg->profile_methods, orig)) g_hash_table_insert (acfg->profile_methods, method, method); - } else if ((acfg->jit_opts & MONO_OPT_GSHAREDVT) && prefer_gsharedvt_method (acfg, method) && mono_method_is_generic_sharable_full (method, FALSE, FALSE, TRUE)) { + + if (!is_open_method (orig) && !mono_method_is_generic_sharable_full (orig, TRUE, FALSE, FALSE)) { + GPtrArray *instances = g_hash_table_lookup (acfg->gshared_instances, method); + if (!instances) { + instances = g_ptr_array_new (); + g_hash_table_insert (acfg->gshared_instances, method, instances); + } + g_ptr_array_add (instances, orig); + } + } else if ((acfg->jit_opts & MONO_OPT_GSHAREDVT) && prefer_gshared && prefer_gsharedvt_method (acfg, method) && mono_method_is_generic_sharable_full (method, FALSE, FALSE, TRUE)) { /* Use the gsharedvt version */ method = mini_get_shared_method_full (method, SHARE_MODE_GSHAREDVT, error); mono_error_assert_ok (error); @@ -4347,6 +4379,12 @@ add_extra_method_with_depth (MonoAotCompile *acfg, MonoMethod *method, int depth add_method_full (acfg, method, TRUE, depth); } +static void +add_extra_method_with_depth (MonoAotCompile *acfg, MonoMethod *method, int depth) +{ + add_extra_method_full (acfg, method, TRUE, depth); +} + static void add_extra_method (MonoAotCompile *acfg, MonoMethod *method) { @@ -9209,6 +9247,22 @@ compile_method (MonoAotCompile *acfg, MonoMethod *method) printf ("%s ### %d\n", mono_method_get_full_name (method), cfg->code_size); } + if (cfg->prefer_instances) { + /* + * Compile the original specific instances in addition to the gshared method + * for performance reasons, since gshared methods cannot implement some + * features like static virtual methods efficiently. + */ + g_hash_table_insert (acfg->prefer_instances, method, method); + GPtrArray *instances = g_hash_table_lookup (acfg->gshared_instances, method); + if (instances) { + for (guint i = 0; i < instances->len; ++i) { + MonoMethod *instance = (MonoMethod*)g_ptr_array_index (instances, i); + add_extra_method_full (acfg, instance, FALSE, 0); + } + } + } + /* Adds generic instances referenced by this method */ /* * The depth is used to avoid infinite loops when generic virtual recursion is @@ -13631,6 +13685,8 @@ acfg_create (MonoAssembly *ass, guint32 jit_opts) acfg->gsharedvt_in_signatures = g_hash_table_new ((GHashFunc)mono_signature_hash, (GEqualFunc)mono_metadata_signature_equal); acfg->gsharedvt_out_signatures = g_hash_table_new ((GHashFunc)mono_signature_hash, (GEqualFunc)mono_metadata_signature_equal); acfg->profile_methods = g_hash_table_new (NULL, NULL); + acfg->gshared_instances = g_hash_table_new (NULL, NULL); + acfg->prefer_instances = g_hash_table_new (NULL, NULL); mono_os_mutex_init_recursive (&acfg->mutex); init_got_info (&acfg->got_info); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 12fa5bbf1a4505..85c1d453ca2c72 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7493,10 +7493,14 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (constrained_class) { if (m_method_is_static (cil_method) && mini_class_check_context_used (cfg, constrained_class)) { /* get_constrained_method () doesn't work on the gparams used by generic sharing */ - // FIXME: Other configurations - //if (!cfg->gsharedvt) - // GENERIC_SHARING_FAILURE (CEE_CALL); gshared_static_virtual = TRUE; + if (!cfg->gsharedvt) + /* + * We can't resolve these calls at compile time, and they are used in + * perf-sensitive code in the BCL, so ask the AOT compiler to try to use specific instances + * instead of this gshared method. + */ + cfg->prefer_instances = TRUE; } else { cmethod = get_constrained_method (cfg, image, token, cil_method, constrained_class, generic_context); CHECK_CFG_ERROR; diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 3d02bd18a0230c..5a6a061b9ffbb0 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -1492,6 +1492,7 @@ typedef struct { guint after_method_to_ir : 1; guint disable_inline_rgctx_fetch : 1; guint deopt : 1; + guint prefer_instances : 1; guint8 uses_simd_intrinsics; int r4_stack_type; gpointer debug_info;