Skip to content

Commit

Permalink
Needs measurements
Browse files Browse the repository at this point in the history
  • Loading branch information
MichalStrehovsky committed Dec 6, 2023
1 parent 71f3bf1 commit b32d899
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ private void CreateNodeCaches()
return new NotReadOnlyFieldNode(field);
});

_writtenStaticFields = new NodeCache<FieldDesc, WrittenStaticFieldNode>(field =>
{
return new WrittenStaticFieldNode(field);
});

_genericStaticBaseInfos = new NodeCache<MetadataType, GenericStaticBaseInfoNode>(type =>
{
return new GenericStaticBaseInfoNode(type);
Expand Down Expand Up @@ -1020,6 +1025,12 @@ public NotReadOnlyFieldNode NotReadOnlyField(FieldDesc field)
return _notReadOnlyFields.GetOrAdd(field);
}

private NodeCache<FieldDesc, WrittenStaticFieldNode> _writtenStaticFields;
public WrittenStaticFieldNode WrittenStaticField(FieldDesc field)
{
return _writtenStaticFields.GetOrAdd(field);
}

private NodeCache<MetadataType, GenericStaticBaseInfoNode> _genericStaticBaseInfos;
internal GenericStaticBaseInfoNode GenericStaticBaseInfo(MetadataType type)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,15 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto

// readonly static fields are not reflection settable, the rest are
if (!_field.IsInitOnly || !_field.IsStatic)
{
dependencies.Add(factory.NotReadOnlyField(_field), "Reflection writable field");

if (_field.IsStatic)
{
dependencies.Add(factory.WrittenStaticField(_field), "Reflection writable field");
}
}

FieldDesc typicalField = _field.GetTypicalFieldDefinition();
if (typicalField != _field)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;

using ILCompiler.DependencyAnalysisFramework;

using Internal.TypeSystem;

using Debug = System.Diagnostics.Debug;

namespace ILCompiler.DependencyAnalysis
{
/// <summary>
/// Represents a static field that is written at runtime (could be written from lazy .cctor, or reflection, or code).
/// </summary>
public class WrittenStaticFieldNode : DependencyNodeCore<NodeFactory>
{
private readonly FieldDesc _field;

public WrittenStaticFieldNode(FieldDesc field)
{
Debug.Assert(!field.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)
|| field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific) == field.OwningType);
Debug.Assert(field.IsStatic);
_field = field;
}

public FieldDesc Field => _field;

protected override string GetName(NodeFactory factory)
{
return "Written static field: " + _field.ToString();
}

public override bool InterestingForDynamicDependencyAnalysis => false;
public override bool HasDynamicDependencies => false;
public override bool HasConditionalStaticDependencies => false;
public override bool StaticDependenciesAreComputed => true;
public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory) => null;
public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
}
}
27 changes: 22 additions & 5 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -831,20 +831,26 @@ public override bool CanPreinitializeAllConcreteFormsForCanonForm(DefType type)

private sealed class ScannedReadOnlyPolicy : ReadOnlyFieldPolicy
{
private HashSet<FieldDesc> _writtenFields = new();
private HashSet<FieldDesc> _notReadOnlyFields = new();
private HashSet<FieldDesc> _writtenStaticFields = new();

public ScannedReadOnlyPolicy(ImmutableArray<DependencyNodeCore<NodeFactory>> markedNodes)
{
foreach (var node in markedNodes)
{
if (node is NotReadOnlyFieldNode writtenField)
if (node is NotReadOnlyFieldNode notReadOnlyField)
{
_writtenFields.Add(writtenField.Field);
_notReadOnlyFields.Add(notReadOnlyField.Field);
}

if (node is WrittenStaticFieldNode writtenStaticField)
{
_writtenStaticFields.Add(writtenStaticField.Field);
}
}
}

public override bool IsReadOnly(FieldDesc field)
private static FieldDesc ConvertToCanon(FieldDesc field)
{
FieldDesc typicalField = field.GetTypicalFieldDefinition();
if (field != typicalField)
Expand All @@ -854,8 +860,19 @@ public override bool IsReadOnly(FieldDesc field)
if (owningType != canonOwningType)
field = field.Context.GetFieldForInstantiatedType(typicalField, canonOwningType);
}
return field;
}

return !_writtenFields.Contains(field);
public override bool IsReadOnly(FieldDesc field)
{
return !_notReadOnlyFields.Contains(ConvertToCanon(field));
}

