Skip to content

Fix type generator test failures #44041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Nov 1, 2020
Merged

Fix type generator test failures #44041

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b1719ac
Add constrained dispatch tests to crossgen2smoke
davidwrighton Oct 28, 2020
8e2d605
Test bits
davidwrighton Oct 28, 2020
9101b01
Fix bug around method signatures where method is referenced via a der…
davidwrighton Oct 29, 2020
4f9a2c6
Mark tests associated with issue 43466 as fixed.
davidwrighton Oct 30, 2020
8482e34
Fix concern where constrained calls do not properly specify the corre…
davidwrighton Oct 30, 2020
114597a
Fix crossgen2 composite build
davidwrighton Oct 30, 2020
7cdd676
Simplify naming and remove duplicate logic from method signature gene…
davidwrighton Oct 30, 2020
2e9c8cb
Run crossgen2 composite compilation with multi-core
davidwrighton Oct 30, 2020
1f2fdab
Disable non-crossgen2 variant of crossgen2smoke test on Mono
davidwrighton Oct 30, 2020
164a658
Improve Comparison functions, and fix bug with unnecessarily specific…
davidwrighton Nov 1, 2020
bdab5e5
Fix issues found in testing
davidwrighton Nov 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ...aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelegateCtorSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ public class DelegateCtorSignature : Signature

private readonly IMethodNode _targetMethod;

private readonly ModuleToken _methodToken;
private readonly MethodWithToken _methodToken;

public DelegateCtorSignature(
TypeDesc delegateType,
IMethodNode targetMethod,
ModuleToken methodToken)
MethodWithToken methodToken)
{
_delegateType = delegateType;
_targetMethod = targetMethod;
Expand All @@ -40,10 +40,10 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)

if (!relocsOnly)
{
SignatureContext innerContext = builder.EmitFixup(factory, ReadyToRunFixupKind.DelegateCtor, _methodToken.Module, factory.SignatureContext);
SignatureContext innerContext = builder.EmitFixup(factory, ReadyToRunFixupKind.DelegateCtor, _methodToken.Token.Module, factory.SignatureContext);

builder.EmitMethodSignature(
new MethodWithToken(_targetMethod.Method, _methodToken, constrainedType: null, unboxing: false),
_methodToken,
enforceDefEncoding: false,
enforceOwningType: false,
innerContext,
Expand Down Expand Up @@ -88,7 +88,7 @@ public override int CompareToImpl(ISortableNode other, CompilerComparer comparer
if (result != 0)
return result;

return _methodToken.CompareTo(otherNode._methodToken);
return _methodToken.Token.CompareTo(otherNode._methodToken.Token);
}
}
}
3 changes: 2 additions & 1 deletion ...s/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
{
sb.Append(nameMangler.CompilationUnitPrefix);
sb.Append($@"TypeFixupSignature({_fixupKind.ToString()}): {_fieldDesc.ToString()}");
sb.Append($@"FieldFixupSignature({_fixupKind.ToString()}): ");
sb.Append(nameMangler.GetMangledFieldName(_fieldDesc));
}

public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
Expand Down
2 changes: 1 addition & 1 deletion ...Compiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)

ArraySignatureBuilder signatureBuilder = new ArraySignatureBuilder();
signatureBuilder.EmitMethodSignature(
new MethodWithToken(method.Method, moduleToken, constrainedType: null, unboxing: false),
new MethodWithToken(method.Method, moduleToken, constrainedType: null, unboxing: false, context: null),
enforceDefEncoding: true,
enforceOwningType: _factory.CompilationModuleGroup.EnforceOwningType(moduleToken.Module),
factory.SignatureContext,
Expand Down
16 changes: 12 additions & 4 deletions .../aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Diagnostics;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;

using Internal.JitInterface;
using Internal.Text;
Expand Down Expand Up @@ -32,6 +35,8 @@ public MethodFixupSignature(
// Ensure types in signature are loadable and resolvable, otherwise we'll fail later while emitting the signature
CompilerTypeSystemContext compilerContext = (CompilerTypeSystemContext)method.Method.Context;
compilerContext.EnsureLoadableMethod(method.Method);
compilerContext.EnsureLoadableType(_method.OwningType);

if (method.ConstrainedType != null)
compilerContext.EnsureLoadableType(method.ConstrainedType);
}
Expand Down Expand Up @@ -68,8 +73,11 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
}
else if (_method.Token.TokenType == CorTokenType.mdtMemberRef)
{
fixupKind = ReadyToRunFixupKind.MethodEntry_RefToken;
optimized = true;
if (!_method.OwningTypeNotDerivedFromToken)
{
fixupKind = ReadyToRunFixupKind.MethodEntry_RefToken;
optimized = true;
}
}
}
}
Expand All @@ -80,13 +88,13 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
if (method.Token.TokenType == CorTokenType.mdtMethodSpec)
{
method = new MethodWithToken(method.Method, factory.SignatureContext.GetModuleTokenForMethod(method.Method, throwIfNotFound: false), method.ConstrainedType, unboxing: _method.Unboxing);
method = new MethodWithToken(method.Method, factory.SignatureContext.GetModuleTokenForMethod(method.Method, throwIfNotFound: false), method.ConstrainedType, unboxing: _method.Unboxing, null);
}
else if (!optimized && (method.Token.TokenType == CorTokenType.mdtMemberRef))
{
if (method.Method.OwningType.GetTypeDefinition() is EcmaType)
{
method = new MethodWithToken(method.Method, factory.SignatureContext.GetModuleTokenForMethod(method.Method, throwIfNotFound: false), method.ConstrainedType, unboxing: _method.Unboxing);
method = new MethodWithToken(method.Method, factory.SignatureContext.GetModuleTokenForMethod(method.Method, throwIfNotFound: false), method.ConstrainedType, unboxing: _method.Unboxing, null);
}
}
}
Expand Down
77 changes: 6 additions & 71 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,62 +412,12 @@ public void EmitMethodSignature(
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_Constrained;
}
if (enforceOwningType)
if (enforceOwningType || method.OwningTypeNotDerivedFromToken)
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType;
}

