[interp] Remove hack for nint/nfloat (#21395)mono-6.12.0.165

These structures are valuetypes, but their mint_type is a primitive. This means that a LDFLD applied on the type would have attempted to do a pointer dereference, because it saw that the current top of stack is not STACK_TYPE_VT. This was fixed in the past by passing the managed pointer to the valuetype rather than the valuetype itself, against the normal call convention, which lead to inconsistencies in the code.

This commit removes that hack and fixes the problem by ignoring LDFLD applied to nint/nfloat valuetypes in the first place.

1 files changed, 9 insertions, 38 deletions

diff --git a/mono/mini/interp/transform.c b/mono/mini/interp/transform.c
index 60cfb37c5a2..de7725aa8cc 100644
--- a/mono/mini/interp/transform.c
+++ b/mono/mini/interp/transform.c
@@ -931,24 +931,6 @@ interp_method_get_header (MonoMethod* method, MonoError *error)
 		return mono_method_get_header_internal (method, error);
 }
 
-/* stores top of stack as local and pushes address of it on stack */
-static void
-emit_store_value_as_local (TransformData *td, MonoType *src)
-{
-	int size = mini_magic_type_size (NULL, src);
-	int local = create_interp_local (td, mini_native_type_replace_type (src));
-
-	store_local (td, local);
-
-	size = ALIGN_TO (size, MINT_VT_ALIGNMENT);
-	interp_add_ins (td, MINT_LDLOC_VT);
-	td->last_ins->data [0] = local;
-	WRITE32_INS (td->last_ins, 1, &size);
-
-	PUSH_VT (td, size);
-	PUSH_TYPE (td, STACK_TYPE_VT, NULL);
-}
-
 // Returns whether we can optimize away the instructions starting at start.
 // If any instructions are part of a new basic block, we can't remove them.
 static gboolean
@@ -1246,15 +1228,15 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 			int src_size = mini_magic_type_size (NULL, src);
 			int dst_size = mini_magic_type_size (NULL, dst);
 
-			gboolean store_value_as_local = FALSE;
+			gboolean managed_fallback = FALSE;
 
 			switch (type_index) {
 			case 0: case 1:
 				if (!mini_magic_is_int_type (src) || !mini_magic_is_int_type (dst)) {
 					if (mini_magic_is_int_type (src))
-						store_value_as_local = TRUE;
+						managed_fallback = TRUE;
 					else if (mono_class_is_magic_float (src_klass))
-						store_value_as_local = TRUE;
+						managed_fallback = TRUE;
 					else
 						return FALSE;
 				}
@@ -1262,21 +1244,17 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 			case 2:
 				if (!mini_magic_is_float_type (src) || !mini_magic_is_float_type (dst)) {
 					if (mini_magic_is_float_type (src))
-						store_value_as_local = TRUE;
+						managed_fallback = TRUE;
 					else if (mono_class_is_magic_int (src_klass))
-						store_value_as_local = TRUE;
+						managed_fallback = TRUE;
 					else
 						return FALSE;
 				}
 				break;
 			}
 
-			if (store_value_as_local) {
-				emit_store_value_as_local (td, src);
-
-				/* emit call to managed conversion method */
+			if (managed_fallback)
 				return FALSE;
-			}
 
 			if (src_size > dst_size) { // 8 -> 4
 				switch (type_index) {
@@ -1327,14 +1305,6 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 			td->ip += 5;
 			return TRUE;
 		} else if (!strcmp ("CompareTo", tm) || !strcmp ("Equals", tm)) {
-			MonoType *arg = csignature->params [0];
-
-			/* on 'System.n*::{CompareTo,Equals} (System.n*)' variant we need to push managed
-			 * pointer instead of value */
-			if (arg->type == MONO_TYPE_VALUETYPE)
-				emit_store_value_as_local (td, arg);
-
-			/* emit call to managed conversion method */
 			return FALSE;
 		} else if (!strcmp (".cctor", tm)) {
 			/* white list */
@@ -1938,8 +1908,7 @@ interp_method_check_inlining (TransformData *td, MonoMethod *method)
 	if (method->wrapper_type != MONO_WRAPPER_NONE)
 		return FALSE;
 
-	/* Our usage of `emit_store_value_as_local ()` for nint, nuint and nfloat
-	 * is kinda hacky, and doesn't work with the inliner */
+	// FIXME Re-enable this
 	if (mono_class_get_magic_index (method->klass) >= 0)
 		return FALSE;
 
@@ -4752,6 +4721,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
 					interp_add_ins (td, MINT_POP);
 					interp_emit_sfld_access (td, field, field_klass, mt, TRUE, error);
 					goto_if_nok (error, exit);
+				} else if (td->sp [-1].type != STACK_TYPE_O && td->sp [-1].type != STACK_TYPE_MP && (mono_class_is_magic_int (klass) || mono_class_is_magic_float (klass))) {
+					// No need to load anything, the value is already on the execution stack
 				} else {
 					int opcode = MINT_LDFLD_I1 + mt - MINT_TYPE_I1;
 #ifdef NO_UNALIGNED_ACCESS