Skip to content

Have mono handle ConvertToIntegerNative for Double and Single #112206

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

Merged
merged 5 commits into from
Feb 10, 2025
Merged

Have mono handle ConvertToIntegerNative for Double and Single #112206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8bf0780
Have mono handle ConvertToIntegerNative for Double and Single
tannergooding Feb 5, 2025
e90351f
Ensure indirect calls on Mono produce the expected result for Convert…
tannergooding Feb 6, 2025
c0d450e
Ensure the mono interpreter also handles ConvertToIntegerNative for d…
tannergooding Feb 6, 2025
4a263b9
Ensure mono uses OP_RCONV_TO_* for Single.ConvertIntegerToNative
tannergooding Feb 7, 2025
6f6a197
Ensure we decompose the opcode to support ones that are emulated
tannergooding Feb 7, 2025
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
3 changes: 0 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Double.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,15 +665,12 @@ public static TInteger ConvertToInteger<TInteger>(double value)
public static TInteger ConvertToIntegerNative<TInteger>(double value)
where TInteger : IBinaryInteger<TInteger>
{
#if !MONO
if (typeof(TInteger).IsPrimitive)
{
// We need this to be recursive so indirect calls (delegates
// for example) produce the same result as direct invocation
return ConvertToIntegerNative<TInteger>(value);
}
#endif

return TInteger.CreateSaturating(value);
}

Expand Down
3 changes: 0 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/Single.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,15 +660,12 @@ public static TInteger ConvertToInteger<TInteger>(float value)
public static TInteger ConvertToIntegerNative<TInteger>(float value)
where TInteger : IBinaryInteger<TInteger>
{
#if !MONO
if (typeof(TInteger).IsPrimitive)
{
// We need this to be recursive so indirect calls (delegates
// for example) produce the same result as direct invocation
return ConvertToIntegerNative<TInteger>(value);
}
#endif

return TInteger.CreateSaturating(value);
}

Expand Down
49 changes: 48 additions & 1 deletion src/mono/mono/mini/interp/transform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2676,7 +2676,54 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
!strncmp ("Vector", klass_name, 6) &&
!strcmp (tm, "get_IsHardwareAccelerated"))) {
*op = MINT_LDC_I4_0;
}
} else if ((target_method->klass == mono_defaults.double_class) || (target_method->klass == mono_defaults.single_class)) {
MonoGenericContext *method_context = mono_method_get_context (target_method);
bool isDouble = target_method->klass == mono_defaults.double_class;
if (!strcmp (tm, "ConvertToIntegerNative") &&
method_context != NULL &&
method_context->method_inst->type_argc == 1) {
MonoTypeEnum tto_type = method_context->method_inst->type_argv [0]->type;
switch (tto_type) {
case MONO_TYPE_I1:
*op = isDouble ? MINT_CONV_I1_R8 : MINT_CONV_I1_R4;
break;
case MONO_TYPE_I2:
*op = isDouble ? MINT_CONV_I2_R8 : MINT_CONV_I2_R4;
break;
#if TARGET_SIZEOF_VOID_P == 4
case MONO_TYPE_I:
#endif
case MONO_TYPE_I4:
*op = isDouble ? MINT_CONV_I4_R8 : MINT_CONV_I4_R4;
break;
#if TARGET_SIZEOF_VOID_P == 8
case MONO_TYPE_I:
#endif
case MONO_TYPE_I8:
*op = isDouble ? MINT_CONV_I8_R8 : MINT_CONV_I8_R4;
break;
case MONO_TYPE_U1:
*op = isDouble ? MINT_CONV_U1_R8 : MINT_CONV_U1_R4;
break;
case MONO_TYPE_U2:
*op = isDouble ? MINT_CONV_U2_R8 : MINT_CONV_U2_R4;
break;
#if TARGET_SIZEOF_VOID_P == 4
case MONO_TYPE_U:
#endif
case MONO_TYPE_U4:
*op = isDouble ? MINT_CONV_U4_R8 : MINT_CONV_U4_R4;
break;
#if TARGET_SIZEOF_VOID_P == 8
case MONO_TYPE_U:
#endif
case MONO_TYPE_U8:
*op = isDouble ? MINT_CONV_U8_R8 : MINT_CONV_U8_R4;
break;
default: return FALSE;
}
}
}

