Skip to content
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

Remove non-determinism from R2R field access #73663

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Remove non-determinism from R2R field access #73663

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7e077c
Remove non-determinism from R2R field access
davidwrighton Aug 9, 2022
82b4e99
Handle the generic lookup case reliably
davidwrighton Aug 10, 2022
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
7 changes: 1 addition & 6 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1713,12 +1713,7 @@ private void resolveToken(ref CORINFO_RESOLVED_TOKEN pResolvedToken)
_compilation.TypeSystemContext.EnsureLoadableType(owningClass);
#endif

#if READYTORUN
if (recordToken)
{
_compilation.NodeFactory.Resolver.AddModuleTokenForField(field, HandleToModuleToken(ref pResolvedToken));
}
#else
#if !READYTORUN
_compilation.NodeFactory.MetadataManager.GetDependenciesDueToAccess(ref _additionalDependencies, _compilation.NodeFactory, (MethodIL)methodIL, field);
#endif
}
Expand Down
28 changes: 14 additions & 14 deletions ...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 @@ -17,15 +17,15 @@ public class FieldFixupSignature : Signature
public const int MaxCheckableOffset = 0x1FFFFFFF;
private readonly ReadyToRunFixupKind _fixupKind;

private readonly FieldDesc _fieldDesc;
private readonly FieldWithToken _fieldWithToken;

public FieldFixupSignature(ReadyToRunFixupKind fixupKind, FieldDesc fieldDesc, NodeFactory factory)
public FieldFixupSignature(ReadyToRunFixupKind fixupKind, FieldWithToken fieldWithToken, NodeFactory factory)
{
_fixupKind = fixupKind;
_fieldDesc = fieldDesc;
_fieldWithToken = fieldWithToken;

// Ensure types in signature are loadable and resolvable, otherwise we'll fail later while emitting the signature
((CompilerTypeSystemContext)fieldDesc.Context).EnsureLoadableType(fieldDesc.OwningType);
((CompilerTypeSystemContext)fieldWithToken.Field.Context).EnsureLoadableType(fieldWithToken.Field.OwningType);
}

public override int ClassCode => 271828182;
Expand All @@ -38,19 +38,19 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
dataBuilder.AddSymbol(this);

IEcmaModule targetModule = factory.SignatureContext.GetTargetModule(_fieldDesc);
IEcmaModule targetModule = _fieldWithToken.Token.Module;
SignatureContext innerContext = dataBuilder.EmitFixup(factory, _fixupKind, targetModule, factory.SignatureContext);
uint baseOffset = 0;
uint fieldOffset = (uint)_fieldDesc.Offset.AsInt;
uint fieldOffset = (uint)_fieldWithToken.Field.Offset.AsInt;

if (_fixupKind == ReadyToRunFixupKind.Verify_FieldOffset)
{
TypeDesc baseType = _fieldDesc.OwningType.BaseType;
if ((_fieldDesc.OwningType.BaseType != null)
&& !_fieldDesc.IsStatic
&& !_fieldDesc.OwningType.IsValueType)
TypeDesc baseType = _fieldWithToken.Field.OwningType.BaseType;
if ((_fieldWithToken.Field.OwningType.BaseType != null)
&& !_fieldWithToken.Field.IsStatic
&& !_fieldWithToken.Field.OwningType.IsValueType)
{
MetadataType owningType = (MetadataType)_fieldDesc.OwningType;
MetadataType owningType = (MetadataType)_fieldWithToken.Field.OwningType;
baseOffset = (uint)owningType.FieldBaseOffset().AsInt;
if (factory.CompilationModuleGroup.NeedsAlignmentBetweenBaseTypeAndDerived((MetadataType)baseType, owningType))
{
Expand All @@ -67,7 +67,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
dataBuilder.EmitUInt(fieldOffset);
}

dataBuilder.EmitFieldSignature(_fieldDesc, innerContext);
dataBuilder.EmitFieldSignature(_fieldWithToken, innerContext);
}

return dataBuilder.ToObjectData();
Expand All @@ -77,7 +77,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
{
sb.Append(nameMangler.CompilationUnitPrefix);
sb.Append($@"FieldFixupSignature({_fixupKind.ToString()}): ");
sb.Append(nameMangler.GetMangledFieldName(_fieldDesc));
_fieldWithToken.AppendMangledName(nameMangler, sb);
}

public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
Expand All @@ -87,7 +87,7 @@ public override int CompareToImpl(ISortableNode other, CompilerComparer comparer
if (result != 0)
return result;

return comparer.Compare(_fieldDesc, otherNode._fieldDesc);
return _fieldWithToken.CompareTo(otherNode._fieldWithToken, comparer);
}
}
}
10 changes: 5 additions & 5 deletions ...ot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class GenericLookupSignature : Signature