public override bool IsNeverWrittenAtRuntime(FieldDesc field)
{
Debug.Assert(field.IsStatic);
Debug.Assert(!field.HasRva);
return !_writtenStaticFields.Contains(ConvertToCanon(field));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ namespace ILCompiler
public class ReadOnlyFieldPolicy
{
public virtual bool IsReadOnly(FieldDesc field) => field.IsInitOnly;

/// <summary>
/// This might look the same as IsReadOnly, but this will also return false for readonly fields
/// that are set in a lazy static constructor. This method will only return true if the value cannot possibly
/// change at runtime. This will return true for fields that either have their value set in the preinit data blob
/// or are default-initialized, and there's no code that could change them at runtime (not even a lazy cctor).
/// </summary>
public virtual bool IsNeverWrittenAtRuntime(FieldDesc field) => false;
}

public sealed class StaticReadOnlyFieldPolicy : ReadOnlyFieldPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,10 +379,9 @@ private Status TryScanMethod(MethodIL methodIL, Value[] parameters, Stack<Method
else
return Status.Fail(methodIL.OwningMethod, opcode);
}
else if (_readOnlyPolicy.IsReadOnly(field)
&& !field.OwningType.HasStaticConstructor)
else if (_readOnlyPolicy.IsNeverWrittenAtRuntime(field))
{
// (Effectively) read only field but no static constructor to set it: the value is default-initialized.
// (Effectively) read only field: nobody can write it at runtime.
stack.PushFromLocation(field.FieldType, NewUninitializedLocationValue(field.FieldType));
}
else
Expand Down
17 changes: 11 additions & 6 deletions src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -999,16 +999,21 @@ private void ImportFieldAccess(int token, bool isStatic, bool? write, string rea
&& ((field.IsStatic && _methodIL.OwningMethod.IsStaticConstructor)
|| (!field.IsStatic && _methodIL.OwningMethod.IsConstructor));

FieldDesc fieldToReport = canonField;
DefType fieldOwningType = canonField.OwningType;
TypeDesc canonFieldOwningType = fieldOwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
if (fieldOwningType != canonFieldOwningType)
fieldToReport = _factory.TypeSystemContext.GetFieldForInstantiatedType(fieldToReport.GetTypicalFieldDefinition(), (InstantiatedType)canonFieldOwningType);

if (!isInitOnlyWrite)
{
FieldDesc fieldToReport = canonField;
DefType fieldOwningType = canonField.OwningType;
TypeDesc canonFieldOwningType = fieldOwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
if (fieldOwningType != canonFieldOwningType)
fieldToReport = _factory.TypeSystemContext.GetFieldForInstantiatedType(fieldToReport.GetTypicalFieldDefinition(), (InstantiatedType)canonFieldOwningType);

_dependencies.Add(_factory.NotReadOnlyField(fieldToReport), "Field written outside initializer");
}

if (field.IsStatic)
{
_dependencies.Add(_factory.WrittenStaticField(fieldToReport), "Written static field");
}
}

// Covers both ldsfld/ldsflda and ldfld/ldflda with a static field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@
<Compile Include="Compiler\DependencyAnalysis\CustomAttributeBasedDependencyAlgorithm.cs" />
<Compile Include="Compiler\DependencyAnalysis\FieldMetadataNode.cs" />
<Compile Include="Compiler\DependencyAnalysis\ILScanNodeFactory.cs" />
<Compile Include="Compiler\DependencyAnalysis\WrittenStaticFieldNode.cs" />
<Compile Include="Compiler\DictionaryLayoutProvider.cs" />
<Compile Include="Compiler\DirectPInvokePolicy.cs" />
<Compile Include="Compiler\Disassembler.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ internal RyuJitCompilation(

public bool IsInitOnly(FieldDesc field) => _readOnlyFieldPolicy.IsReadOnly(field);

public bool IsNeverWrittenStaticField(FieldDesc field) => _readOnlyFieldPolicy.IsNeverWrittenAtRuntime(field);

public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type)
{
// RyuJIT makes assumptions around the value of these symbols - in particular, it assumes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2289,9 +2289,9 @@ private bool getStaticFieldContent(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buf
}
}
}
else if (!owningType.HasStaticConstructor)
else if (_compilation.IsNeverWrittenStaticField(field))
{
// (Effectively) read only field but no static constructor to set it: the value is default-initialized.
// (Effectively) read only field: it gets never set to anything but the default value.
int size = field.FieldType.GetElementSize().AsInt;
if (size >= bufferSize && valueOffset <= size - bufferSize)
{
Expand Down

0 comments on commit b32d899

Please sign in to comment.