Adapt Tarjan generic cycle detector for use in Crossgen2

src/coreclr/tools/Common/Compiler/GenericCycleDetection/GenericCycleDetection.projitems

@@ -0,0 +1,13 @@
+<Project>
+
+  <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)Graph.cs" Link="Common\Compiler\GenericCycleDetection\Graph.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Graph.Cycles.cs" Link="Common\Compiler\GenericCycleDetection\Graph.Cycles.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Graph.Vertex.cs" Link="Common\Compiler\GenericCycleDetection\Graph.Vertex.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)GraphBuilder.cs" Link="Common\Compiler\GenericCycleDetection\GraphBuilder.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)GraphBuilder.ForEach.cs" Link="Common\Compiler\GenericCycleDetection\GraphBuilder.ForEach.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)GraphBuilder.MethodCall.cs" Link="Common\Compiler\GenericCycleDetection\GraphBuilder.MethodCall.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)ModuleCycleInfo.cs" Link="Common\Compiler\GenericCycleDetection\ModuleCycleInfo.cs" />
+  </ItemGroup>
+
+</Project>

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/ModuleCycleInfo.cs → src/coreclr/tools/Common/Compiler/GenericCycleDetection/ModuleCycleInfo.cs

@@ -8,7 +8,9 @@
 using Internal.TypeSystem;
 using Internal.TypeSystem.Ecma;
 
+#if !READYTORUN
 using ILLink.Shared;
+#endif
 
 using Debug = System.Diagnostics.Debug;
 
@@ -91,11 +93,13 @@ public EntityPair(TypeSystemEntity owner, TypeSystemEntity referent)
             // from the key, but since this is a key/value pair, might as well use the value too...
             private readonly ConcurrentDictionary<EntityPair, ModuleCycleInfo> _actualProblems = new ConcurrentDictionary<EntityPair, ModuleCycleInfo>();
 
-            private readonly int _cutoffPoint;
+            private readonly int _depthCutoff;
+            private readonly int _breadthCutoff;
 
-            public GenericCycleDetector(int cutoffPoint)
+            public GenericCycleDetector(int depthCutoff, int breadthCutoff)
             {
-                _cutoffPoint = cutoffPoint;
+                _depthCutoff = depthCutoff;
+                _breadthCutoff = breadthCutoff;
             }
 
             private bool IsDeepPossiblyCyclicInstantiation(TypeSystemEntity entity)
@@ -110,17 +114,31 @@ private bool IsDeepPossiblyCyclicInstantiation(TypeSystemEntity entity)
                 }
             }
 
-            private bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, List<TypeDesc> seenTypes = null)
+            private bool IsDeepPossiblyCyclicInstantiation(TypeDesc type)
+            {
+                int breadthCounter = 0;
+                return IsDeepPossiblyCyclicInstantiation(type, ref breadthCounter, seenTypes: null);
+            }
+
+            private bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, ref int breadthCounter, List<TypeDesc> seenTypes = null)
             {
                 switch (type.Category)
                 {
                     case TypeFlags.Array:
                     case TypeFlags.SzArray:
-                        return IsDeepPossiblyCyclicInstantiation(((ParameterizedType)type).ParameterType, seenTypes);
+                        return IsDeepPossiblyCyclicInstantiation(((ParameterizedType)type).ParameterType, ref breadthCounter, seenTypes);
                     default:
                         TypeDesc typeDef = type.GetTypeDefinition();
                         if (type != typeDef)
                         {
+                            if (FormsCycle(typeDef, out ModuleCycleInfo _))
+                            {
+                                if (_breadthCutoff >= 0 && ++breadthCounter >= _breadthCutoff)
+                                {
+                                    return true;
+                                }
+                            }
+
                             (seenTypes ??= new List<TypeDesc>()).Add(typeDef);
                             for (int i = 0; i < seenTypes.Count; i++)
                             {
@@ -133,26 +151,26 @@ private bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, List<TypeDesc> see
                                         count++;
                                     }
 
-                                    if (count > _cutoffPoint)
+                                    if (count > _depthCutoff)
                                     {
                                         return true;
                                     }
                                 }
                             }
 
-                            bool result = IsDeepPossiblyCyclicInstantiation(type.Instantiation, seenTypes);
+                            bool result = IsDeepPossiblyCyclicInstantiation(type.Instantiation, ref breadthCounter, seenTypes);
                             seenTypes.RemoveAt(seenTypes.Count - 1);
                             return result;
                         }
                         return false;
                 }
             }
 