private readonly MethodWithToken _methodArgument;

private readonly FieldDesc _fieldArgument;
private readonly FieldWithToken _fieldArgument;

private readonly GenericContext _methodContext;

Expand All @@ -31,7 +31,7 @@ public GenericLookupSignature(
ReadyToRunFixupKind fixupKind,
TypeDesc typeArgument,
MethodWithToken methodArgument,
FieldDesc fieldArgument,
FieldWithToken fieldArgument,
GenericContext methodContext)
{
Debug.Assert(typeArgument != null || methodArgument != null || fieldArgument != null);
Expand Down Expand Up @@ -64,7 +64,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
}
else if (_fieldArgument != null)
{
targetModule = factory.SignatureContext.GetTargetModule(_fieldArgument);
targetModule = _fieldArgument.Token.Module;
}
else
{
Expand Down Expand Up @@ -173,7 +173,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
}
if (_fieldArgument != null)
{
sb.Append(nameMangler.GetMangledFieldName(_fieldArgument));
_fieldArgument.AppendMangledName(nameMangler, sb);
}
sb.Append(" (");
_methodContext.AppendMangledName(nameMangler, sb);
Expand Down Expand Up @@ -210,7 +210,7 @@ public override int CompareToImpl(ISortableNode other, CompilerComparer comparer
if (otherNode._fieldArgument == null)
return 1;

result = comparer.Compare(_fieldArgument, otherNode._fieldArgument);
result = _fieldArgument.CompareTo(otherNode._fieldArgument, comparer);
if (result != 0)
return result;
}
Expand Down
61 changes: 0 additions & 61 deletions ...s/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleTokenResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ public class ModuleTokenResolver
/// </summary>
private readonly ConcurrentDictionary<EcmaType, ModuleToken> _typeToRefTokens = new ConcurrentDictionary<EcmaType, ModuleToken>();

private readonly ConcurrentDictionary<FieldDesc, ModuleToken> _fieldToRefTokens = new ConcurrentDictionary<FieldDesc, ModuleToken>();

private readonly CompilationModuleGroup _compilationModuleGroup;

private Func<IEcmaModule, int> _moduleIndexLookup;
Expand Down Expand Up @@ -127,36 +125,6 @@ public ModuleToken GetModuleTokenForMethod(MethodDesc method, bool allowDynamica
}
}

public ModuleToken GetModuleTokenForField(FieldDesc field, bool throwIfNotFound = true)
{
if (_compilationModuleGroup.VersionsWithType(field.OwningType) && field is EcmaField ecmaField)
{
return new ModuleToken(ecmaField.Module, ecmaField.Handle);
}

TypeDesc owningCanonType = field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
FieldDesc canonField = field;
if (owningCanonType != field.OwningType)
{
canonField = CompilerContext.GetFieldForInstantiatedType(field.GetTypicalFieldDefinition(), (InstantiatedType)owningCanonType);
}

ModuleToken token;
if (_fieldToRefTokens.TryGetValue(canonField, out token))
{
return token;
}

if (throwIfNotFound)
{
throw new NotImplementedException(field.ToString());
}
else
{
return default(ModuleToken);
}
}

