Skip to content

Commit a791373

Browse files
authored
[mono][aot] Prevent localloc in a loop during constrained gsharedvt calls (dotnet#117679)
* [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. * Re-enable test * milos review
1 parent 6e14d81 commit a791373

File tree

3 files changed

+66
-11
lines changed

3 files changed

+66
-11
lines changed

src/libraries/System.Runtime/tests/System.Runtime.Tests/System/StringTests.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,6 @@ public static IEnumerable<object[]> NonRandomizedGetHashCode_EquivalentForString
993993

994994
[Theory]
995995
[MemberData(nameof(NonRandomizedGetHashCode_EquivalentForStringAndSpan_MemberData))]
996-
[ActiveIssue("https://github.com/dotnet/runtime/issues/116815", TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst)]
997996
public static void NonRandomizedGetHashCode_EquivalentForStringAndSpan(int charValueLimit, bool ignoreCase)
998997
{
999998
// This is testing internal API. If that API changes, this test will need to be updated.

src/mono/mono/mini/method-to-ir.c

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3732,6 +3732,45 @@ handle_delegate_ctor (MonoCompile *cfg, MonoClass *klass, MonoInst *target, Mono
37323732
return obj;
37333733
}
37343734

3735+
static MonoInst*
3736+
mono_emit_cached_localloc (MonoCompile *cfg, int cache_index, int new_size)
3737+
{
3738+
g_assert (cache_index < (int)G_N_ELEMENTS (cfg->localloc_cache));
3739+
MonoCachedLocallocInfo *info = &cfg->localloc_cache [cache_index];
3740+
3741+
// Create var or update the size. All locallocs will be patched to the max size after IR code emit ends.
3742+
if (info->alloc_size == 0) {
3743+
info->addr_var = mono_compile_create_var (cfg, mono_get_int_type (), OP_LOCAL);
3744+
info->alloc_size = new_size;
3745+
} else if (info->alloc_size < new_size) {
3746+
info->alloc_size = new_size;
3747+
}
3748+
3749+
int cache_dreg = info->addr_var->dreg;
3750+
MonoInst* ins;
3751+
MonoBasicBlock *done_bb;
3752+
3753+
NEW_BBLOCK (cfg, done_bb);
3754+
3755+
MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, cache_dreg, 0);
3756+
MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBNE_UN, done_bb);
3757+
3758+
// If we have no localloc-ed memory, allocate it now and save it in the cache
3759+
MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM);
3760+
ins->dreg = alloc_preg (cfg);
3761+
ins->inst_imm = new_size;
3762+
MONO_ADD_INS (cfg->cbb, ins);
3763+
3764+
info->localloc_ins = g_slist_append (info->localloc_ins, ins);
3765+
3766+
MONO_EMIT_NEW_UNALU (cfg, OP_MOVE, cache_dreg, ins->dreg);
3767+
3768+
// Return the value from the cache
3769+
MONO_START_BB (cfg, done_bb);
3770+
EMIT_NEW_UNALU (cfg, ins, OP_MOVE, alloc_preg (cfg), cache_dreg);
3771+
return ins;
3772+
}
3773+
37353774
/*
37363775
* handle_constrained_gsharedvt_call:
37373776
*
@@ -3865,20 +3904,12 @@ handle_constrained_gsharedvt_call (MonoCompile *cfg, MonoMethod *cmethod, MonoMe
38653904

38663905
/* Pass an array of bools which signal whenever the corresponding argument is a gsharedvt ref type */
38673906
if (has_gsharedvt) {
3868-
MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM);
3869-
ins->dreg = alloc_preg (cfg);
3870-
ins->inst_imm = fsig->param_count;
3871-
MONO_ADD_INS (cfg->cbb, ins);
3872-
is_gsharedvt_ins = ins;
3907+
is_gsharedvt_ins = mono_emit_cached_localloc (cfg, 0, fsig->param_count);
38733908
} else {
38743909
EMIT_NEW_PCONST (cfg, is_gsharedvt_ins, 0);
38753910
}
38763911
/* Pass the arguments using a localloc-ed array using the format expected by runtime_invoke () */
3877-
MONO_INST_NEW (cfg, ins, OP_LOCALLOC_IMM);
3878-
ins->dreg = alloc_preg (cfg);
3879-
ins->inst_imm = fsig->param_count * sizeof (target_mgreg_t);
3880-
MONO_ADD_INS (cfg->cbb, ins);
3881-
args_ins = ins;
3912+
args_ins = mono_emit_cached_localloc (cfg, 1, fsig->param_count * sizeof (target_mgreg_t));
38823913

38833914
for (int i = 0; i < fsig->param_count; ++i) {
38843915
int addr_reg;
@@ -12335,8 +12366,25 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
1233512366
cfg->cbb = init_localsbb;
1233612367
emit_init_local (cfg, i, header->locals [i], init_locals);
1233712368
}
12369+
12370+
// Patch all locallocs using the cache to allocate the max used size.
12371+
for (int i = 0; i < 2; i++) {
12372+
MonoCachedLocallocInfo *info = &cfg->localloc_cache [i];
12373+
if (info->alloc_size != 0) {
12374+
MONO_EMIT_NEW_PCONST (cfg, info->addr_var->dreg, NULL);
12375+
GSList *p = info->localloc_ins;
12376+
while (p != NULL) {
12377+
MonoInst *localloc_ins = (MonoInst*)p->data;
12378+
localloc_ins->inst_imm = info->alloc_size;
12379+
p = p->next;
12380+
}
12381+
g_slist_free (info->localloc_ins);
12382+
info->localloc_ins = NULL;
12383+
}
12384+
}
1233812385
}
1233912386

12387+
1234012388
if (cfg->init_ref_vars && cfg->method == method) {
1234112389
/* Emit initialization for ref vars */
1234212390
// FIXME: Avoid duplication initialization for IL locals.

src/mono/mono/mini/mini.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,6 +1306,12 @@ typedef enum {
13061306
#define vreg_is_ref(cfg, vreg) (GINT_TO_UINT32(vreg) < (cfg)->vreg_is_ref_len ? (cfg)->vreg_is_ref [(vreg)] : 0)
13071307
#define vreg_is_mp(cfg, vreg) (GINT_TO_UINT32(vreg) < (cfg)->vreg_is_mp_len ? (cfg)->vreg_is_mp [(vreg)] : 0)
13081308

1309+
typedef struct {
1310+
MonoInst* addr_var;
1311+
int alloc_size;
1312+
GSList* localloc_ins;
1313+
} MonoCachedLocallocInfo;
1314+
13091315
/*
13101316
* Control Flow Graph and compilation unit information
13111317
*/
@@ -1661,6 +1667,8 @@ typedef struct {
16611667

16621668
gboolean *clause_is_dead;
16631669

1670+
MonoCachedLocallocInfo localloc_cache [2];
1671+
16641672
/* Stats */
16651673
int stat_allocate_var;
16661674
int stat_locals_stack_size;

0 commit comments

Comments
 (0)