-            private bool IsDeepPossiblyCyclicInstantiation(Instantiation instantiation, List<TypeDesc> seenTypes = null)
+            private bool IsDeepPossiblyCyclicInstantiation(Instantiation instantiation, ref int breadthCounter, List<TypeDesc> seenTypes)
             {
                 foreach (TypeDesc arg in instantiation)
                 {
-                    if (IsDeepPossiblyCyclicInstantiation(arg, seenTypes))
+                    if (IsDeepPossiblyCyclicInstantiation(arg, ref breadthCounter, seenTypes))
                     {
                         return true;
                     }
@@ -163,13 +181,30 @@ private bool IsDeepPossiblyCyclicInstantiation(Instantiation instantiation, List
 
             public bool IsDeepPossiblyCyclicInstantiation(MethodDesc method)
             {
-                return IsDeepPossiblyCyclicInstantiation(method.Instantiation) || IsDeepPossiblyCyclicInstantiation(method.OwningType);
+                int breadthCounter = 0;
+                return IsDeepPossiblyCyclicInstantiation(method.Instantiation, ref breadthCounter, seenTypes: null)
+                    || IsDeepPossiblyCyclicInstantiation(method.OwningType, ref breadthCounter, seenTypes: null);
+            }
+
+            private bool FormsCycle(TypeSystemEntity entity, out ModuleCycleInfo cycleInfo)
+            {
+                EcmaModule ownerModule = (entity as EcmaType)?.EcmaModule ?? (entity as EcmaMethod)?.Module;
+                if (ownerModule != null)
+                {
+                    cycleInfo = _hashtable.GetOrCreateValue(ownerModule);
+                    return cycleInfo.FormsCycle(entity);
+                }
+                else
+                {
+                    cycleInfo = null;
+                    return false;
+                }
             }
 
             public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
             {
                 // This allows to disable cycle detection completely (typically for perf reasons as the algorithm is pretty slow)
-                if (_cutoffPoint < 0)
+                if (_depthCutoff < 0)
                     return;
 
                 // Not clear if generic recursion through fields is a thing
@@ -192,10 +227,7 @@ public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
                     return;
                 }
 
-                EcmaModule ownerModule = (ownerDefinition as EcmaType)?.EcmaModule ?? ((EcmaMethod)ownerDefinition).Module;
-
-                ModuleCycleInfo cycleInfo = _hashtable.GetOrCreateValue(ownerModule);
-                if (cycleInfo.FormsCycle(ownerDefinition))
+                if (FormsCycle(ownerDefinition, out ModuleCycleInfo cycleInfo))
                 {
                     // Just the presence of a cycle is not a problem, but once we start getting too deep,
                     // we need to cut our losses.
@@ -217,6 +249,7 @@ public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
                 }
             }
 
+#if !READYTORUN
             public void LogWarnings(Logger logger)
             {
                 // Might need to sort these if we care about warning determinism, but we probably don't.
@@ -252,6 +285,7 @@ public void LogWarnings(Logger logger)
                     logger.LogWarning(actualProblem.Key.Owner, DiagnosticId.GenericRecursionCycle, actualProblem.Key.Referent.GetDisplayName(), message);
                 }
             }
+#endif
         }
     }
 }

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

@@ -552,6 +552,17 @@ private void PublishCode()
                 {
                     if (computedNodes.Add(fixup))
                     {
+                        if (fixup is IMethodNode methodNode)
+                        {
+                            try
+                            {
+                                _compilation.NodeFactory.DetectGenericCycles(_methodCodeNode.Method, methodNode.Method);
+                            }
+                            catch (TypeLoadException)
+                            {
+                                throw new RequiresRuntimeJitException("Requires runtime JIT - potential generic cycle detected");
+                            }
+                        }
                         _methodCodeNode.Fixups.Add(fixup);
                     }
                 }

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs

@@ -22,7 +22,9 @@ public partial class CompilerTypeSystemContext
         // We want this to be high enough so that it doesn't cut off too early. But also not too
         // high because things that are recursive often end up expanding laterally as well
         // through various other generic code the deep code calls into.