public void AddModuleTokenForMethod(MethodDesc method, ModuleToken token)
{
if (token.TokenType == CorTokenType.mdtMethodSpec)
Expand Down Expand Up @@ -188,35 +156,6 @@ private void AddModuleTokenForFieldReference(TypeDesc owningType, ModuleToken to
memberRef.DecodeFieldSignature<DummyTypeInfo, ModuleTokenResolver>(new TokenResolverProvider(this, token.Module), this);
}

public void AddModuleTokenForField(FieldDesc field, ModuleToken token)
{
if (_compilationModuleGroup.VersionsWithType(field.OwningType) && field.OwningType is EcmaType)
{
// We don't need to store handles within the current compilation group
// as we can read them directly from the ECMA objects.
return;
}

TypeDesc owningCanonType = field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
FieldDesc canonField = field;
if (owningCanonType != field.OwningType)
{
canonField = CompilerContext.GetFieldForInstantiatedType(field.GetTypicalFieldDefinition(), (InstantiatedType)owningCanonType);
}

SetModuleTokenForTypeSystemEntity(_fieldToRefTokens, canonField, token);

switch (token.TokenType)
{
case CorTokenType.mdtMemberRef:
AddModuleTokenForFieldReference(owningCanonType, token);
break;

default:
throw new NotImplementedException();
}
}

// 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)
Expand Down
14 changes: 4 additions & 10 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 @@ -530,23 +530,17 @@ private void EmitMethodSpecificationSignature(MethodWithToken method,
}
}

public void EmitFieldSignature(FieldDesc field, SignatureContext context)
public void EmitFieldSignature(FieldWithToken field, SignatureContext context)
{
uint fieldSigFlags = 0;
TypeDesc canonOwnerType = field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
TypeDesc ownerType = null;
if (canonOwnerType.HasInstantiation)
if (field.OwningTypeNotDerivedFromToken)
{
ownerType = field.OwningType;
ownerType = field.Field.OwningType;
fieldSigFlags |= (uint)ReadyToRunFieldSigFlags.READYTORUN_FIELD_SIG_OwnerType;
}
if (canonOwnerType != field.OwningType)
{
// Convert field to canonical form as this is what the field - module token lookup stores
field = field.Context.GetFieldForInstantiatedType(field.GetTypicalFieldDefinition(), (InstantiatedType)canonOwnerType);
}

ModuleToken fieldToken = context.GetModuleTokenForField(field);
ModuleToken fieldToken = field.Token;
switch (fieldToken.TokenType)
{
case CorTokenType.mdtMemberRef:
Expand Down
10 changes: 0 additions & 10 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/SignatureContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ public IEcmaModule GetTargetModule(TypeDesc type)
return LocalContext;
}

public IEcmaModule GetTargetModule(FieldDesc field)
{
return GetModuleTokenForField(field).Module;
}

public IEcmaModule GetTargetModule(MethodDesc method)
{
return GetModuleTokenForMethod(method).Module;
Expand All @@ -86,11 +81,6 @@ public ModuleToken GetModuleTokenForMethod(MethodDesc method)
return Resolver.GetModuleTokenForMethod(method, throwIfNotFound: false, allowDynamicallyCreatedReference: false);
}

public ModuleToken GetModuleTokenForField(FieldDesc field, bool throwIfNotFound = true)
{
return Resolver.GetModuleTokenForField(field, throwIfNotFound);
}

public bool Equals(SignatureContext other)
{
return GlobalContext == other.GlobalContext
Expand Down
36 changes: 18 additions & 18 deletions ...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 @@ -65,7 +65,7 @@ private void CreateNodeCaches()
new ReadyToRunInstructionSetSupportSignature(key));
});

