Skip to content

[mono][aot] Prevent localloc in a loop during constrained gsharedvt calls #117679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,6 @@ public static IEnumerable<object[]> 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.
Expand Down
68 changes: 58 additions & 10 deletions src/mono/mono/mini/method-to-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -3732,6 +3732,45 @@ 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)
{
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.
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:
*
Expand Down Expand Up @@ -3865,20 +3904,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;
Expand Down Expand Up @@ -12335,8 +12366,25 @@ 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);
info->localloc_ins = NULL;
}
}
}


if (cfg->init_ref_vars && cfg->method == method) {
/* Emit initialization for ref vars */
// FIXME: Avoid duplication initialization for IL locals.
Expand Down
8 changes: 8 additions & 0 deletions src/mono/mono/mini/mini.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -1661,6 +1667,8 @@ typedef struct {

gboolean *clause_is_dead;

MonoCachedLocallocInfo localloc_cache [2];

/* Stats */
int stat_allocate_var;
int stat_locals_stack_size;
Expand Down
Loading