-        public const int DefaultGenericCycleCutoffPoint = 4;
+        public const int DefaultGenericCycleDepthCutoff = 4;
+
+        public const int DefaultGenericCycleBreadthCutoff = 10;
 
         public SharedGenericsConfiguration GenericsConfig
         {
@@ -40,7 +42,9 @@ public SharedGenericsConfiguration GenericsConfig
         private MetadataType _arrayOfTType;
         private MetadataType _attributeType;
 
-        public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode genericsMode, DelegateFeature delegateFeatures, int genericCycleCutoffPoint = DefaultGenericCycleCutoffPoint)
+        public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode genericsMode, DelegateFeature delegateFeatures,
+            int genericCycleDepthCutoff = DefaultGenericCycleDepthCutoff,
+            int genericCycleBreadthCutoff = DefaultGenericCycleBreadthCutoff)
             : base(details)
         {
             _genericsMode = genericsMode;
@@ -51,7 +55,7 @@ public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode gener
 
             _delegateInfoHashtable = new DelegateInfoHashtable(delegateFeatures);
 
-            _genericCycleDetector = new LazyGenericsSupport.GenericCycleDetector(genericCycleCutoffPoint);
+            _genericCycleDetector = new LazyGenericsSupport.GenericCycleDetector(genericCycleDepthCutoff, genericCycleBreadthCutoff);
 
             GenericsConfig = new SharedGenericsConfiguration();
         }

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

@@ -438,13 +438,6 @@
     <Compile Include="Compiler\FeatureSwitchManager.cs" />
     <Compile Include="Compiler\GeneratingMetadataManager.cs" />
     <Compile Include="Compiler\GenericRootProvider.cs" />
-    <Compile Include="Compiler\LazyGenerics\ModuleCycleInfo.cs" />
-    <Compile Include="Compiler\LazyGenerics\Graph.cs" />
-    <Compile Include="Compiler\LazyGenerics\Graph.Cycles.cs" />
-    <Compile Include="Compiler\LazyGenerics\Graph.Vertex.cs" />
-    <Compile Include="Compiler\LazyGenerics\GraphBuilder.cs" />
-    <Compile Include="Compiler\LazyGenerics\GraphBuilder.ForEach.cs" />
-    <Compile Include="Compiler\LazyGenerics\GraphBuilder.MethodCall.cs" />
     <Compile Include="Compiler\HardwareIntrinsicHelpers.Aot.cs" />
     <Compile Include="Compiler\IInliningPolicy.cs" />
     <Compile Include="Compiler\Logging\NativeAotFatalErrorException.cs" />
@@ -711,4 +704,6 @@
   </ItemGroup>
 
   <Import Project="..\..\..\..\tools\illink\src\ILLink.Shared\ILLink.Shared.projitems" Label="Shared" />
+  <Import Project="..\..\Common\Compiler\GenericCycleDetection\GenericCycleDetection.projitems" />
+
 </Project>

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/AllMethodsOnTypeNode.cs

@@ -40,7 +40,14 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
                 if (!method.IsGenericMethodDefinition &&
                     context.CompilationModuleGroup.ContainsMethodBody(method, false))
                 {
-                    dependencies.Add(context.CompiledMethodNode(method), $"Method on type {Type.ToString()}");
+                    try
+                    {
+                        context.DetectGenericCycles(Type, method);
+                        dependencies.Add(context.CompiledMethodNode(method), $"Method on type {Type.ToString()}");
+                    }
+                    catch (TypeSystemException)
+                    {
+                    }
                 }
             }
 

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelayLoadHelperMethodImport.cs

@@ -48,8 +48,20 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
                 MethodDesc canonMethod = _method.Method.GetCanonMethodTarget(CanonicalFormKind.Specific);
                 if (factory.CompilationModuleGroup.ContainsMethodBody(canonMethod, false))
                 {
-                    ISymbolNode canonMethodNode = factory.CompiledMethodNode(canonMethod);
-                    yield return new DependencyListEntry(canonMethodNode, "Canonical method for instantiating stub");
+                    bool useDependency = true;
+                    try
+                    {
+                        factory.DetectGenericCycles(_method.Method, canonMethod);
+                    }
+                    catch (TypeSystemException)
+                    {
+                        useDependency = false;
+                    }
+                    if (useDependency)
+                    {
+                        ISymbolNode canonMethodNode = factory.CompiledMethodNode(canonMethod);
+                        yield return new DependencyListEntry(canonMethodNode, "Canonical method for instantiating stub");
+                    }
                 }
             }
         }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

