Skip to content
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.

Sign up for GitHub

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

[mono] RuntimeHelpers.CreateSpan<T> is now intrinsic. #81695

Merged
merged 13 commits into from
Feb 13, 2023
Merged

[mono] RuntimeHelpers.CreateSpan<T> is now intrinsic. #81695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
789a6f7
[mono] RuntimeHelpers.CreateSpan is now intrinsic.
jandupej Jan 31, 2023
b76e424
[mono][jit] RuntimeHelpers.CreateSpan intrinsic fixes.
jandupej Feb 1, 2023
5c23aa3
[mono][jit] RuntimeHelpers.CreateSpan is intrinsic and seems to work.
jandupej Feb 6, 2023
ff717ca
[mono] Fixed a warning that broke build.
jandupej Feb 6, 2023
14dba0f
[mono] Pointer arithmetic warnings hopefully solved.
jandupej Feb 7, 2023
9882de0
[mono] Fix pointer conversion warning, take 2.
jandupej Feb 7, 2023
aad9e93
[mono] RuntimeHelpers.CreateSpan<T> is no longer intrinsic under AOT.
jandupej Feb 7, 2023
1174cb5
[mono] RuntimeHelpers.CreateSpan<T> now constructs the result in a sa…
jandupej Feb 8, 2023
e831cdb
[mono][aot] RuntimeHelpers.CreateSpan<T> intrinsic fixed for AOT case.
jandupej Feb 8, 2023
fe79643
[mono] Cleaned up whitespace changes in decompose.c, YAGNI in ir-emit.h.
jandupej Feb 8, 2023
c55bbdf
[mono] OP_LDTOKEN_FIELD mini opcode introduced for RuntimeHelpers.Cre…
jandupej Feb 10, 2023
619cba6
[mono][aot] RuntimeHelpers.CreateSpan<T> intrinsic now emits the corr…
jandupej Feb 10, 2023
0e03cf0
[mono] RuntimeHelpers.CreateSpan intrinsic, removed superfluous sanit…
jandupej Feb 13, 2023
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
4 changes: 4 additions & 0 deletions src/mono/mono/mini/decompose.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,10 @@ mono_decompose_vtype_opts (MonoCompile *cfg)
mono_simd_decompose_intrinsic (cfg, bb, ins);
#endif
switch (ins->opcode) {
case OP_LDTOKEN_FIELD:
ins->opcode = OP_VMOVE;
restart = TRUE;
break;
case OP_VMOVE: {
g_assert (ins->klass);
if (COMPILE_LLVM (cfg) && !mini_is_gsharedvt_klass (ins->klass))
Expand Down
43 changes: 43 additions & 0 deletions src/mono/mono/mini/intrinsics.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,6 +1008,49 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign
EMIT_NEW_UNALU (cfg, ins, OP_ICGT, dreg, -1);
ins->type = STACK_I4;
return ins;
} else if (!strcmp (cmethod->name, "CreateSpan") && fsig->param_count == 1) {
MonoGenericContext* ctx = mono_method_get_context (cmethod);
g_assert (ctx);
g_assert (ctx->method_inst);
g_assert (ctx->method_inst->type_argc == 1);
MonoType* arg_type = ctx->method_inst->type_argv [0];
MonoType* t = mini_get_underlying_type (arg_type);
g_assert (!MONO_TYPE_IS_REFERENCE (t) && t->type != MONO_TYPE_VALUETYPE);

// This OP_LDTOKEN_FIELD later changes into a OP_VMOVE.
MonoClassField* field = (MonoClassField*) args [0]->inst_p1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a little brittle, it depends on OP_VMOVE never having its inst_p1 set otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I could also use backend; the comments there do not indicate that the field is used with OP_VMOVE. Another way would be to do what you suggested earlier - emit a new opcode that would carry it in inst_p1 and then decompose it away. I had some issues getting that approach to work, so I tried this. It seems to work, although I cannot honestly verify that its inst_p1 is always untouched. Wouldn't that also be the case for the new opcode?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a new opcode would mean that this code could be sure that the input comes from the CEE_LDTOKEN code. The downside is that new opcode needs to be decomposed/lowered if its not followed by CreateSpan etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now a OP_LDTOKEN_FIELD is emitted, which is equivalent in every way to a OP_VMOVE, except its opcode. This should protect the MonoInst contents until it is used by CreateSpan intrinsic. The decomposition of OP_LDTOKEN_FIELD is then only rewriting the opcode to OP_VMOVE.

if (args [0]->opcode != OP_LDTOKEN_FIELD)
return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The !field etc. checks should not be needed any more.


int alignment = 0;
const int element_size = mono_type_size (t, &alignment);
const int num_elements = mono_type_size (field->type, &alignment) / element_size;
const int obj_size = MONO_ABI_SIZEOF (MonoObject);

MonoInst* span = mono_compile_create_var (cfg, fsig->ret, OP_LOCAL);
MonoInst* span_addr;
EMIT_NEW_TEMPLOADA (cfg, span_addr, span->inst_c0);

MonoInst* ptr_inst;
if (cfg->compile_aot) {
NEW_RVACONST (cfg, ptr_inst, mono_class_get_image (mono_field_get_parent (field)), args [0]->inst_c0);
MONO_ADD_INS (cfg->cbb, ptr_inst);
} else {
#if G_BYTE_ORDER == G_LITTLE_ENDIAN
const int swizzle = 1;
#else
const int swizzle = element_size;
#endif
gpointer data_ptr = (gpointer)mono_field_get_rva (field, swizzle);
EMIT_NEW_PCONST (cfg, ptr_inst, data_ptr);
}

MonoClassField* field_ref = mono_class_get_field_from_name_full (span->klass, "_reference", NULL);
MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STOREP_MEMBASE_REG, span_addr->dreg, field_ref->offset - obj_size, ptr_inst->dreg);
MonoClassField* field_len = mono_class_get_field_from_name_full (span->klass, "_length", NULL);
MONO_EMIT_NEW_STORE_MEMBASE_IMM (cfg, OP_STOREI4_MEMBASE_IMM, span_addr->dreg, field_len->offset - obj_size, num_elements);
EMIT_NEW_TEMPLOAD (cfg, ins, span->inst_c0);
return ins;
} else
return NULL;
} else if (cmethod->klass == mono_class_try_get_memory_marshal_class ()) {
Expand Down
2 changes: 2 additions & 0 deletions src/mono/mono/mini/ir-emit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ alloc_dreg (MonoCompile *cfg, MonoStackType stack_type)

#define NEW_LDSTRCONST(cfg,dest,image,token) NEW_AOTCONST_TOKEN ((cfg), (dest), MONO_PATCH_INFO_LDSTR, (image), (token), NULL, STACK_OBJ, mono_defaults.string_class)

#define NEW_RVACONST(cfg,dest,image,token) NEW_AOTCONST_TOKEN ((cfg), (dest), MONO_PATCH_INFO_RVA, (image), (token), NULL, STACK_MP, NULL)

#define NEW_LDSTRLITCONST(cfg,dest,val) NEW_AOTCONST ((cfg), (dest), MONO_PATCH_INFO_LDSTR_LIT, (val))

#define NEW_TYPE_FROM_HANDLE_CONST(cfg,dest,image,token,generic_context) NEW_AOTCONST_TOKEN ((cfg), (dest), MONO_PATCH_INFO_TYPE_FROM_HANDLE, (image), (token), (generic_context), STACK_OBJ, mono_defaults.runtimetype_class)
Expand Down
7 changes: 7 additions & 0 deletions src/mono/mono/mini/method-to-ir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10820,9 +10820,16 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
} else {
EMIT_NEW_PCONST (cfg, ins, handle);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

EMIT_NEW_TEMPLOADA (cfg, addr, vtvar->inst_c0);
MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STORE_MEMBASE_REG, addr->dreg, 0, ins->dreg);
EMIT_NEW_TEMPLOAD (cfg, ins, vtvar->inst_c0);
ins->opcode = OP_LDTOKEN_FIELD;
ins->inst_c0 = n;
ins->inst_p1 = handle;

cfg->flags |= MONO_CFG_NEEDS_DECOMPOSE;
cfg->cbb->needs_decompose = TRUE;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/mini-ops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,7 @@ MINI_OP(OP_LOAD_GOTADDR, "load_gotaddr", IREG, NONE, NONE)
MINI_OP(OP_DUMMY_USE, "dummy_use", NONE, IREG, NONE)
MINI_OP(OP_NOT_REACHED, "not_reached", NONE, NONE, NONE)
MINI_OP(OP_NOT_NULL, "not_null", NONE, IREG, NONE)
MINI_OP(OP_LDTOKEN_FIELD, "ldtoken_field", VREG, VREG, NONE)

/* SIMD opcodes. */

Expand Down