[r2r] Enable generic cycles detection by default (#128957)

BrzVlad · web-flow · commit e099ef56d8b4 · 2026-06-08T22:47:32.000+03:00
diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.cs
@@ -170,7 +170,12 @@ public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependenci
                                 }
                                 else
                                 {
-                                    dynamicDependencies.Add(new CombinedDependencyListEntry(factory.GVMDependencies(canonImpl), null, "ImplementingMethodInstantiation"));
+#if READYTORUN
+                                    if (!factory.CanBeInGenericCycle(canonImpl))
+#endif
+                                    {
+                                        dynamicDependencies.Add(new CombinedDependencyListEntry(factory.GVMDependencies(canonImpl), null, "ImplementingMethodInstantiation"));
+                                    }
                                 }
 
 #if !READYTORUN
@@ -265,15 +270,7 @@ private static DependencyNodeCore<NodeFactory> GetVirtualMethodImplNode(NodeFact
             if (!factory.CompilationModuleGroup.ContainsMethodBody(method, false))
                 return null;
 
-            try
-            {
-                factory.DetectGenericCycles(method, method);
-                return factory.CompiledMethodNode(method);
-            }
-            catch (TypeSystemException)
-            {
-                return null;
-            }
+            return factory.CompiledMethodNode(method);
 #endif
         }
     }
diff --git a/src/coreclr/tools/Common/Compiler/GenericCycleDetection/ModuleCycleInfo.cs b/src/coreclr/tools/Common/Compiler/GenericCycleDetection/ModuleCycleInfo.cs
@@ -230,6 +230,15 @@ private bool FormsCycle(TypeSystemEntity entity, out ModuleCycleInfo cycleInfo)
                 }
             }
 
