Crossgen2 fixes to enable composite build with shared framework (#34431)
authorTomáš Rylek <trylek@microsoft.com>
Fri, 3 Apr 2020 17:48:08 +0000 (19:48 +0200)
committerGitHub <noreply@github.com>
Fri, 3 Apr 2020 17:48:08 +0000 (19:48 +0200)
These changes involve targeted fixes to issues seen due to the new
build mode and generic signature encoding fixes identified in
my offline investigation with JanV. While I locally see the tests
passing with just a handful of failures, I still seem unable to
make the tests run in the lab; I keep investigating that but
I believe it's useful to merge this delta in to let us parallelize
follow-up efforts.

Thanks

Tomas

src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InliningInfoNode.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureContext.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/MethodExtensions.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs
src/coreclr/src/tools/crossgen2/crossgen2/Program.cs

index 03fe687..86f4693 100644 (file)
@@ -502,5 +502,15 @@ namespace ILCompiler
 
             return constrainedType ?? genericParam.Context.GetWellKnownType(WellKnownType.Object);
         }
+
+        /// <summary>
+        /// Return true when the type in question is marked with the NonVersionable attribute.
+        /// </summary>
+        /// <param name="type">Type to check</param>
+        /// <returns>True when the type is marked with the non-versionable custom attribute, false otherwise.</returns>
+        public static bool IsNonVersionable(this MetadataType type)
+        {
+            return type.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
+        }
     }
 }
index 89eb57a..e9f62a8 100644 (file)
@@ -66,6 +66,15 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                         continue;
                     }
 
+                    if (!factory.CompilationModuleGroup.VersionsWithMethodBody(inlinee))
+                    {
+                        // We cannot record inlining info across version bubble as cross-bubble assemblies
+                        // are not guaranteed to preserve token values. Only non-versionable methods may be
+                        // inlined across the version bubble.
+                        Debug.Assert(inlinee.IsNonVersionable());
+                        continue;
+                    }
+
                     if (!inlineeToInliners.TryGetValue(ecmaInlineeDefinition, out HashSet<EcmaMethod> inliners))
                     {
                         inliners = new HashSet<EcmaMethod>();
index 4d76fb7..a61fea8 100644 (file)
@@ -310,6 +310,12 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 case TypeFlags.ValueType:
                 case TypeFlags.Nullable:
                 case TypeFlags.Enum:
+                    if (typeDesc.IsWellKnownType(WellKnownType.TypedReference))
+                    {
+                        EmitElementType(CorElementType.ELEMENT_TYPE_TYPEDBYREF);
+                        return;
+                    }
+
                     {
                         ModuleToken token = context.GetModuleTokenForType((EcmaType)typeDesc);
                         EmitModuleOverride(token.Module, context);
@@ -345,10 +351,11 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             EmitModuleOverride(targetModule, context);
             EmitElementType(CorElementType.ELEMENT_TYPE_GENERICINST);
             EmitTypeSignature(type.GetTypeDefinition(), context.InnerContext(targetModule));
+            SignatureContext outerContext = context.OuterContext;
             EmitUInt((uint)type.Instantiation.Length);
             for (int paramIndex = 0; paramIndex < type.Instantiation.Length; paramIndex++)
             {
-                EmitTypeSignature(type.Instantiation[paramIndex], context);
+                EmitTypeSignature(type.Instantiation[paramIndex], outerContext);
             }
         }
 
@@ -530,9 +537,10 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             {
                 Instantiation instantiation = method.Method.Instantiation;
                 EmitUInt((uint)instantiation.Length);
+                SignatureContext outerContext = context.OuterContext;
                 for (int typeParamIndex = 0; typeParamIndex < instantiation.Length; typeParamIndex++)
                 {
-                    EmitTypeSignature(instantiation[typeParamIndex], context);
+                    EmitTypeSignature(instantiation[typeParamIndex], outerContext);
                 }
             }
         }
