From c7c6aa032175f4ee48c38757d1d9f32df25f02c6 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Tue, 27 Sep 2022 11:44:40 -0400 Subject: [PATCH] [mono][aot] Prefer specific instances instead of gshared for methods containing static virtual calls. (#76033) 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 39d73d02e4683..c79ba81ec77ae 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 12fa5bbf1a450..85c1d453ca2c7 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 3d02bd18a0230..5a6a061b9ffbb 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;