+            /// <summary>
+            /// Returns true if the given entity (method or type definition) could be part of a
+            /// generic cycle.
+            /// </summary>
+            public bool CanBeInCycle(TypeSystemEntity entity)
+            {
+                return FormsCycle(entity, out _);
+            }
+
             public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
             {
                 // This allows to disable cycle detection completely (typically for perf reasons as the algorithm is pretty slow)
diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@@ -85,7 +85,7 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
             {
                 // Because methods with generic parameters are already compiled in their canonical form, we are only interested in finding
                 // instantiations of virtual methods that have at least one non-canonical argument (aka a valuetype).
-                if (HasNonCanonicalInstantiationArguments(canonMethod))
+                if (HasNonCanonicalInstantiationArguments(canonMethod) && !factory.CanBeInGenericCycle(Method))
                 {
                     list = list ?? new DependencyList();
                     list.Add(factory.GVMDependencies(Method), "Virtual dispatch dependency");
diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
@@ -1277,6 +1277,15 @@ public void DetectGenericCycles(TypeSystemEntity caller, TypeSystemEntity callee
             _genericCycleDetector?.DetectCycle(caller, callee);
         }
 
+        public bool CanBeInGenericCycle(MethodDesc method)
+        {
+            if (_genericCycleDetector is null)
+                return false;
+
+            MethodDesc methodDefinition = method.GetTypicalMethodDefinition();
+            return _genericCycleDetector.CanBeInCycle(methodDefinition);
+        }
+
         public Utf8String GetSymbolAlternateName(ISymbolNode node, out bool isHidden)
         {
             isHidden = false;
diff --git a/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs b/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs
@@ -88,8 +88,6 @@ internal class Crossgen2RootCommand : RootCommand
             new("--imagebase") { Description = SR.ImageBase };
         public Option<string> TargetArchitecture { get; } =
             new("--targetarch") { Description = SR.TargetArchOption };
-        public Option<bool> EnableGenericCycleDetection { get; } =
-            new("--enable-generic-cycle-detection") { Description = SR.EnableGenericCycleDetection };
         public Option<int> GenericCycleDepthCutoff { get; } =
             new("--maxgenericcycle") { DefaultValueFactory = _ => ReadyToRunCompilerContext.DefaultGenericCycleDepthCutoff, Description = SR.GenericCycleDepthCutoff };
         public Option<int> GenericCycleBreadthCutoff { get; } =
@@ -197,7 +195,6 @@ public Crossgen2RootCommand(string[] args) : base(SR.Crossgen2BannerText)
             Options.Add(SupportIbc);
             Options.Add(Resilient);
             Options.Add(ImageBase);
-            Options.Add(EnableGenericCycleDetection);
             Options.Add(GenericCycleDepthCutoff);
             Options.Add(GenericCycleBreadthCutoff);
             Options.Add(TargetArchitecture);
diff --git a/src/coreclr/tools/aot/crossgen2/Program.cs b/src/coreclr/tools/aot/crossgen2/Program.cs
@@ -679,12 +679,9 @@ private void RunSingleCompilation(Dictionary<string, string> inFilePaths, Instru
                         .UseCompilationRoots(compilationRoots)
                         .UseOptimizationMode(optimizationMode);
 
-                    if (Get(_command.EnableGenericCycleDetection))
-                    {
-                        builder.UseGenericCycleDetection(
-                            depthCutoff: Get(_command.GenericCycleDepthCutoff),
-                            breadthCutoff: Get(_command.GenericCycleBreadthCutoff));
-                    }
+                    builder.UseGenericCycleDetection(
+                        depthCutoff: Get(_command.GenericCycleDepthCutoff),
+                        breadthCutoff: Get(_command.GenericCycleBreadthCutoff));
 
                     builder.UsePrintReproInstructions(CreateReproArgumentString);
 
diff --git a/src/coreclr/tools/aot/crossgen2/Properties/Resources.resx b/src/coreclr/tools/aot/crossgen2/Properties/Resources.resx
@@ -423,9 +423,6 @@
   <data name="TypeValidation" xml:space="preserve">
     <value>Configure the runtime's behavior around validating the correctness of types defined in assemblies compiled via crossgen2</value>
   </data>
-  <data name="EnableGenericCycleDetection" xml:space="preserve">
-    <value>Perform generic cycle detection during compilation (incurs longer compilation time)</value>
-  </data>
   <data name="GenericCycleBreadthCutoff" xml:space="preserve">
     <value>Total number of occurrences of potentially cyclic generic types within a generic type to cut off</value>
   </data>
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Negative/param02.ilproj b/src/tests/Loader/classloader/generics/Instantiation/Negative/param02.ilproj
@@ -1,6 +1,11 @@
 <Project Sdk="Microsoft.NET.Sdk.IL">
   <PropertyGroup>
+    <!-- Needed for CrossGenTest -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
     <CLRTestPriority>1</CLRTestPriority>
+
+    <!-- Crossgen is not intended to serve as an IL verifier, and is not designed to provide error semantics on bad input. -->
+    <CrossGenTest>false</CrossGenTest>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="param02.il" />
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Negative/param04.ilproj b/src/tests/Loader/classloader/generics/Instantiation/Negative/param04.ilproj
@@ -1,6 +1,11 @@
 <Project Sdk="Microsoft.NET.Sdk.IL">
   <PropertyGroup>
+    <!-- Needed for CrossGenTest -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
     <CLRTestPriority>1</CLRTestPriority>
+
+    <!-- Crossgen is not intended to serve as an IL verifier, and is not designed to provide error semantics on bad input. -->
+    <CrossGenTest>false</CrossGenTest>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="param04.il" />
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Negative/param06.ilproj b/src/tests/Loader/classloader/generics/Instantiation/Negative/param06.ilproj
@@ -1,6 +1,11 @@
 <Project Sdk="Microsoft.NET.Sdk.IL">
   <PropertyGroup>
+    <!-- Needed for CrossGenTest -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
     <CLRTestPriority>1</CLRTestPriority>
+
+    <!-- Crossgen is not intended to serve as an IL verifier, and is not designed to provide error semantics on bad input. -->
+    <CrossGenTest>false</CrossGenTest>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="param06.il" />
diff --git a/src/tests/Loader/classloader/generics/Instantiation/Negative/param08.ilproj b/src/tests/Loader/classloader/generics/Instantiation/Negative/param08.ilproj
@@ -1,6 +1,11 @@
 <Project Sdk="Microsoft.NET.Sdk.IL">
   <PropertyGroup>
+    <!-- Needed for CrossGenTest -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
     <CLRTestPriority>1</CLRTestPriority>
+
+    <!-- Crossgen is not intended to serve as an IL verifier, and is not designed to provide error semantics on bad input. -->
+    <CrossGenTest>false</CrossGenTest>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="param08.il" />
diff --git a/src/tests/readytorun/GenericCycleDetection/Breadth1Test.csproj b/src/tests/readytorun/GenericCycleDetection/Breadth1Test.csproj
@@ -6,7 +6,7 @@
     <!-- This is an explicit crossgen test -->
     <AlwaysUseCrossGen2>true</AlwaysUseCrossGen2>
     <!-- Without this flag Crossgen2 crashes after several minutes with arithmetic overflow -->
-    <CrossGen2TestExtraArguments>--enable-generic-cycle-detection --maxgenericcycle:1 --maxgenericcyclebreadth:1</CrossGen2TestExtraArguments>
+    <CrossGen2TestExtraArguments>--maxgenericcycle:1 --maxgenericcyclebreadth:1</CrossGen2TestExtraArguments>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildProjectName).cs" />
diff --git a/src/tests/readytorun/GenericCycleDetection/Depth1Test.csproj b/src/tests/readytorun/GenericCycleDetection/Depth1Test.csproj
@@ -6,7 +6,7 @@
     <!-- This is an explicit crossgen test -->
     <AlwaysUseCrossGen2>true</AlwaysUseCrossGen2>
     <!-- Without this flag Crossgen2 crashes after several minutes with arithmetic overflow -->
-    <CrossGen2TestExtraArguments>--enable-generic-cycle-detection --maxgenericcycle:1 --maxgenericcyclebreadth:-1</CrossGen2TestExtraArguments>
+    <CrossGen2TestExtraArguments>--maxgenericcycle:1 --maxgenericcyclebreadth:-1</CrossGen2TestExtraArguments>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildProjectName).cs" />
diff --git a/src/tests/readytorun/GenericCycleDetection/Depth3Test.csproj b/src/tests/readytorun/GenericCycleDetection/Depth3Test.csproj
@@ -6,7 +6,7 @@
     <!-- This is an explicit crossgen test -->
     <AlwaysUseCrossGen2>true</AlwaysUseCrossGen2>
     <!-- Without this flag Crossgen2 crashes after several minutes with arithmetic overflow -->
-    <CrossGen2TestExtraArguments>--enable-generic-cycle-detection --maxgenericcycle:3 --maxgenericcyclebreadth:-1</CrossGen2TestExtraArguments>
+    <CrossGen2TestExtraArguments>--maxgenericcycle:3 --maxgenericcyclebreadth:-1</CrossGen2TestExtraArguments>
     <!-- This test OOMs Crossgen2 when running in 32-bit address space -->
     <CLRTestTargetUnsupported Condition="'$(TargetBits)' == '32'">true</CLRTestTargetUnsupported>
   </PropertyGroup>

Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,12 @@ public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependenci`
`170`	`170`	`}`
`171`	`171`	`else`
`172`	`172`	`{`
`173`		`- dynamicDependencies.Add(new CombinedDependencyListEntry(factory.GVMDependencies(canonImpl), null, "ImplementingMethodInstantiation"));`
	`173`	`+#if READYTORUN`
	`174`	`+ if (!factory.CanBeInGenericCycle(canonImpl))`
	`175`	`+#endif`
	`176`	`+ {`
	`177`	`+ dynamicDependencies.Add(new CombinedDependencyListEntry(factory.GVMDependencies(canonImpl), null, "ImplementingMethodInstantiation"));`
	`178`	`+ }`
`174`	`179`	`}`
`175`	`180`
`176`	`181`	`#if !READYTORUN`
`@@ -265,15 +270,7 @@ private static DependencyNodeCore<NodeFactory> GetVirtualMethodImplNode(NodeFact`
`265`	`270`	`if (!factory.CompilationModuleGroup.ContainsMethodBody(method, false))`
`266`	`271`	`return null;`
`267`	`272`
`268`		`- try`
`269`		`- {`
`270`		`- factory.DetectGenericCycles(method, method);`
`271`		`- return factory.CompiledMethodNode(method);`
`272`		`- }`
`273`		`- catch (TypeSystemException)`
`274`		`- {`
`275`		`- return null;`
`276`		`- }`
	`273`	`+ return factory.CompiledMethodNode(method);`
`277`	`274`	`#endif`
`278`	`275`	`}`
`279`	`276`	`}`
Original file line number	Diff line number	Diff line change
`@@ -230,6 +230,15 @@ private bool FormsCycle(TypeSystemEntity entity, out ModuleCycleInfo cycleInfo)`
`230`	`230`	`}`
`231`	`231`	`}`
`232`	`232`
	`233`	`+ /// <summary>`
	`234`	`+ /// Returns true if the given entity (method or type definition) could be part of a`
	`235`	`+ /// generic cycle.`
	`236`	`+ /// </summary>`
	`237`	`+ public bool CanBeInCycle(TypeSystemEntity entity)`
	`238`	`+ {`
	`239`	`+ return FormsCycle(entity, out _);`
	`240`	`+ }`
	`241`	`+`
`233`	`242`	`public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)`
`234`	`243`	`{`
`235`	`244`	`// This allows to disable cycle detection completely (typically for perf reasons as the algorithm is pretty slow)`
Original file line number	Diff line number	Diff line change
`@@ -85,7 +85,7 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact`
`85`	`85`	`{`
`86`	`86`	`// Because methods with generic parameters are already compiled in their canonical form, we are only interested in finding`
`87`	`87`	`// instantiations of virtual methods that have at least one non-canonical argument (aka a valuetype).`
`88`		`- if (HasNonCanonicalInstantiationArguments(canonMethod))`
	`88`	`+ if (HasNonCanonicalInstantiationArguments(canonMethod) && !factory.CanBeInGenericCycle(Method))`
`89`	`89`	`{`
`90`	`90`	`list = list ?? new DependencyList();`
`91`	`91`	`list.Add(factory.GVMDependencies(Method), "Virtual dispatch dependency");`