Skip to content

Commit

Permalink
Crossgen2 fixes to enable composite build with shared framework (#34431)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
trylek authored Apr 3, 2020
1 parent dad74cc commit b8c9296
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 11 deletions.
10 changes: 10 additions & 0 deletions src/coreclr/src/tools/Common/Compiler/TypeExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -502,5 +502,15 @@ private static TypeDesc GetTypeThatMeetsConstraints(GenericParameterDesc generic

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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,12 @@ public void EmitTypeSignature(TypeDesc typeDesc, SignatureContext context)
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);
Expand Down Expand Up @@ -345,10 +351,11 @@ private void EmitInstantiatedTypeSignature(InstantiatedType type, SignatureConte
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);
}
}

Expand Down Expand Up @@ -530,9 +537,10 @@ private void EmitMethodSpecificationSignature(MethodWithToken method,
{
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);
}
}
}
Expand Down Expand Up @@ -616,7 +624,7 @@ public SignatureContext EmitFixup(NodeFactory factory, ReadyToRunFixupKind fixup
{
EmitByte((byte)(fixupKind | ReadyToRunFixupKind.ModuleOverride));
EmitUInt((uint)factory.ManifestMetadataTable.ModuleToIndex(targetModule));
return outerContext.InnerContext(targetModule);
return new SignatureContext(targetModule, outerContext.Resolver);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class SignatureContext
/// </summary>
public readonly ModuleTokenResolver Resolver;

public SignatureContext OuterContext => new SignatureContext(GlobalContext, Resolver);

public SignatureContext(EcmaModule context, ModuleTokenResolver resolver)
{
GlobalContext = context;
Expand All @@ -53,7 +55,7 @@ public SignatureContext InnerContext(EcmaModule innerContext)

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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,17 @@ public static bool IsSuppressGCTransition(this MethodDesc method)

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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ public sealed override bool CanInline(MethodDesc callerMethod, MethodDesc callee
// (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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2061,7 +2061,7 @@ private bool IsLayoutFixedInCurrentVersionBubble(TypeDesc type)
}

// 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class CommandLineOptions

public string[] CodegenOptions { get; set; }

public bool CompositeOrInputBubble => Composite || InputBubble;

public static Command RootCommand()
{
// For some reason, arity caps at 255 by default
Expand Down
11 changes: 7 additions & 4 deletions src/coreclr/src/tools/crossgen2/crossgen2/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private void ProcessCommandLine()

if (_commandLineOptions.CompileBubbleGenerics)
{
if (!_commandLineOptions.InputBubble)
if (!_commandLineOptions.CompositeOrInputBubble)
{
Console.WriteLine(SR.WarningIgnoringBubbleGenerics);
_commandLineOptions.CompileBubbleGenerics = false;
Expand Down Expand Up @@ -223,7 +223,7 @@ private int Run()
rootingModules.Add(module);
versionBubbleModulesHash.Add(module);

if (!_commandLineOptions.InputBubble)
if (!_commandLineOptions.CompositeOrInputBubble)
{
break;
}
Expand Down Expand Up @@ -335,9 +335,12 @@ private int Run()
// 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;
}
Expand Down

0 comments on commit b8c9296

Please sign in to comment.