@@ -58,7 +58,14 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
                 factory.CompilationModuleGroup.ContainsMethodBody(canonMethod, false))
             {
                 list = list ?? new DependencyAnalysisFramework.DependencyNodeCore<NodeFactory>.DependencyList();
-                list.Add(factory.CompiledMethodNode(canonMethod), "Virtual function dependency on cross module inlineable method");
+                try
+                {
+                    factory.DetectGenericCycles(_method.Method, canonMethod);
+                    list.Add(factory.CompiledMethodNode(canonMethod), "Virtual function dependency on cross module inlineable method");
+                }
+                catch (TypeSystemException)
+                {
+                }
             }
 
             return list;

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs

@@ -182,7 +182,14 @@ public static void AddDependenciesForAsyncStateMachineBox(ref DependencyList dep
                         {
                             case "MoveNext":
                             case ".cctor":
-                                dependencies.Add(factory.CompiledMethodNode(method), $"AsyncStateMachineBox Method on type {type.ToString()}");
+                                try
+                                {
+                                    factory.DetectGenericCycles(type, method);
+                                    dependencies.Add(factory.CompiledMethodNode(method), $"AsyncStateMachineBox Method on type {type.ToString()}");
+                                }
+                                catch (TypeSystemException)
+                                {
+                                }
                                 break;
                         }
                     }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs

@@ -188,7 +188,9 @@ public NodeFactory(
             ResourceData win32Resources,
             ReadyToRunFlags flags,
             NodeFactoryOptimizationFlags nodeFactoryOptimizationFlags,
-            ulong imageBase)
+            ulong imageBase,
+            int genericCycleDepthCutoff,
+            int genericCycleBreadthCutoff)
         {
             OptimizationFlags = nodeFactoryOptimizationFlags;
             TypeSystemContext = context;
@@ -216,6 +218,13 @@ public NodeFactory(
             }
 
             CreateNodeCaches();
+
+            if (genericCycleBreadthCutoff >= 0 || genericCycleDepthCutoff >= 0)
+            {
+                _genericCycleDetector = new LazyGenericsSupport.GenericCycleDetector(
+                    depthCutoff: genericCycleDepthCutoff,
+                    breadthCutoff: genericCycleBreadthCutoff);
+            }
         }
 
         private void CreateNodeCaches()
@@ -389,6 +398,8 @@ private void CreateNodeCaches()
 
         private NodeCache<ReadyToRunHelper, Import> _constructedHelpers;
 
+        private LazyGenericsSupport.GenericCycleDetector _genericCycleDetector;
+
         public Import GetReadyToRunHelperCell(ReadyToRunHelper helperId)
         {
             return _constructedHelpers.GetOrAdd(helperId);
@@ -1010,5 +1021,10 @@ public CopiedManagedResourcesNode CopiedManagedResources(EcmaModule module)
         {
             return _copiedManagedResources.GetOrAdd(module);
         }
+
+        public void DetectGenericCycles(TypeSystemEntity caller, TypeSystemEntity callee)
+        {
+            _genericCycleDetector?.DetectCycle(caller, callee);
+        }
     }
 }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs

@@ -106,6 +106,8 @@ public bool CanInline(MethodDesc caller, MethodDesc callee)
                 }
             }
 
+            _nodeFactory.DetectGenericCycles(caller, callee);
+
             return NodeFactory.CompilationModuleGroup.CanInline(caller, callee);
         }
 
@@ -426,7 +428,9 @@ private void RewriteComponentFile(string inputFile, string outputFile, string ow
                 win32Resources: new Win32Resources.ResourceData(inputModule),
                 flags,
                 _nodeFactory.OptimizationFlags,
-                _nodeFactory.ImageBase);
+                _nodeFactory.ImageBase,
+                genericCycleDepthCutoff: -1, // We don't need generic cycle detection when rewriting component assemblies
+                genericCycleBreadthCutoff: -1); // as we're not actually compiling anything
 
             IComparer<DependencyNodeCore<NodeFactory>> comparer = new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance);
             DependencyAnalyzerBase<NodeFactory> componentGraph = new DependencyAnalyzer<NoLogStrategy<NodeFactory>, NodeFactory>(componentFactory, comparer);