From 5e55fa7480bfc1be6ce45d70ad09693ddf6419de Mon Sep 17 00:00:00 2001 From: Ivan Povazan Date: Thu, 16 Jun 2022 17:46:59 +0200 Subject: [PATCH 1/5] Do not generate impossible instantiations for contrained generics --- src/mono/mono/mini/aot-compiler.c | 49 +++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index fac7efd0fa3fe4..256fb2278ead16 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12307,6 +12307,51 @@ emit_unwind_info_sections_win32 (MonoAotCompile *acfg, const char *function_star } #endif +/** + * should_emit_extra_method_for_generics: + * \param method The method to check for generic parameters. + * \param reference_type Specifies which constraint type to look for. + * + * Tests whether a generic method or a method of a generic type has all of its generic parameter contraints set to a specific type, and based on this setting decides whether + * extra methods should be generated for the method. + * For example: If a method has at least one generic parameter constrained to a reference type (reference_type=TRUE), then REF shared method must be generated. + * On the other hand, if a method has at least one generic parameters constrained to a value type (reference_type=FALSE), then GSHAREDVT method must be generated. + * Special case is when there are no constrains specified, then extra methods must be generated. + * + * Returns: TRUE - extra method should be generated, otherwise FALSE + */ +static gboolean +should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_type) +{ + MonoGenericContainer *gen_container = NULL; + MonoGenericParam *gen_param = NULL; + unsigned int gen_param_count; + unsigned int gen_constraint_mask = reference_type ? GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT : GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT; + + if (method->is_generic && mono_class_is_gtd (method->klass)) { + // TODO: This is rarely encountered and would increase the complexity of covering such case. + // For now always generate extra methods for such case. + return TRUE; + } else if (method->is_generic) { + gen_container = mono_method_get_generic_container (method); + gen_param_count = mono_method_signature_internal (method)->generic_param_count; + } else if (mono_class_is_gtd (method->klass)) { + MonoGenericContainer *gen_container_method = mono_method_get_generic_container (method); + gen_container = mono_class_get_generic_container (method->klass); + gen_param_count = gen_container->type_argc; + } else { + return FALSE; // not a generic + } + g_assert (gen_param_count != 0); + + for (unsigned int i = 0; i < gen_param_count; i++) { + gen_param = mono_generic_container_get_param (gen_container, i); + if ((gen_param->info.flags & gen_constraint_mask) || (gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT)) + return TRUE; + } + return FALSE; +} + static gboolean should_emit_gsharedvt_method (MonoAotCompile *acfg, MonoMethod *method) { @@ -12318,7 +12363,7 @@ should_emit_gsharedvt_method (MonoAotCompile *acfg, MonoMethod *method) if (acfg->image == mono_get_corlib () && !strcmp (m_class_get_name (method->klass), "Volatile")) /* Read/Write are not needed and cause JIT failures */ return FALSE; - return TRUE; + return should_emit_extra_method_for_generics (method, FALSE); } static gboolean @@ -12368,7 +12413,7 @@ collect_methods (MonoAotCompile *acfg) } */ - if (method->is_generic || mono_class_is_gtd (method->klass)) { + if (should_emit_extra_method_for_generics (method, TRUE)) { /* Compile the ref shared version instead */ method = mini_get_shared_method_full (method, SHARE_MODE_NONE, error); if (!method) { From dc4dbb96ca869b00e0f83c11b6517f9892103685 Mon Sep 17 00:00:00 2001 From: Ivan Povazan Date: Thu, 16 Jun 2022 18:33:11 +0200 Subject: [PATCH 2/5] Removing unused variable --- src/mono/mono/mini/aot-compiler.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 256fb2278ead16..822d87c21e513a 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12336,7 +12336,6 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty gen_container = mono_method_get_generic_container (method); gen_param_count = mono_method_signature_internal (method)->generic_param_count; } else if (mono_class_is_gtd (method->klass)) { - MonoGenericContainer *gen_container_method = mono_method_get_generic_container (method); gen_container = mono_class_get_generic_container (method->klass); gen_param_count = gen_container->type_argc; } else { From 1fb385e3fb2d5eaf89714fcacb45ba08140c963c Mon Sep 17 00:00:00 2001 From: Ivan Povazan Date: Fri, 17 Jun 2022 13:08:16 +0200 Subject: [PATCH 3/5] Always generate extra methods when new() constraint is specified --- src/mono/mono/mini/aot-compiler.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 822d87c21e513a..70d90112ee5ea1 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12316,7 +12316,10 @@ emit_unwind_info_sections_win32 (MonoAotCompile *acfg, const char *function_star * extra methods should be generated for the method. * For example: If a method has at least one generic parameter constrained to a reference type (reference_type=TRUE), then REF shared method must be generated. * On the other hand, if a method has at least one generic parameters constrained to a value type (reference_type=FALSE), then GSHAREDVT method must be generated. - * Special case is when there are no constrains specified, then extra methods must be generated. + * Special cases when extra methods are always generated: + * - method of generic type is generic + * - no constraints specified + * - new() constraint specified * * Returns: TRUE - extra method should be generated, otherwise FALSE */ @@ -12345,7 +12348,9 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty for (unsigned int i = 0; i < gen_param_count; i++) { gen_param = mono_generic_container_get_param (gen_container, i); - if ((gen_param->info.flags & gen_constraint_mask) || (gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT)) + if (gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT || + gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_CONSTRUCTOR_CONSTRAINT || + (gen_param->info.flags & gen_constraint_mask)) return TRUE; } return FALSE; From 4c6c1945b8d22736f0f36a445331449dcec9b6bf Mon Sep 17 00:00:00 2001 From: Ivan Povazan Date: Sat, 18 Jun 2022 11:30:52 +0200 Subject: [PATCH 4/5] Fixing the logic for detecting which type parameter constraints require extra methods --- src/mono/mono/mini/aot-compiler.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 70d90112ee5ea1..cf46224bfb1da6 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12312,14 +12312,10 @@ emit_unwind_info_sections_win32 (MonoAotCompile *acfg, const char *function_star * \param method The method to check for generic parameters. * \param reference_type Specifies which constraint type to look for. * - * Tests whether a generic method or a method of a generic type has all of its generic parameter contraints set to a specific type, and based on this setting decides whether - * extra methods should be generated for the method. - * For example: If a method has at least one generic parameter constrained to a reference type (reference_type=TRUE), then REF shared method must be generated. - * On the other hand, if a method has at least one generic parameters constrained to a value type (reference_type=FALSE), then GSHAREDVT method must be generated. - * Special cases when extra methods are always generated: - * - method of generic type is generic - * - no constraints specified - * - new() constraint specified + * Tests whether a generic method or a method of a generic type requires generation of extra methods based its constraints. + * If a method has at least one generic parameter constrained to a reference type, then REF shared method must be generated. + * On the other hand, if a method has at least one generic parameters constrained to a value type, then GSHAREDVT method must be generated. + * Special case when extra methods are always generated is for generic methods of generic types. * * Returns: TRUE - extra method should be generated, otherwise FALSE */ @@ -12329,11 +12325,10 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty MonoGenericContainer *gen_container = NULL; MonoGenericParam *gen_param = NULL; unsigned int gen_param_count; - unsigned int gen_constraint_mask = reference_type ? GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT : GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT; - + if (method->is_generic && mono_class_is_gtd (method->klass)) { // TODO: This is rarely encountered and would increase the complexity of covering such case. - // For now always generate extra methods for such case. + // For now always generate extra methods for such case. return TRUE; } else if (method->is_generic) { gen_container = mono_method_get_generic_container (method); @@ -12346,14 +12341,16 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty } g_assert (gen_param_count != 0); + // Inverted logic - for example: + // if we are checking whether REF shared method should be generated (reference_type = TRUE), + // we are looking for at least one constraint which is not a value type constraint + gboolean should_emit = FALSE; + unsigned int gen_constraint_mask = reference_type ? GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT : GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT; for (unsigned int i = 0; i < gen_param_count; i++) { gen_param = mono_generic_container_get_param (gen_container, i); - if (gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_NO_SPECIAL_CONSTRAINT || - gen_param->info.flags == GENERIC_PARAMETER_ATTRIBUTE_CONSTRUCTOR_CONSTRAINT || - (gen_param->info.flags & gen_constraint_mask)) - return TRUE; + should_emit |= !(gen_param->info.flags & gen_constraint_mask); } - return FALSE; + return should_emit; } static gboolean From a02ceb00de66c1e858bf85a1ed3a7474195a11be Mon Sep 17 00:00:00 2001 From: Ivan Povazan Date: Sat, 18 Jun 2022 11:40:27 +0200 Subject: [PATCH 5/5] Fixing typos and wording --- src/mono/mono/mini/aot-compiler.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index cf46224bfb1da6..a4b1e021ad2ba8 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12310,12 +12310,12 @@ emit_unwind_info_sections_win32 (MonoAotCompile *acfg, const char *function_star /** * should_emit_extra_method_for_generics: * \param method The method to check for generic parameters. - * \param reference_type Specifies which constraint type to look for. + * \param reference_type Specifies which constraint type to look for (TRUE - reference type, FALSE - value type). * - * Tests whether a generic method or a method of a generic type requires generation of extra methods based its constraints. + * Tests whether a generic method or a method of a generic type requires generation of extra methods based on its constraints. * If a method has at least one generic parameter constrained to a reference type, then REF shared method must be generated. - * On the other hand, if a method has at least one generic parameters constrained to a value type, then GSHAREDVT method must be generated. - * Special case when extra methods are always generated is for generic methods of generic types. + * On the other hand, if a method has at least one generic parameter constrained to a value type, then GSHAREDVT method must be generated. + * A special case, when extra methods are always generated, is for generic methods of generic types. * * Returns: TRUE - extra method should be generated, otherwise FALSE */ @@ -12327,8 +12327,8 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty unsigned int gen_param_count; if (method->is_generic && mono_class_is_gtd (method->klass)) { - // TODO: This is rarely encountered and would increase the complexity of covering such case. - // For now always generate extra methods for such case. + // TODO: This is rarely encountered and would increase the complexity of covering such cases. + // For now always generate extra methods. return TRUE; } else if (method->is_generic) { gen_container = mono_method_get_generic_container (method); @@ -12337,13 +12337,13 @@ should_emit_extra_method_for_generics (MonoMethod *method, gboolean reference_ty gen_container = mono_class_get_generic_container (method->klass); gen_param_count = gen_container->type_argc; } else { - return FALSE; // not a generic + return FALSE; // Not a generic. } g_assert (gen_param_count != 0); // Inverted logic - for example: // if we are checking whether REF shared method should be generated (reference_type = TRUE), - // we are looking for at least one constraint which is not a value type constraint + // we are looking for at least one constraint which is not a value type constraint. gboolean should_emit = FALSE; unsigned int gen_constraint_mask = reference_type ? GENERIC_PARAMETER_ATTRIBUTE_VALUE_TYPE_CONSTRAINT : GENERIC_PARAMETER_ATTRIBUTE_REFERENCE_TYPE_CONSTRAINT; for (unsigned int i = 0; i < gen_param_count; i++) {