return FALSE;
}
Expand Down
58 changes: 58 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 @@ -2322,6 +2322,64 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign
return ins;
}
}
} else if ((cmethod->klass == mono_defaults.double_class) || (cmethod->klass == mono_defaults.single_class)) {
MonoGenericContext *method_context = mono_method_get_context (cmethod);
bool isDouble = cmethod->klass == mono_defaults.double_class;
if (!strcmp (cmethod->name, "ConvertToIntegerNative") &&
method_context != NULL &&
method_context->method_inst->type_argc == 1) {
int opcode = 0;
MonoTypeEnum tto_type = method_context->method_inst->type_argv [0]->type;
MonoStackType tto_stack = STACK_I4;
switch (tto_type) {
case MONO_TYPE_I1:
opcode = isDouble ? OP_FCONV_TO_I1 : OP_RCONV_TO_I1;
break;
case MONO_TYPE_I2:
opcode = isDouble ? OP_FCONV_TO_I2 : OP_RCONV_TO_I2;
break;
#if TARGET_SIZEOF_VOID_P == 4
Copy link
Member

@lateralusX lateralusX Feb 6, 2025

Choose a reason for hiding this comment

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

nit: in other places we use SIZEOF_REGISTER for these kinds of checks.

Copy link
Member

@matouskozak matouskozak Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

looks like it varies based on a file, e.g., in simd-intrinsics.c, the TARGET_SIZEOF_VOID_P looks to be the dominant way.

Copy link
Member

@lateralusX lateralusX Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yeah its mixed, in this file SIZEOF_REGISTER seems to dominate. An interesting observation is that we have some code like this:

#if (SIZEOF_REGISTER > TARGET_SIZEOF_VOID_P) && (G_BYTE_ORDER == G_BIG_ENDIAN)

and we have this comment in mono-machine.h

SIZEOF_REGISTER is usually the same as TARGET_SIZEOF_VOID_P, except when MONO_ARCH_ILP32 is defined

Copy link
Member Author

@tannergooding tannergooding Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Conversions care about the size of a pointer, not the size of the register; hence why this one was selected. It matches how other code paths, such as method-to-ir, typically deal with MONO_TYPE_I.

case MONO_TYPE_I:
#endif
case MONO_TYPE_I4:
opcode = isDouble ? OP_FCONV_TO_I4 : OP_RCONV_TO_I4;
break;
#if TARGET_SIZEOF_VOID_P == 8
case MONO_TYPE_I:
#endif
case MONO_TYPE_I8:
opcode = isDouble ? OP_FCONV_TO_I8 : OP_RCONV_TO_I8;
tto_stack = STACK_I8;
break;
case MONO_TYPE_U1:
opcode = isDouble ? OP_FCONV_TO_U1 : OP_RCONV_TO_U1;
break;
case MONO_TYPE_U2:
opcode = isDouble ? OP_FCONV_TO_U2 : OP_RCONV_TO_U2;
break;
#if TARGET_SIZEOF_VOID_P == 4
case MONO_TYPE_U:
#endif
case MONO_TYPE_U4:
opcode = isDouble ? OP_FCONV_TO_U4 : OP_RCONV_TO_U4;
break;
#if TARGET_SIZEOF_VOID_P == 8
case MONO_TYPE_U:
#endif
case MONO_TYPE_U8:
opcode = isDouble ? OP_FCONV_TO_U8 : OP_RCONV_TO_U8;
tto_stack = STACK_I8;
break;
default: return NULL;
}

if (opcode != 0) {
int ireg = mono_alloc_ireg (cfg);
EMIT_NEW_UNALU (cfg, ins, opcode, ireg, args [0]->dreg);
ins->type = tto_stack;
return mono_decompose_opcode(cfg, ins);
}
}
}

ins = mono_emit_simd_intrinsics (cfg, cmethod, fsig, args);
Expand Down
Loading