From f440154e9e8682129e612a3b95917f0fd9edb65a Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 15 Jul 2025 19:55:48 +0300 Subject: [PATCH 1/3] [mono][aot] Prevent localloc in a loop during constrained gsharedvt calls We create a var that stores the address of some localloc memory. This var is nulled at method entry. In every place where we need to obtain temporary localloc memory, we check if the cache var was initialized, if not we do a localloc, otherwise we use the cached ptr as the temporary memory buffer. This commit adds 2 such caches because the constrained gsharedvt call can require 2 separate temporary buffers. --- src/mono/mono/mini/method-to-ir.c | 66 ++++++++++++++++++++++++++----- src/mono/mono/mini/mini.h | 8 ++++ 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 91e7b446a5316a..a320e8cc5652c9 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -3732,6 +3732,44 @@ handle_delegate_ctor (MonoCompile *cfg, MonoClass *klass, MonoInst *target, Mono return obj; } +static MonoInst* +mono_emit_cached_localloc (MonoCompile *cfg, int cache_index, int new_size) +{ + MonoCachedLocallocInfo *info = &cfg->localloc_cache [cache_index]; + + // Create var or update the size. All locallocs will be patched to the max size after IR code emit ends. + if (info->alloc_size == 0) { + info->addr_var = mono_compile_create_var (cfg, mono_get_int_type (), OP_LOCAL); + info->alloc_size = new_size; + } else if (info->alloc_size < new_size) { + info->alloc_size = new_size; + } + + int cache_dreg = info->addr_var->dreg; + MonoInst* ins; + MonoBasicBlock *done_bb; + + NEW_BBLOCK (cfg, done_bb); + + MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, cache_dreg, 0); + MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBNE_UN, done_bb); + + // If we have no localloc-ed memory, allocate it now and save it in the cache + MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM); + ins->dreg = alloc_preg (cfg); + ins->inst_imm = new_size; + MONO_ADD_INS (cfg->cbb, ins); + + info->localloc_ins = g_slist_append (info->localloc_ins, ins); + + MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cache_dreg, ins->dreg); + + // Return the value from the cache + MONO_START_BB (cfg, done_bb); + EMIT_NEW_UNALU (cfg, ins, OP_MOVE, alloc_preg (cfg), cache_dreg); + return ins; +} + /* * handle_constrained_gsharedvt_call: * @@ -3865,20 +3903,12 @@ handle_constrained_gsharedvt_call (MonoCompile *cfg, MonoMethod *cmethod, MonoMe /* Pass an array of bools which signal whenever the corresponding argument is a gsharedvt ref type */ if (has_gsharedvt) { - MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM); - ins->dreg = alloc_preg (cfg); - ins->inst_imm = fsig->param_count; - MONO_ADD_INS (cfg->cbb, ins); - is_gsharedvt_ins = ins; + is_gsharedvt_ins = mono_emit_cached_localloc (cfg, 0, fsig->param_count); } else { EMIT_NEW_PCONST (cfg, is_gsharedvt_ins, 0); } /* Pass the arguments using a localloc-ed array using the format expected by runtime_invoke () */ - MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM); - ins->dreg = alloc_preg (cfg); - ins->inst_imm = fsig->param_count * sizeof (target_mgreg_t); - MONO_ADD_INS (cfg->cbb, ins); - args_ins = ins; + args_ins = mono_emit_cached_localloc (cfg, 1, fsig->param_count * sizeof (target_mgreg_t)); for (int i = 0; i < fsig->param_count; ++i) { int addr_reg; @@ -12335,8 +12365,24 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b cfg->cbb = init_localsbb; emit_init_local (cfg, i, header->locals [i], init_locals); } + + // Patch all locallocs using the cache to allocate the max used size. + for (int i = 0; i < 2; i++) { + MonoCachedLocallocInfo *info = &cfg->localloc_cache [i]; + if (info->alloc_size != 0) { + MONO_EMIT_NEW_PCONST (cfg, info->addr_var->dreg, NULL); + GSList *p = info->localloc_ins; + while (p != NULL) { + MonoInst *localloc_ins = (MonoInst*)p->data; + localloc_ins->inst_imm = info->alloc_size; + p = p->next; + } + g_slist_free (info->localloc_ins); + } + } } + if (cfg->init_ref_vars && cfg->method == method) { /* Emit initialization for ref vars */ // FIXME: Avoid duplication initialization for IL locals. diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 26ebd7f90fdf6b..30e5f5c4728811 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -1306,6 +1306,12 @@ typedef enum { #define vreg_is_ref(cfg, vreg) (GINT_TO_UINT32(vreg) < (cfg)->vreg_is_ref_len ? (cfg)->vreg_is_ref [(vreg)] : 0) #define vreg_is_mp(cfg, vreg) (GINT_TO_UINT32(vreg) < (cfg)->vreg_is_mp_len ? (cfg)->vreg_is_mp [(vreg)] : 0) +typedef struct { + MonoInst* addr_var; + int alloc_size; + GSList* localloc_ins; +} MonoCachedLocallocInfo; + /* * Control Flow Graph and compilation unit information */ @@ -1661,6 +1667,8 @@ typedef struct { gboolean *clause_is_dead; + MonoCachedLocallocInfo localloc_cache [2]; + /* Stats */ int stat_allocate_var; int stat_locals_stack_size; From 88a9aeeb510123aab97d3f008dc1b8bf769700dc Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 15 Jul 2025 22:42:06 +0300 Subject: [PATCH 2/3] Re-enable test --- .../tests/System.Runtime.Tests/System/StringTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs index 990e55f1d09a5c..4d7258fb4c0985 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs @@ -993,7 +993,6 @@ public static IEnumerable NonRandomizedGetHashCode_EquivalentForString [Theory] [MemberData(nameof(NonRandomizedGetHashCode_EquivalentForStringAndSpan_MemberData))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/116815", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)] public static void NonRandomizedGetHashCode_EquivalentForStringAndSpan(int charValueLimit, bool ignoreCase) { // This is testing internal API. If that API changes, this test will need to be updated. From d385aec8675e8e64e2899787fb3b6c9afc7f3013 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 21 Jul 2025 09:33:57 +0300 Subject: [PATCH 3/3] milos review --- src/mono/mono/mini/method-to-ir.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index a320e8cc5652c9..f4e661edd4eea2 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -3735,6 +3735,7 @@ handle_delegate_ctor (MonoCompile *cfg, MonoClass *klass, MonoInst *target, Mono static MonoInst* mono_emit_cached_localloc (MonoCompile *cfg, int cache_index, int new_size) { + g_assert (cache_index < (int)G_N_ELEMENTS (cfg->localloc_cache)); MonoCachedLocallocInfo *info = &cfg->localloc_cache [cache_index]; // Create var or update the size. All locallocs will be patched to the max size after IR code emit ends. @@ -12378,6 +12379,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b p = p->next; } g_slist_free (info->localloc_ins); + info->localloc_ins = NULL; } } }