@@ -616,7 +624,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             {
                 EmitByte((byte)(fixupKind | ReadyToRunFixupKind.ModuleOverride));
                 EmitUInt((uint)factory.ManifestMetadataTable.ModuleToIndex(targetModule));
-                return outerContext.InnerContext(targetModule);
+                return new SignatureContext(targetModule, outerContext.Resolver);
             }
         }
     }
index 901cc90..02c28f5 100644 (file)
@@ -32,6 +32,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         /// </summary>
         public readonly ModuleTokenResolver Resolver;
 
+        public SignatureContext OuterContext => new SignatureContext(GlobalContext, Resolver);
+
         public SignatureContext(EcmaModule context, ModuleTokenResolver resolver)
         {
             GlobalContext = context;
@@ -53,7 +55,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         public EcmaModule GetTargetModule(TypeDesc type)
         {
-            if (type.IsPrimitive || type.IsString || type.IsObject)
+            if (type.IsPrimitive || type.IsString || type.IsObject || type.IsWellKnownType(WellKnownType.TypedReference))
             {
                 return LocalContext;
             }
index e2d4aa7..1f1d3da 100644 (file)
@@ -33,5 +33,17 @@ namespace ILCompiler
 
             return method.HasCustomAttribute("System.Runtime.InteropServices", "SuppressGCTransitionAttribute");
         }
+
+        /// <summary>
+        /// Return true when the method is marked as non-versionable. Non-versionable methods
+        /// may be freely inlined into ReadyToRun images even when they don't reside in the
+        /// same version bubble as the module being compiled.
+        /// </summary>
+        /// <param name="method">Method to check</param>
+        /// <returns>True when the method is marked as non-versionable, false otherwise.</returns>
+        public static bool IsNonVersionable(this MethodDesc method)
+        {
+            return method.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
+        }
     }
 }
index 031764a..7d55bc5 100644 (file)
@@ -153,8 +153,7 @@ namespace ILCompiler
             // (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"));
+                (VersionsWithMethodBody(calleeMethod) || calleeMethod.IsNonVersionable());
 
             return canInline;
         }
index 8aa4adc..bd99070 100644 (file)
@@ -2061,7 +2061,7 @@ namespace Internal.JitInterface
                 }
 
                 // Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
-                return type is MetadataType metadataType && metadataType.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
+                return type is MetadataType metadataType && metadataType.IsNonVersionable();
             }
 
             return true;
index 93791b1..236e1df 100644 (file)
@@ -44,6 +44,8 @@ namespace ILCompiler
 
         public string[] CodegenOptions { get; set; }
 
+        public bool CompositeOrInputBubble => Composite || InputBubble;
+
         public static Command RootCommand()
         {
             // For some reason, arity caps at 255 by default
index ecb8954..9b8b4ce 100644 (file)
@@ -81,7 +81,7 @@ namespace ILCompiler
 
             if (_commandLineOptions.CompileBubbleGenerics)
             {
-                if (!_commandLineOptions.InputBubble)
+                if (!_commandLineOptions.CompositeOrInputBubble)
                 {
                     Console.WriteLine(SR.WarningIgnoringBubbleGenerics);
                     _commandLineOptions.CompileBubbleGenerics = false;
@@ -223,7 +223,7 @@ namespace ILCompiler
                         rootingModules.Add(module);
                         versionBubbleModulesHash.Add(module);
 
-                        if (!_commandLineOptions.InputBubble)
+                        if (!_commandLineOptions.CompositeOrInputBubble)
                         {
                             break;
                         }
@@ -335,9 +335,12 @@ namespace ILCompiler
                         // For non-single-method compilations add compilation roots.
                         foreach (var module in rootingModules)
                         {
-                            compilationRoots.Add(new ReadyToRunRootProvider(module, profileDataManager, _commandLineOptions.Partial));
+                            compilationRoots.Add(new ReadyToRunRootProvider(
+                                module,
+                                profileDataManager,
+                                profileDrivenPartialNGen: _commandLineOptions.Partial));
 
-                            if (!_commandLineOptions.InputBubble)
+                            if (!_commandLineOptions.CompositeOrInputBubble)
                             {
                                 break;
                             }