_fieldAddressCache = new NodeCache<FieldDesc, ISymbolNode>(key =>
_fieldAddressCache = new NodeCache<FieldWithToken, ISymbolNode>(key =>
{
return new DelayLoadHelperImport(
_codegenNodeFactory,
Expand All @@ -75,7 +75,7 @@ private void CreateNodeCaches()
);
});

_fieldOffsetCache = new NodeCache<FieldDesc, ISymbolNode>(key =>
_fieldOffsetCache = new NodeCache<FieldWithToken, ISymbolNode>(key =>
{
return new PrecodeHelperImport(
_codegenNodeFactory,
Expand All @@ -91,7 +91,7 @@ private void CreateNodeCaches()
);
});

_checkFieldOffsetCache = new NodeCache<FieldDesc, ISymbolNode>(key =>
_checkFieldOffsetCache = new NodeCache<FieldWithToken, ISymbolNode>(key =>
{
return new PrecodeHelperImport(
_codegenNodeFactory,
Expand Down Expand Up @@ -241,7 +241,7 @@ private ISymbolNode CreateReadyToRunHelper(ReadyToRunHelperKey key)
return CreateMethodHandleHelper((MethodWithToken)key.Target);

case ReadyToRunHelperId.FieldHandle:
return CreateFieldHandleHelper((FieldDesc)key.Target);
return CreateFieldHandleHelper((FieldWithToken)key.Target);

case ReadyToRunHelperId.CctorTrigger:
return CreateCctorTrigger((TypeDesc)key.Target);
Expand Down Expand Up @@ -361,7 +361,7 @@ private ISymbolNode CreateMethodHandleHelper(MethodWithToken method)
isInstantiatingStub: useInstantiatingStub));
}

private ISymbolNode CreateFieldHandleHelper(FieldDesc field)
private ISymbolNode CreateFieldHandleHelper(FieldWithToken field)
{
return new PrecodeHelperImport(
_codegenNodeFactory,
Expand Down Expand Up @@ -395,25 +395,25 @@ private ISymbolNode CreateMethodDictionary(MethodWithToken method)
isInstantiatingStub: true));
}

private NodeCache<FieldDesc, ISymbolNode> _fieldAddressCache;
private NodeCache<FieldWithToken, ISymbolNode> _fieldAddressCache;

public ISymbolNode FieldAddress(FieldDesc fieldDesc)
public ISymbolNode FieldAddress(FieldWithToken fieldWithToken)
{
return _fieldAddressCache.GetOrAdd(fieldDesc);
return _fieldAddressCache.GetOrAdd(fieldWithToken);
}

private NodeCache<FieldDesc, ISymbolNode> _fieldOffsetCache;
private NodeCache<FieldWithToken, ISymbolNode> _fieldOffsetCache;

public ISymbolNode FieldOffset(FieldDesc fieldDesc)
public ISymbolNode FieldOffset(FieldWithToken fieldWithToken)
{
return _fieldOffsetCache.GetOrAdd(fieldDesc);
return _fieldOffsetCache.GetOrAdd(fieldWithToken);
}

private NodeCache<FieldDesc, ISymbolNode> _checkFieldOffsetCache;
private NodeCache<FieldWithToken, ISymbolNode> _checkFieldOffsetCache;

public ISymbolNode CheckFieldOffset(FieldDesc fieldDesc)
public ISymbolNode CheckFieldOffset(FieldWithToken fieldWithToken)
{
return _checkFieldOffsetCache.GetOrAdd(fieldDesc);
return _checkFieldOffsetCache.GetOrAdd(fieldWithToken);
}

private NodeCache<TypeDesc, ISymbolNode> _fieldBaseOffsetCache;
Expand Down Expand Up @@ -502,15 +502,15 @@ private struct GenericLookupKey : IEquatable<GenericLookupKey>
public readonly ReadyToRunFixupKind FixupKind;
public readonly TypeDesc TypeArgument;
public readonly MethodWithToken MethodArgument;
public readonly FieldDesc FieldArgument;
public readonly FieldWithToken FieldArgument;
public readonly GenericContext MethodContext;

public GenericLookupKey(
CORINFO_RUNTIME_LOOKUP_KIND lookupKind,
ReadyToRunFixupKind fixupKind,
TypeDesc typeArgument,
MethodWithToken methodArgument,
FieldDesc fieldArgument,
FieldWithToken fieldArgument,
GenericContext methodContext)
{
LookupKind = lookupKind;
Expand Down Expand Up @@ -603,7 +603,7 @@ public ISymbolNode GenericLookupHelper(
return GenericLookupFieldHelper(
runtimeLookupKind,
ReadyToRunFixupKind.FieldHandle,
(FieldDesc)helperArgument,
(FieldWithToken)helperArgument,
methodContext);

default:
Expand Down Expand Up @@ -638,7 +638,7 @@ private ISymbolNode GenericLookupTypeHelper(
private ISymbolNode GenericLookupFieldHelper(
CORINFO_RUNTIME_LOOKUP_KIND runtimeLookupKind,
ReadyToRunFixupKind fixupKind,
FieldDesc fieldArgument,
FieldWithToken fieldArgument,
GenericContext methodContext)
{
GenericLookupKey key = new GenericLookupKey(runtimeLookupKind, fixupKind, typeArgument: null, methodArgument: null, fieldArgument: fieldArgument, methodContext);
Expand Down
Loading