if ((method.Method.HasInstantiation || method.Method.OwningType.HasInstantiation) && !method.Method.IsGenericMethodDefinition)
{
EmitMethodSpecificationSignature(method, flags, enforceDefEncoding, context);
}
else
{
switch (method.Token.TokenType)
{
case CorTokenType.mdtMethodDef:
{
EmitUInt(flags);
if ((flags & (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType) != 0)
{
EmitTypeSignature(method.Method.OwningType, context);
}
EmitMethodDefToken(method.Token);
}
break;

case CorTokenType.mdtMemberRef:
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_MemberRefToken;

// Owner type is needed for type specs to instantiating stubs or generics with signature variables still present
if (!method.Method.OwningType.IsDefType &&
((flags & (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_InstantiatingStub) != 0 || method.Method.OwningType.ContainsSignatureVariables()))
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType;
}
else if (method.Method.IsArrayMethod())
{
var memberRefMethod = method.Token.Module.GetMethod(MetadataTokens.EntityHandle((int)method.Token.Token));
if (memberRefMethod.OwningType != method.Method.OwningType)
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType;
}
}

EmitUInt(flags);
if ((flags & (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType) != 0)
{
EmitTypeSignature(method.Method.OwningType, context);
}
EmitMethodRefToken(method.Token);
}
break;

default:
throw new NotImplementedException();
}
}
EmitMethodSpecificationSignature(method, flags, enforceDefEncoding, context);

if (method.ConstrainedType != null)
{
Expand All @@ -491,7 +441,7 @@ private void EmitMethodSpecificationSignature(MethodWithToken method,
uint flags, bool enforceDefEncoding, SignatureContext context)
{
ModuleToken methodToken = method.Token;
if (method.Method.HasInstantiation)
if (method.Method.HasInstantiation && !method.Method.IsGenericMethodDefinition)
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_MethodInstantiation;
if (!method.Token.IsNull)
Expand All @@ -504,23 +454,7 @@ private void EmitMethodSpecificationSignature(MethodWithToken method,
}
}

if (methodToken.IsNull && !enforceDefEncoding)
{
methodToken = context.GetModuleTokenForMethod(method.Method, throwIfNotFound: false);
}
if (methodToken.IsNull)
{
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType;
methodToken = context.GetModuleTokenForMethod(method.Method);
}

if (method.Method.OwningType.HasInstantiation)
{
// resolveToken currently resolves the token in the context of a given scope;
// in such case, we receive a method on instantiated type along with the
// generic definition token.
flags |= (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType;
}
Debug.Assert(!methodToken.IsNull);

switch (methodToken.TokenType)
{
Expand All @@ -538,7 +472,8 @@ private void EmitMethodSpecificationSignature(MethodWithToken method,
EmitUInt(flags);
if ((flags & (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_OwnerType) != 0)
{
EmitTypeSignature(method.Method.OwningType, context);
// The type here should be the type referred to by the memberref (if this is one, not the type where the method was eventually found!
EmitTypeSignature(method.OwningType, context);
}
EmitTokenRid(methodToken.Token);
if ((flags & (uint)ReadyToRunMethodSigFlags.READYTORUN_METHOD_SIG_MethodInstantiation) != 0)
Expand Down
2 changes: 1 addition & 1 deletion ...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ public IEnumerable<MethodWithGCInfo> EnumerateCompiledMethods(EcmaModule moduleT
EcmaModule module = ((EcmaMethod)method.GetTypicalMethodDefinition()).Module;
ModuleToken moduleToken = Resolver.GetModuleTokenForMethod(method, throwIfNotFound: true);

IMethodNode methodNodeDebug = MethodEntrypoint(new MethodWithToken(method, moduleToken, constrainedType: null, unboxing: false), false, false);
IMethodNode methodNodeDebug = MethodEntrypoint(new MethodWithToken(method, moduleToken, constrainedType: null, unboxing: false, context: null), false, false);
MethodWithGCInfo methodCodeNodeDebug = methodNodeDebug as MethodWithGCInfo;
if (methodCodeNodeDebug == null && methodNodeDebug is DelayLoadMethodImport DelayLoadMethodImport)
{
Expand Down
2 changes: 1 addition & 1 deletion ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ private void CreateNodeCaches()
_codegenNodeFactory,
_codegenNodeFactory.HelperImports,
ReadyToRunHelper.DelayLoad_Helper_ObjObj,
new DelegateCtorSignature(ctorKey.Type, targetMethodNode, ctorKey.Method.Token));
new DelegateCtorSignature(ctorKey.Type, targetMethodNode, ctorKey.Method));
});

_checkTypeLayoutCache = new NodeCache<TypeDesc, ISymbolNode>(key =>
Expand Down
Loading