Refactor compilation module group (#1889)

- Our CompilationModule groups used for single method compilation and general compilation had diverged substantially, reducing the effectiveness of single method compilation substantially
- The profiler guided optimization logic also needs to use the VersionWithCode logic to identify which methods are interesting to consider, this will happen in a future change
- The VersionsWith logic in CompilationModuleGroup did not correctly model versionable logic.
  - Methods with generic parameters were not handled correctly. Method generic parameters did not contribute to the VersionsWith logic.
  - The newer ideas in our recent work on upcoming enhancements to crossgen2 driven compilation enhancements are aligned with this effort
  - Add support for the concept of considering instantiations over primitive types to be legal even if the primitive types aren't part of the version bubble. (Only if there are no constraints in the generic)

- Also a bonus determinism fix. In cases where MemberRef's were being used in combination with ENCODE_FIELD_SIG_OwnerType, we could possibly emit multiple different choices for the token emitting in a signature.

docs/design/features/crossgen2-compilation-structure-enhancements.md

+# Crossgen2 Driven Compilation structure enhancements
+-----
+
+Adding version bubbles and complex compilation rules to compilation in crossgen2 for .NET 5.
+
+This document describes the concept of a version bubble, how to specify them to the ahead of time compiler (crossgen2), and the effect of various options on compilation.
+
+## Behavior of code that shares a version bubble
+
+1. Inlining is only permitted within a version bubble unless the method is marked with System.Runtime.Versioning.NonVersionableAttribute. If the inlinee is marked as NonVersionable, it may ALWAYS be inlined into the method being compiled.
+2. Generic instantiations may only be ahead of time compiled into the application if the definition of the generic is defined within the version bubble as well as the instantiation arguments to the generic. As an exception to the requirement for the instantiation arguments to be defined within the bubble, the shared generic type `System.__Canon` will always be considered to be part of the version bubble. Also, the list of very well known types (object, string, int, uint, short, ushort, byte, sbyte, long, ulong, float, double, IntPtr, and UIntPtr) are also considered to be part of the version bubble as long as the generic is not constrained on an interface or class. For Example
+```
+class MyGeneric<T> {}
+class ConstrainedGeneric<T> where T : IEquatable<T> {}
+...
+//MyGeneric<int> would be always be in the version bubble where MyGeneric was defined.
+//ConstrainedGeneric<int> would only be in the version bubble of ConstrainedGeneric if ConstrainedGeneric shared a version bubble with System.Private.CoreLib.
+//MyGeneric<DateTime> would would only be in the version bubble of MyGeneric if MyGeneric shared a version bubble with System.Private.CoreLib.
+//ConstrainedGeneric<DateTime> would would only be in the version bubble of ConstrainedGeneric if ConstrainedGeneric shared a version bubble with System.Private.CoreLib.
+```
+
+## Behavior of code that shares a compilation group
+
+A compilation group is a set of assemblies that are compiled together. Typically in the customer case, this is a set of assemblies that is compiled in a similar timescale. In general, all assemblies compiled at once are considered to be part of the same version bubble, but in the case of the the core libraries and ASP.NET this isn't actually true. Both of these layers include assemblies which support replacement by either higher layers (in the case of the WinForms/WPF frameworks) or by the application (in the case of ASP.NET) 
+
+## Specifying a version bubble in a project file
+The end user developer will specify which version bubble an application is using with a notation such as the following in the project file of application.
+```
+<PropertyGroup>
+    <CompilationVersionBubble>XXX</CompilationVersionBubble>
+</PropertyGroup>
+```
+
+If `CompilationVersionBubble` is set to `IncludeFrameworks`, then the application will be compiled with a version bubble that includes the entire set of frameworks, and application. The framework in this scenario is the core-sdk concept of frameworks of which today we have the ASP.NET framework, WinForms, WPF, and the Core-Sdk. If the  property is `Application`, then the version bubble will be that of the application only. (Note that `Application` will require additional development effort in the runtime to support some new token resolution behavior, etc.) If the  property is `Assembly`, then the version bubble will be at the individual assembly level. (Note, naming here is theoretical and unvetted, this is simply the level of control proposed to give to typical developers.)
+
+The default value of the CompilationVersionBubble flag would be dependent on how the application is published. My expectation is that a standalone application will default to `IncludeFrameworks`, and that if we implement sufficient support for `Application` that will be the other default. Otherwise the default shall be `Assembly`. Interaction with Docker build scenarios is also quite interesting, I suspect we would like to enable `IncludeFrameworks` by default for Docker scenarios if possible.
+
+## Specifying the version bubble on the crossgen2 command line
+
+There are 3 sets of files to pass to crossgen2.
+
+1. The set of files that are referenceable by the compilation. These are specified via the --reference switch.
+2. The set of files that comprise the set of assemblies being compiled at once. These are specified via the --input-file-path switch. (The first assembly specified in this list describes module actually to be compiled and output. When we add support for composite R2R images, we will produce a single module file from all of these input assemblies at once.)
+3. The set of modules that comprise the version bubble tied to the assembly being compiled. The specification of this is complicated and unclear at this time. My current idea is to have two modes. Input bubble mode (where the set of input files matches the version bubble), and all mode (where all assemblies are considered to be part of the bubble. For framework ahead of time compilation scenarios such as containers and such where we may be compiling the framework and supporting scenarios where not all files in the framework are necessarily unified in the bubble, we may need an exclude function to exclude specific binaries.
+
+## Choice of what code to compile
+
+###Principles
+1. Ahead of time generated code exists to improve startup, and for some scenarios will be retained for the lifetime of the process.
+2. Our default scenario relies on on tiered compilation rejit for best performance.
+3. Too much pregenerated code will negatively affect applications. Startup is the critical detail for most pregeneration scenarios, and for those, there is a blend of time to pull the file to the CPU (from disk, over the network, etc.) and to compile functions. Striking the right blend has been discovered to be critical.
+
+###Proposed approach
+Note, this approach is probably more complete than we will finish in one release, but encompasses a large set of future vision in this space.
+
+For non-generic code this is straightforward. Either compile all the non-generic code in the binary, or compile only that which is specified via a profile guided optimization step. This choice shall be driven by a per "input assembly" switch as in the presence of a composite R2R image we likely will want to have different policy for different assemblies, as has proven valuable in the past. Until proven otherwise, per assembly specification of this behavior shall be considered to be sufficient.
+
+We shall set a guideline for how much generic code to generate, and the amount of generic code to generate shall be gated as a multiplier of the amount of non-generic code generated. 
+
+For generic code we also need a per assembly switch to adjust between various behaviors, but the proposal is as follows.
+
+1. We compile the non-generic code aggressively and we compile generic code specified via a profile guide aggressively. These compilations will ignore any generics size limitations.
+2. We compile the generic code directly referenced by the above logic. These compilations will be gated on fitting within the generics compilation budget.
+3. We compile generic code driven by heuristics. Intentionally we shall choose a small set of heuristics driven by customer data. A set of proposed heuristics is below, but we should determine which heuristics through experimentation, not decide them before the technology is developed.
+  - Precompilation of generic task infrastructure related to async state machines. (Only if CoreLib is part of the version bubble)
+  - Precompilation of MoveNext and Current methods for C# generated iterators
+  - Precompilation of interface methods used by LINQ expressions. (Don't force generation of all virtual functions on a type, just pick a subset related to common uses)
+  - Precompilation of statically discoverable cross-module instantiations for self-contained and docker scenarios if CompilationVersionBubble >= ‘Application’. Here, statically discoverable instantiations can be loaded from the typespecs and methodspecs in the metadata of the input files that comprise the version bubble tied to the assembly being compiled
+
+## Reducing generics duplication
+With the advent of a version bubble larger than a single binary and the ability to generate generics, comes the problem of managing the multiple copies of generic code that might be generated. 
+
+The traditional NGEN model was to greedily generate generic code everywhere and assume it wouldn't get out of hand. As generics have become more used and applications have become more broken down into many assemblies, this model has become less workable, and thus this model attempts to prevent such overgeneration.
+
+Application construction consists of 1 or more frameworks, which may be built and distributed independently from the application (docker container scenario) and the application.
+
+For instance,
+
+Application
+ASP.NET
+Runtime Layer
+
+Each layer in this stack will be compiled as a consistent set of crossgen2 compilations.
+
+I propose to reduce the generics duplication problem to allow duplication between layers, but not within a layer. There are two ways to do this. The first of which is to produce composite R2R images for a layer. Within a single composite R2R image generation, running heuristics and generating generics eagerly should be straightforward. This composite R2R image would have all instantiations statically computed that are local to that particular layer of compilation, and also any instantiations from other layers. The duplication problem would be reduced in that a single analysis would trigger these multi-layer dependent compilations, and so which there may be duplication between layers, there wouldn't be duplication within a layer. And given that the count of layers is not expected to exceed 3 or 4, that duplication will not be a major concern. 
+
+The second approach is to split compilation up into assembly level units, run the heuristics per assembly, generate the completely local generics in the individual assemblies, and then nominate a final mop up assembly that consumes a series of data files produced by the individual assembly compilations and holds all of the stuff that didn't make sense in the individual assemblies. In my opinion this second approach would be better for debug builds, but the first approach is strictly better for release builds, and really shouldn't be terribly slow.
+
+# Proposal
+Address the reducing generics duplication concern by implementing composite R2R files instead of attempting to build a multifile approach, where each layer is expected to know the exact layer above, or be fully R2R with respect to all layers above. Loading of R2R images built with version bubble dependence will lazily verify that version rules are not violated, and produce a FailFast if version bubble usage rules are used incorrectly. For customers which produce customized versions of the underlying frameworks, if any differences are present they will likely be forced to provide their own targeting packs to use the fully AOT layered scenarios.
+
+## Expected Benefits of this design 
+1. Allow for best performance standalone applications. (Primarily targeting the hyperscale scenario. Benefits for regular UI standalone applications would also likely be present, but not significant enough to drive this effort.)
+2. Support a layered Docker image model for high performance smaller applications. This is driven by the presence of a generics budget that is application wide (to allow customers to control distribution size of applications, as well as the ability to capture a really good set of precompiled generics to optimize startup time. The biggest precompiled wins here are the ability to AOT compile portions of CoreLib into these applications such as appropriate parts of the task infrastructure, as well as collections.)
+3.  Allows use of proven technology (profile guided optimization) to drive generics compilation for high value customers, while also provided an opportunity for innovation in heuristics.
+
+## Issues to resolve with this proposal
+1. This proposal needs to be squared with our servicing policies and ensure that we aren't affecting preventing promised servicing capabilities.
+  - In particular, there has been a concept in the past about making self contained applications serviceable through a machine wide policy key. We would need to design what our plans are here.
\ No newline at end of file

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs

@@ -219,7 +219,9 @@ public virtual void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder
         protected override string GetName(NodeFactory factory)
         {
             Utf8StringBuilder sb = new Utf8StringBuilder();
+            sb.Append("MethodWithGCInfo(");
             AppendMangledName(factory.NameMangler, sb);
+            sb.Append(")");
             return sb.ToString();
         }
 

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleTokenResolver.cs

@@ -169,7 +169,7 @@ public void AddModuleTokenForField(FieldDesc field, ModuleToken token)
                 canonField = CompilerContext.GetFieldForInstantiatedType(field.GetTypicalFieldDefinition(), (InstantiatedType)owningCanonType);
             }
 
-            _fieldToRefTokens[canonField] = token;
+            SetModuleTokenForTypeSystemEntity(_fieldToRefTokens, canonField, token);
 
             switch (token.TokenType)
             {
@@ -182,6 +182,25 @@ public void AddModuleTokenForField(FieldDesc field, ModuleToken token)
             }
         }
 
+        // Add TypeSystemEntity -> ModuleToken mapping to a ConcurrentDictionary. Using CompareTo sort the token used, so it will
+        // be consistent in all runs of the compiler
+        void SetModuleTokenForTypeSystemEntity<T>(ConcurrentDictionary<T, ModuleToken> dictionary, T tse, ModuleToken token)
+        {
+            if (!dictionary.TryAdd(tse, token))
+            {
+                ModuleToken oldToken;
+                do
+                {
+                    // We will reach here, if the field already has a token
+                    if (!dictionary.TryGetValue(tse, out oldToken))
+                        throw new InternalCompilerErrorException("TypeSystemEntity both present and not present in emission dictionary.");
+
+                    if (oldToken.CompareTo(token) <= 0)
+                        break;
+                } while (dictionary.TryUpdate(tse, token, oldToken));
+            }
+        }
+
         public void AddModuleTokenForType(TypeDesc type, ModuleToken token)
         {
             bool specialTypeFound = false;
@@ -205,7 +224,7 @@ public void AddModuleTokenForType(TypeDesc type, ModuleToken token)
                 // Don't store typespec tokens where a generic parameter resolves to the type in question
                 if (token.TokenType == CorTokenType.mdtTypeDef || token.TokenType == CorTokenType.mdtTypeRef)
                 {
-                    _typeToRefTokens[ecmaType] = token;
+                    SetModuleTokenForTypeSystemEntity(_typeToRefTokens, ecmaType, token);
                 }
             }
             else if (!specialTypeFound)

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using Internal.TypeSystem;
+using Internal.TypeSystem.Ecma;
+using Internal.TypeSystem.Interop;
+using ILCompiler.DependencyAnalysis.ReadyToRun;
+using Debug = System.Diagnostics.Debug;
+using System;
+
+namespace ILCompiler
+{
+    public abstract class ReadyToRunCompilationModuleGroupBase : CompilationModuleGroup
+    {
+        protected readonly HashSet<ModuleDesc> _compilationModuleSet;
+        private readonly HashSet<ModuleDesc> _versionBubbleModuleSet;
+        private Dictionary<TypeDesc, ModuleToken> _typeRefsInCompilationModuleSet;
+        private readonly bool _compileGenericDependenciesFromVersionBubbleModuleSet;
+        private readonly ConcurrentDictionary<TypeDesc, bool> _containsTypeLayoutCache = new ConcurrentDictionary<TypeDesc, bool>();
+        private readonly ConcurrentDictionary<TypeDesc, bool> _versionsWithTypeCache = new ConcurrentDictionary<TypeDesc, bool>();
+        private readonly ConcurrentDictionary<MethodDesc, bool> _versionsWithMethodCache = new ConcurrentDictionary<MethodDesc, bool>();
+
+        public ReadyToRunCompilationModuleGroupBase(
+            TypeSystemContext context,
+            IEnumerable<ModuleDesc> compilationModuleSet,
+            IEnumerable<ModuleDesc> versionBubbleModuleSet,
+            bool compileGenericDependenciesFromVersionBubbleModuleSet)
+        {
+            _compilationModuleSet = new HashSet<ModuleDesc>(compilationModuleSet);
+
+            _versionBubbleModuleSet = new HashSet<ModuleDesc>(versionBubbleModuleSet);
+            _versionBubbleModuleSet.UnionWith(_compilationModuleSet);
+
+            _compileGenericDependenciesFromVersionBubbleModuleSet = compileGenericDependenciesFromVersionBubbleModuleSet;
+        }
+
+        public sealed override bool ContainsType(TypeDesc type)
+        {
+            return type.GetTypeDefinition() is EcmaType ecmaType && IsModuleInCompilationGroup(ecmaType.EcmaModule);
+        }
+
+        private bool IsModuleInCompilationGroup(EcmaModule module)
+        {
+            return _compilationModuleSet.Contains(module);
+        }
+
+        protected bool CompileVersionBubbleGenericsIntoCurrentModule(MethodDesc method)
+        {
+            if (!_compileGenericDependenciesFromVersionBubbleModuleSet)
+                return false;
+
+            if (!VersionsWithMethodBody(method))
+                return false;
+
+            if (!method.HasInstantiation && !method.OwningType.HasInstantiation)
+                return false;
+
+            return true;
+        }
+
+        /// <summary>
+        /// If true, the type is fully contained in the current compilation group.
+        /// </summary>
+        public override bool ContainsTypeLayout(TypeDesc type)
+        {
+            return _containsTypeLayoutCache.GetOrAdd(type, ContainsTypeLayoutUncached);
+        }
+
+        private bool ContainsTypeLayoutUncached(TypeDesc type)
+        {
+            if (type.IsObject ||
+                type.IsPrimitive ||
+                type.IsEnum ||
+                type.IsPointer ||
+                type.IsFunctionPointer ||
+                type.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+            {
+                return true;
+            }
+            var defType = (MetadataType)type;
+            if (!ContainsType(defType))
+            {
+                if (!defType.IsValueType)
+                {
+                    // Eventually, we may respect the non-versionable attribute for reference types too. For now, we are going
+                    // to play is safe and ignore it.
+                    return false;
+                }
+
+                // Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
+                if (!defType.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute"))
+                {
+                    return false;
+                }
+            }
+            if (!defType.IsValueType && !ContainsTypeLayout(defType.BaseType))
+            {
+                return false;
+            }
+            foreach (FieldDesc field in defType.GetFields())
+            {
+                if (field.IsStatic)
+                    continue;
+
+                TypeDesc fieldType = field.FieldType;
+
+                if (fieldType.IsValueType &&
+                    !ContainsTypeLayout(fieldType))
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        public sealed override bool VersionsWithModule(ModuleDesc module)
+        {
+            return _versionBubbleModuleSet.Contains(module);
+        }
+
+        public sealed override bool VersionsWithType(TypeDesc typeDesc)
+        {
+            return typeDesc.GetTypeDefinition() is EcmaType ecmaType &&
+                _versionsWithTypeCache.GetOrAdd(typeDesc, ComputeTypeVersionsWithCode);
+        }
+
+
+        public sealed override bool VersionsWithMethodBody(MethodDesc method)
+        {
+            return _versionsWithMethodCache.GetOrAdd(method, VersionsWithMethodUncached);
+        }
+
+        private bool VersionsWithMethodUncached(MethodDesc method)
+        {
+            if (method.OwningType is MetadataType owningType)
+            {
+                if (!VersionsWithType(owningType))
+                    return false;
+            }
+            else
+                return false;
+
+            if (method == method.GetMethodDefinition())
+                return true;
+
+            return ComputeInstantiationVersionsWithCode(method.Instantiation, method);
+        }
+
+        public sealed override bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod)
+        {
+            // Allow inlining if the caller is within the current version bubble
+            // (because otherwise we may not be able to encode its tokens)
+            // and if the callee is either in the same version bubble or is marked as non-versionable.
+            bool canInline = VersionsWithMethodBody(callerMethod) &&
+                (VersionsWithMethodBody(calleeMethod) ||
+                    calleeMethod.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute"));
+
+            return canInline;
+        }
+
+        public sealed override bool GeneratesPInvoke(MethodDesc method)
+        {
+            // PInvokes depend on details of the core library, so for now only compile them if:
+            //    1) We're compiling the core library module, or
+            //    2) We're compiling any module, and no marshalling is needed
+            //
+            // TODO Future: consider compiling PInvokes with complex marshalling in version bubble
+            // mode when the core library is included in the bubble.
+
+            Debug.Assert(method is EcmaMethod);
+
+            // If the PInvoke is declared on an external module, we can only compile it if
+            // that module is part of the version bubble.
+            if (!_versionBubbleModuleSet.Contains(((EcmaMethod)method).Module))
+                return false;
+
+            if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule))
+                return true;
+
+            return !Marshaller.IsMarshallingRequired(method);
+        }
+
+        public sealed override bool TryGetModuleTokenForExternalType(TypeDesc type, out ModuleToken token)
+        {
+            Debug.Assert(!VersionsWithType(type));
+
+            if (_typeRefsInCompilationModuleSet == null)
+            {
+                _typeRefsInCompilationModuleSet = new Dictionary<TypeDesc, ModuleToken>();
+
+                foreach (var module in _compilationModuleSet)
+                {
+                    EcmaModule ecmaModule = (EcmaModule)module;
+                    foreach (var typeRefHandle in ecmaModule.MetadataReader.TypeReferences)
+                    {
+                        try
+                        {
+                            TypeDesc typeFromTypeRef = ecmaModule.GetType(typeRefHandle);
+                            if (!_typeRefsInCompilationModuleSet.ContainsKey(typeFromTypeRef))
+                            {
+                                _typeRefsInCompilationModuleSet.Add(typeFromTypeRef, new ModuleToken(ecmaModule, typeRefHandle));
+                            }
+                        }
+                        catch (TypeSystemException) { }
+                    }
+                }
+            }
+
+            return _typeRefsInCompilationModuleSet.TryGetValue(type, out token);
+        }
+
+        private bool ComputeTypeVersionsWithCode(TypeDesc type)
+        {
+            if (type.IsCanonicalDefinitionType(CanonicalFormKind.Any))
+                return true;
+
+            if (type is MetadataType mdType)
+            {
+                if (!_versionBubbleModuleSet.Contains(mdType.Module))
+                    return false;
+            }
+
+            if (type == type.GetTypeDefinition())
+                return true;
+
+            return ComputeInstantiationVersionsWithCode(type.Instantiation, type);
+        }
+
+        private bool ComputeInstantiationVersionsWithCode(Instantiation inst, TypeSystemEntity entityWithInstantiation)
+        {
+            for (int iInstantiation = 0; iInstantiation < inst.Length; iInstantiation++)
+            {
+                TypeDesc instType = inst[iInstantiation];
+
+                if (!ComputeInstantiationTypeVersionsWithCode(this, instType))
+                {
+                    if (instType.IsPrimitive)
+                    {
+                        // Primitive type instantiations are only instantiated in the module of the generic defining type
+                        // if the generic does not apply interface constraints to that type parameter, or if System.Private.CoreLib is part of the version bubble
+
+                        Instantiation entityDefinitionInstantiation;
+                        if (entityWithInstantiation is TypeDesc type)
+                        {
+                            entityDefinitionInstantiation = type.GetTypeDefinition().Instantiation;
+                        }
+                        else
+                        {
+                            entityDefinitionInstantiation = ((MethodDesc)entityWithInstantiation).GetTypicalMethodDefinition().Instantiation;
+                        }
+
+                        GenericParameterDesc genericParam = (GenericParameterDesc)entityDefinitionInstantiation[iInstantiation];
+                        if (genericParam.HasReferenceTypeConstraint)
+                            return false;
+
+                        // This checks to see if the type constraints list is empty
+                        if (genericParam.TypeConstraints.GetEnumerator().MoveNext())
+                            return false;
+                    }
+                    else
+                    {
+                        // Non-primitive which doesn't version with type implies instantiation doesn't version with type
+                        return false;
+                    }
+                }
+            }
+            return true;
+
+            static bool ComputeInstantiationTypeVersionsWithCode(ReadyToRunCompilationModuleGroupBase compilationGroup, TypeDesc type)
+            {
+                if (type == type.Context.CanonType)
+                    return true;
+
+                if (compilationGroup.VersionsWithType(type))
+                    return true;
+
+                if (type.IsArray)
+                    return ComputeInstantiationTypeVersionsWithCode(compilationGroup, type.GetParameterType());
+
+                if (type.IsPointer)
+                    return ComputeInstantiationTypeVersionsWithCode(compilationGroup, type.GetParameterType());
+
+                return false;
+            }
+        }
+
+        public abstract void ApplyProfilerGuidedCompilationRestriction(ProfileDataManager profileGuidedCompileRestriction);
+    }
+}

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunSingleAssemblyCompilationModuleGroup.cs

@@ -12,38 +12,28 @@
 
 namespace ILCompiler
 {
-    public class ReadyToRunSingleAssemblyCompilationModuleGroup : CompilationModuleGroup
+    public class ReadyToRunSingleAssemblyCompilationModuleGroup : ReadyToRunCompilationModuleGroupBase
     {
-        private HashSet<ModuleDesc> _compilationModuleSet;
-        private HashSet<ModuleDesc> _versionBubbleModuleSet;
         private ProfileDataManager _profileGuidedCompileRestriction;
-        private Dictionary<TypeDesc, ModuleToken> _typeRefsInCompilationModuleSet;
-
-        private bool _compileGenericDependenciesFromVersionBubbleModuleSet;
+        private bool _profileGuidedCompileRestrictionSet;
 
         public ReadyToRunSingleAssemblyCompilationModuleGroup(
             TypeSystemContext context, 
             IEnumerable<ModuleDesc> compilationModuleSet,
             IEnumerable<ModuleDesc> versionBubbleModuleSet,
-            bool compileGenericDependenciesFromVersionBubbleModuleSet,
-            ProfileDataManager profileGuidedCompileRestriction)
-        {
-            _compilationModuleSet = new HashSet<ModuleDesc>(compilationModuleSet);
-
-            _versionBubbleModuleSet = new HashSet<ModuleDesc>(versionBubbleModuleSet);
-            _versionBubbleModuleSet.UnionWith(_compilationModuleSet);
-
-            _compileGenericDependenciesFromVersionBubbleModuleSet = compileGenericDependenciesFromVersionBubbleModuleSet;
-            _profileGuidedCompileRestriction = profileGuidedCompileRestriction;
-        }
-
-        public sealed override bool ContainsType(TypeDesc type)
+            bool compileGenericDependenciesFromVersionBubbleModuleSet) :
+                base(context,
+                     compilationModuleSet,
+                     versionBubbleModuleSet,
+                     compileGenericDependenciesFromVersionBubbleModuleSet)
         {
-            return type.GetTypeDefinition() is EcmaType ecmaType && IsModuleInCompilationGroup(ecmaType.EcmaModule);
         }
 
         public sealed override bool ContainsMethodBody(MethodDesc method, bool unboxingStub)
         {
+            if (!_profileGuidedCompileRestrictionSet)
+                throw new InternalCompilerErrorException("Called ContainsMethodBody without setting profile guided restriction");
+
             if (_profileGuidedCompileRestriction != null)
             {
                 bool found = false;
@@ -84,161 +74,16 @@ public sealed override bool ContainsMethodBody(MethodDesc method, bool unboxingS
                 return false;
             }
 
-            return ContainsType(method.OwningType) || CompileVersionBubbleGenericsIntoCurrentModule(method);
-
-        }
-
-        private bool IsModuleInCompilationGroup(EcmaModule module)
-        {
-            return _compilationModuleSet.Contains(module);
-        }
-
-        private bool CompileVersionBubbleGenericsIntoCurrentModule(MethodDesc method)
-        {
-            if (!_compileGenericDependenciesFromVersionBubbleModuleSet)
-                return false;
-
-            if (!method.HasInstantiation && !method.OwningType.HasInstantiation)
-                return false;
-
-            return VersionsWithType(method.OwningType);
-        }
-
-        private ConcurrentDictionary<TypeDesc, bool> _containsTypeLayoutCache = new ConcurrentDictionary<TypeDesc, bool>();
-
-        /// <summary>
-        /// If true, the type is fully contained in the current compilation group.
-        /// </summary>
-        public override bool ContainsTypeLayout(TypeDesc type)
-        {
-            return _containsTypeLayoutCache.GetOrAdd(type, ContainsTypeLayoutUncached);
-        }
-
-        private bool ContainsTypeLayoutUncached(TypeDesc type)
-        {
-            if (type.IsObject || 
-                type.IsPrimitive || 
-                type.IsEnum || 
-                type.IsPointer ||
-                type.IsFunctionPointer ||
-                type.IsCanonicalDefinitionType(CanonicalFormKind.Any))
-            {
-                return true;
-            }
-            var defType = (MetadataType)type;
-            if (!ContainsType(defType))
-            {
-                if (!defType.IsValueType)
-                {
-                    // Eventually, we may respect the non-versionable attribute for reference types too. For now, we are going
-                    // to play is safe and ignore it.
-                    return false;
-                }
-
-                // Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
-                if (!defType.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute"))
-                {
-                    return false;
-                }
-            }
-            if (!defType.IsValueType && !ContainsTypeLayout(defType.BaseType))
-            {
-                return false;
-            }
-            foreach (FieldDesc field in defType.GetFields())
-            {
-                if (field.IsStatic)
-                    continue;
-
-                TypeDesc fieldType = field.FieldType;
-
-                if (fieldType.IsValueType && 
-                    !ContainsTypeLayout(fieldType))
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
-
-        public override bool VersionsWithModule(ModuleDesc module)
-        {
-            return _versionBubbleModuleSet.Contains(module);
-        }
-
-        public override bool VersionsWithType(TypeDesc typeDesc)
-        {
-            return typeDesc.GetTypeDefinition() is EcmaType ecmaType &&
-                _versionBubbleModuleSet.Contains(ecmaType.EcmaModule);
-        }
-
-        public override bool VersionsWithMethodBody(MethodDesc method)
-        {
-            return VersionsWithType(method.OwningType);
-        }
-
-        public override bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod)
-        {
-            // Allow inlining if the caller is within the current version bubble
-            // (because otherwise we may not be able to encode its tokens)
-            // and if the callee is either in the same version bubble or is marked as non-versionable.
-            bool canInline = VersionsWithMethodBody(callerMethod) &&
-                (VersionsWithMethodBody(calleeMethod) ||
-                    calleeMethod.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute"));
-
-            return canInline;
-        }
-
-        public override bool GeneratesPInvoke(MethodDesc method)
-        {
-            // PInvokes depend on details of the core library, so for now only compile them if:
-            //    1) We're compiling the core library module, or
-            //    2) We're compiling any module, and no marshalling is needed
-            //
-            // TODO Future: consider compiling PInvokes with complex marshalling in version bubble
-            // mode when the core library is included in the bubble.
-
-            Debug.Assert(method is EcmaMethod);
-
-            // If the PInvoke is declared on an external module, we can only compile it if
-            // that module is part of the version bubble.
-            if (!_versionBubbleModuleSet.Contains(((EcmaMethod)method).Module))
-                return false;
-
-            if (((EcmaMethod)method).Module.Equals(method.Context.SystemModule))
-                return true;
-
-            return !Marshaller.IsMarshallingRequired(method);
+            return (ContainsType(method.OwningType) && VersionsWithMethodBody(method)) || CompileVersionBubbleGenericsIntoCurrentModule(method);
         }
 
-        public override bool TryGetModuleTokenForExternalType(TypeDesc type, out ModuleToken token)
+        public sealed override void ApplyProfilerGuidedCompilationRestriction(ProfileDataManager profileGuidedCompileRestriction)
         {
-            Debug.Assert(!VersionsWithType(type));
-
-            if (_typeRefsInCompilationModuleSet == null)
-            {
-                _typeRefsInCompilationModuleSet = new Dictionary<TypeDesc, ModuleToken>();
-
-                foreach (var module in _compilationModuleSet)
-                {
-                    EcmaModule ecmaModule = (EcmaModule)module;
-                    foreach (var typeRefHandle in ecmaModule.MetadataReader.TypeReferences)
-                    {
-                        try
-                        {
-                            TypeDesc typeFromTypeRef = ecmaModule.GetType(typeRefHandle);
-                            if (!_typeRefsInCompilationModuleSet.ContainsKey(typeFromTypeRef))
-                            {
-                                _typeRefsInCompilationModuleSet.Add(typeFromTypeRef, new ModuleToken(ecmaModule, typeRefHandle));
-                            }
-                        }
-                        catch (TypeSystemException) { }
-                    }
-                }
-            }
+            if (_profileGuidedCompileRestrictionSet)
+                throw new InternalCompilerErrorException("Called ApplyProfilerGuidedCompilationRestriction twice.");
 
-            return _typeRefsInCompilationModuleSet.TryGetValue(type, out token);
+            _profileGuidedCompileRestrictionSet = true;
+            _profileGuidedCompileRestriction = profileGuidedCompileRestriction;
         }
     }
 }

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/SingleMethodCompilationModuleGroup.cs

@@ -2,12 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Diagnostics;
 using System.Collections.Generic;
 
 using Internal.TypeSystem;
-using Internal.TypeSystem.Ecma;
-using ILCompiler.DependencyAnalysis.ReadyToRun;
 
 namespace ILCompiler
 {
@@ -15,12 +12,20 @@ namespace ILCompiler
     /// A compilation group that only contains a single method. Useful for development purposes when investigating
     /// code generation issues.
     /// </summary>
-    public class SingleMethodCompilationModuleGroup : CompilationModuleGroup
+    public class SingleMethodCompilationModuleGroup : ReadyToRunCompilationModuleGroupBase
     {
         private MethodDesc _method;
-        private Dictionary<TypeDesc, ModuleToken> _typeRefsInCompilationModuleSet;
 
-        public SingleMethodCompilationModuleGroup(MethodDesc method)
+        public SingleMethodCompilationModuleGroup(
+            TypeSystemContext context,
+            IEnumerable<ModuleDesc> compilationModuleSet,
+            IEnumerable<ModuleDesc> versionBubbleModuleSet,
+            bool compileGenericDependenciesFromVersionBubbleModuleSet,
+            MethodDesc method) :
+                base(context,
+                     compilationModuleSet,
+                     versionBubbleModuleSet,
+                     compileGenericDependenciesFromVersionBubbleModuleSet)
         {
             _method = method;
         }
@@ -30,42 +35,10 @@ public override bool ContainsMethodBody(MethodDesc method, bool unboxingStub)
             return method == _method;
         }
 
-        public override bool ContainsType(TypeDesc type)
+        public override void ApplyProfilerGuidedCompilationRestriction(ProfileDataManager profileGuidedCompileRestriction)
         {
-            return type == _method.OwningType;
-        }
-
-        public override bool VersionsWithModule(ModuleDesc module)
-        {
-            return ((EcmaMethod)_method.GetTypicalMethodDefinition()).Module == module;
-        }
-
-        public override bool GeneratesPInvoke(MethodDesc method)
-        {
-            return true;
-        }
-
-        public override bool TryGetModuleTokenForExternalType(TypeDesc type, out ModuleToken token)
-        {
-            Debug.Assert(!VersionsWithType(type));
-
-            if (_typeRefsInCompilationModuleSet == null)
-            {
-                _typeRefsInCompilationModuleSet = new Dictionary<TypeDesc, ModuleToken>();
-
-                EcmaModule ecmaModule = ((EcmaMethod)_method.GetTypicalMethodDefinition()).Module;
-                foreach (var typeRefHandle in ecmaModule.MetadataReader.TypeReferences)
-                {
-                    try
-                    {
-                        TypeDesc typeFromTypeRef = ecmaModule.GetType(typeRefHandle);
-                        _typeRefsInCompilationModuleSet[typeFromTypeRef] = new ModuleToken(ecmaModule, typeRefHandle);
-                    }
-                    catch (TypeSystemException) { }
-                }
-            }
-
-            return _typeRefsInCompilationModuleSet.TryGetValue(type, out token);
+            // Profiler guided restrictions are ignored for single method compilation
+            return;
         }
     }
 }

src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

@@ -172,6 +172,7 @@
     <Compile Include="Compiler\DependencyAnalysis\TypeAndMethod.cs" />
     <Compile Include="Compiler\IRootingServiceProvider.cs" />
     <Compile Include="Compiler\MethodExtensions.cs" />
+    <Compile Include="Compiler\ReadyToRunCompilationModuleGroupBase.cs" />
     <Compile Include="IBC\IBCDataModel.cs" />
     <Compile Include="IBC\IBCDataReader.cs" />
     <Compile Include="IBC\ReaderExtensions.cs" />

src/coreclr/src/tools/crossgen2/crossgen2/Program.cs

@@ -185,12 +185,14 @@ private int Run()
                     //  typeSystemContext.InputFilePaths = _inputFilePaths;
                     //
                     Dictionary<string, string> inputFilePaths = new Dictionary<string, string>();
+                    List<ModuleDesc> referenceableModules = new List<ModuleDesc>();
                     foreach (var inputFile in _inputFilePaths)
                     {
                         try
                         {
                             var module = typeSystemContext.GetModuleFromPath(inputFile.Value);
                             inputFilePaths.Add(inputFile.Key, inputFile.Value);
+                            referenceableModules.Add(module);
                         }
                         catch (TypeSystemException.BadImageFormatException)
                         {
@@ -201,6 +203,21 @@ private int Run()
                     typeSystemContext.InputFilePaths = inputFilePaths;
                     typeSystemContext.ReferenceFilePaths = _referenceFilePaths;
 
+                    List<EcmaModule> inputModules = new List<EcmaModule>();
+                    HashSet<ModuleDesc> versionBubbleModulesHash = new HashSet<ModuleDesc>();
+
+                    foreach (var inputFile in typeSystemContext.InputFilePaths)
+                    {
+                        EcmaModule module = typeSystemContext.GetModuleFromPath(inputFile.Value);
+                        inputModules.Add(module);
+                        versionBubbleModulesHash.Add(module);
+
+                        if (!_commandLineOptions.InputBubble)
+                        {
+                            break;
+                        }
+                    }
+
                     string systemModuleName = _commandLineOptions.SystemModule ?? DefaultSystemModule;
                     typeSystemContext.SetSystemModule(typeSystemContext.GetModuleForSimpleName(systemModuleName));
 
@@ -216,33 +233,29 @@ private int Run()
 
                     var logger = new Logger(Console.Out, _commandLineOptions.Verbose);
 
-                    List<ModuleDesc> referenceableModules = new List<ModuleDesc>();
-                    foreach (var inputFile in inputFilePaths)
-                    {
-                        try
-                        {
-                            referenceableModules.Add(typeSystemContext.GetModuleFromPath(inputFile.Value));
-                        }
-                        catch { } // Ignore non-managed pe files
-                    }
-
                     foreach (var referenceFile in _referenceFilePaths.Values)
                     {
                         try
                         {
-                            referenceableModules.Add(typeSystemContext.GetModuleFromPath(referenceFile));
+                            EcmaModule module = typeSystemContext.GetModuleFromPath(referenceFile);
+                            referenceableModules.Add(module);
+                            if (_commandLineOptions.InputBubble)
+                            {
+                                // In large version bubble mode add reference paths to the compilation group
+                                versionBubbleModulesHash.Add(module);
+                            }
                         }
                         catch { } // Ignore non-managed pe files
                     }
 
-                    ProfileDataManager profileDataManager = new ProfileDataManager(logger, referenceableModules);
+                    List<ModuleDesc> versionBubbleModules = new List<ModuleDesc>(versionBubbleModulesHash);
 
-                    CompilationModuleGroup compilationGroup;
+                    ReadyToRunCompilationModuleGroupBase compilationGroup;
                     List<ICompilationRootProvider> compilationRoots = new List<ICompilationRootProvider>();
                     if (singleMethod != null)
                     {
                         // Compiling just a single method
-                        compilationGroup = new SingleMethodCompilationModuleGroup(singleMethod);
+                        compilationGroup = new SingleMethodCompilationModuleGroup(typeSystemContext, inputModules, versionBubbleModules, _commandLineOptions.CompileBubbleGenerics, singleMethod);
                         compilationRoots.Add(new SingleMethodRootProvider(singleMethod));
                     }
                     else
@@ -261,44 +274,32 @@ private int Run()
                             }
                         }
 
-                        List<EcmaModule> inputModules = new List<EcmaModule>();
+                        compilationGroup = new ReadyToRunSingleAssemblyCompilationModuleGroup(
+                            typeSystemContext, inputModules, versionBubbleModules, _commandLineOptions.CompileBubbleGenerics);
+                    }
 
-                        foreach (var inputFile in typeSystemContext.InputFilePaths)
+                    // Examine profile guided information as appropriate
+                    ProfileDataManager profileDataManager =
+                        new ProfileDataManager(logger,
+                        referenceableModules);
+
+                    if (_commandLineOptions.Partial)
+                        compilationGroup.ApplyProfilerGuidedCompilationRestriction(profileDataManager);
+                    else
+                        compilationGroup.ApplyProfilerGuidedCompilationRestriction(null);
+
+                    if (singleMethod == null)
+                    {
+                        // For non-single-method compilations add compilation roots.
+                        foreach (var module in inputModules)
                         {
-                            EcmaModule module = typeSystemContext.GetModuleFromPath(inputFile.Value);
                             compilationRoots.Add(new ReadyToRunRootProvider(module, profileDataManager));
-                            inputModules.Add(module);
 
                             if (!_commandLineOptions.InputBubble)
                             {
                                 break;
                             }
                         }
-
-
-                        List<ModuleDesc> versionBubbleModules = new List<ModuleDesc>();
-                        if (_commandLineOptions.InputBubble)
-                        {
-                            // In large version bubble mode add reference paths to the compilation group
-                            foreach (string referenceFile in _referenceFilePaths.Values)
-                            {
-                                try
-                                {
-                                    // Currently SimpleTest.targets has no easy way to filter out non-managed assemblies
-                                    // from the reference list.
-                                    EcmaModule module = typeSystemContext.GetModuleFromPath(referenceFile);
-                                    versionBubbleModules.Add(module);
-                                }
-                                catch (TypeSystemException.BadImageFormatException ex)
-                                {
-                                    Console.WriteLine("Warning: cannot open reference assembly '{0}': {1}", referenceFile, ex.Message);
-                                }
-                            }
-                        }
-
-                        compilationGroup = new ReadyToRunSingleAssemblyCompilationModuleGroup(
-                            typeSystemContext, inputModules, versionBubbleModules, _commandLineOptions.CompileBubbleGenerics,
-                            _commandLineOptions.Partial ? profileDataManager : null);
                     }
 
                     //

src/coreclr/tests/src/readytorun/crossgen2/Program.cs

@@ -1155,6 +1155,40 @@ private static bool ObjectGetTypeOnGenericParamTest()
         return true;
     }
 
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static string ObjectToStringOnGenericParamTestWorker<T>(ref T t)
+    {
+        return t.ToString();
+    }
+
+    struct LocallyDefinedStructWithToString
+    {
+        public object StoredValue;
+        public override string ToString()
+        {
+            StoredValue = new object();
+            return "LocallyDefined";
+        }
+    }
+
+    private static bool ObjectToStringOnGenericParamTestSByte()
+    {
+        sbyte intVal = 42;
+        string returnedString = ObjectToStringOnGenericParamTestWorker<sbyte>(ref intVal);
+        if (returnedString != "42") return false;
+        return true;
+    }
+
+    private static bool ObjectToStringOnGenericParamTestVersionBubbleLocalStruct()
+    {
+        LocallyDefinedStructWithToString versionBubbleLocalStruct = new LocallyDefinedStructWithToString();
+        string returnedString = ObjectToStringOnGenericParamTestWorker(ref versionBubbleLocalStruct);
+        if (returnedString != "LocallyDefined") return false;
+        if (versionBubbleLocalStruct.StoredValue == null) return false; // ToString method should update struct in place.
+
+        return true;
+    }
+
     private static string EmitTextFileForTesting()
     {
         string file = Path.GetTempFileName();
@@ -1219,6 +1253,8 @@ public static int Main(string[] args)
         RunTest("GVMTest", GVMTest());
         RunTest("RuntimeMethodHandle", RuntimeMethodHandle());
         RunTest("ObjectGetTypeOnGenericParamTest", ObjectGetTypeOnGenericParamTest());
+        RunTest("ObjectToStringOnGenericParamTestSByte", ObjectToStringOnGenericParamTestSByte());
+        RunTest("ObjectToStringOnGenericParamTestVersionBubbleLocalStruct", ObjectToStringOnGenericParamTestVersionBubbleLocalStruct());
 
         File.Delete(TextFileName);