Skip to content

Commit

Permalink
Remove non-determinism from R2R field access (#73663)
Browse files Browse the repository at this point in the history 
* Remove non-determinism from R2R field access
- Remove all trace of storing off the field token in a compiler non-deterministic manner
 - Notably, remove the _fieldToRefTokens field from ModuleTokenResolver, and remove the ability of that class to resolve tokens for fields
- Add a new concept called FieldWithToken modeled after MethodWithToken
- Plumb through the various dependency analysis structures as needed

Fixes #73403
  • Loading branch information
@davidwrighton
davidwrighton authored Aug 11, 2022
1 parent ef77c41 commit 0559d4a
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 138 deletions.
7 changes: 1 addition & 6 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
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
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
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
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
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
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
40 changes: 20 additions & 20 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
View file
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 All @@ -622,9 +622,9 @@ private ISymbolNode GenericLookupTypeHelper(
{
typeArgument = methodWithToken.Method.OwningType;
}
else if (helperArgument is FieldDesc fieldDesc)
else if (helperArgument is FieldWithToken fieldWithToken)
{
typeArgument = fieldDesc.OwningType;
typeArgument = fieldWithToken.Field.OwningType;
}
else
{
Expand All @@ -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

0 comments on commit 0559d4a

Please sign in to comment.