Skip to content

Commit

Permalink
WIP - Code Review Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@cshung
cshung committed Oct 21, 2022
1 parent 43649de commit f17547b
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 176 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/inc/readytorun.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ enum class ReadyToRunSectionType : uint32_t
PgoInstrumentationData = 117, // Added in V5.2
ManifestAssemblyMvids = 118, // Added in V5.3
CrossModuleInlineInfo = 119, // Added in V6.2
HotColdMap = 120, // Added in V6.3
HotColdMap = 120, // Added in V8.0

// If you add a new section consider whether it is a breaking or non-breaking change.
// Usually it is non-breaking, but if it is preferable to have older runtimes fail
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public enum ReadyToRunSectionType
PgoInstrumentationData = 117, // Added in 5.2
ManifestAssemblyMvids = 118, // Added in 5.3
CrossModuleInlineInfo = 119, // Added in 6.3
HotColdMap = 120, // Added in 6.3
HotColdMap = 120, // Added in 8.0

//
// NativeAOT ReadyToRun sections
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ private void PublishCode()
alignment,
new ISymbolDefinitionNode[] { _methodColdCodeNode });
_methodColdCodeNode.SetCode(coldObjectData);
_methodCodeNode.SetColdCodeNode(_methodColdCodeNode);
_methodCodeNode.ColdCodeNode = _methodColdCodeNode;
}
#endif

Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,5 +332,10 @@ public int MaxHomogeneousAggregateElementCount
return 4;
}
}

/// <summary>
/// CodeDelta - encapsulate the fact that ARM requires a thumb bit
/// </summary>
public int CodeDelta { get => (Architecture == TargetArchitecture.ARM) ? 1 : 0; }
}
}
8 changes: 1 addition & 7 deletions ...aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/DelayLoadHelperImport.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,8 @@ public override void EncodeData(ref ObjectDataBuilder dataBuilder, NodeFactory f
{
// This needs to be an empty target pointer since it will be filled in with Module*
// when loaded by CoreCLR
int codeDelta = 0;
if (factory.Target.Architecture == TargetArchitecture.ARM)
{
// THUMB_CODE
codeDelta = 1;
}
dataBuilder.EmitReloc(_delayLoadHelper,
factory.Target.PointerSize == 4 ? RelocType.IMAGE_REL_BASED_HIGHLOW : RelocType.IMAGE_REL_BASED_DIR64, codeDelta);
factory.Target.PointerSize == 4 ? RelocType.IMAGE_REL_BASED_HIGHLOW : RelocType.IMAGE_REL_BASED_DIR64, factory.Target.CodeDelta);
}

public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
Expand Down
15 changes: 13 additions & 2 deletions .../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/HotColdMapNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,29 @@
using System;

using Internal.Text;
using System.Diagnostics;

namespace ILCompiler.DependencyAnalysis.ReadyToRun
{
public class HotColdMapNode : HeaderTableNode
{
public uint[] mapping;
private uint[] _mapping;

public HotColdMapNode(NodeFactory nodeFactory)
: base(nodeFactory.Target)
{
}

public uint[] Mapping
{
get => _mapping;
set
{
Debug.Assert(_mapping == null);
_mapping = value;
}
}

public override int ClassCode => 28963035;

public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
Expand All @@ -32,7 +43,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)

ObjectDataBuilder builder = new ObjectDataBuilder(factory, relocsOnly);
builder.AddSymbol(this);
foreach (uint m in this.mapping)
foreach (uint m in this._mapping)
{
builder.EmitUInt(m);
}
Expand Down
134 changes: 49 additions & 85 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodGCInfoNode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected override void OnMarked(NodeFactory factory)
public int[] CalculateFuncletOffsets(NodeFactory factory)
{
int coldCodeUnwindInfoCount = 0;
if (_methodNode.GetColdCodeNode() != null)
if (_methodNode.ColdCodeNode != null)
{
coldCodeUnwindInfoCount = _methodNode.ColdFrameInfos.Length;
}
Expand Down Expand Up @@ -68,7 +68,7 @@ public int[] CalculateFuncletOffsets(NodeFactory factory)
}
}

if (_methodNode.GetColdCodeNode() != null)
if (_methodNode.ColdCodeNode != null)
{
// TODO: Take a look at deduplicatedResult
for (int frameInfoIndex = 0; frameInfoIndex < _methodNode.ColdFrameInfos.Length; frameInfoIndex++)
Expand Down Expand Up @@ -156,104 +156,68 @@ private IEnumerable<GCInfoComponent> EncodeDataCore(NodeFactory factory)
{
TargetArchitecture targetArch = factory.Target.Architecture;

for (int frameInfoIndex = 0; frameInfoIndex < _methodNode.FrameInfos.Length; frameInfoIndex++)
int numHotFrameInfos = _methodNode.FrameInfos.Length;
int numFrameInfos = numHotFrameInfos;
if (_methodNode.ColdCodeNode != null)
{
byte[] unwindInfo = _methodNode.FrameInfos[frameInfoIndex].BlobData;

if (targetArch == TargetArchitecture.X64)
{
// On Amd64, patch the first byte of the unwind info by setting the flags to EHANDLER | UHANDLER
// as that's what CoreCLR does (zapcode.cpp, ZapUnwindData::Save).
const byte UNW_FLAG_EHANDLER = 1;
const byte UNW_FLAG_UHANDLER = 2;
const byte FlagsShift = 3;
numFrameInfos += _methodNode.ColdFrameInfos.Length;
}

unwindInfo[0] |= (byte)((UNW_FLAG_EHANDLER | UNW_FLAG_UHANDLER) << FlagsShift);
}
else if ((targetArch == TargetArchitecture.ARM) || (targetArch == TargetArchitecture.ARM64) || (targetArch == TargetArchitecture.LoongArch64))
const byte UNW_FLAG_EHANDLER = 1;
const byte UNW_FLAG_UHANDLER = 2;
const byte UNW_FLAG_CHAININFO = 4;
const byte FlagsShift = 3;

for (int frameInfoIndex = 0; frameInfoIndex < numFrameInfos; frameInfoIndex++)
{
FrameInfo frameInfo = (frameInfoIndex >= numHotFrameInfos) ?
_methodNode.ColdFrameInfos[frameInfoIndex - numHotFrameInfos] :
_methodNode.FrameInfos[frameInfoIndex];
byte[] unwindInfo = frameInfo.BlobData;
if (unwindInfo == null)
{
// Set the 'X' bit to indicate that there is a personality routine associated with this method
unwindInfo[2] |= 1 << 4;
// Chain unwind info
byte[] header = new byte[4];
int i = 0;
header[i++] = 1 + (UNW_FLAG_CHAININFO << FlagsShift); // Version = 1, UNW_FLAG_CHAININFO
header[i++] = 0; // SizeOfProlog = 0
header[i++] = 0; // CountOfCode = 0
header[i++] = _methodNode.FrameInfos[0].BlobData[3]; // Copying frame and frame offset from main function
yield return new GCInfoComponent(header);
yield return new GCInfoComponent(_methodNode, 0);
yield return new GCInfoComponent(_methodNode, _methodNode.Size);
// TODO: Is this correct?
yield return new GCInfoComponent(factory.RuntimeFunctionsGCInfo.StartSymbol, this.OffsetFromBeginningOfArray);
}

yield return new GCInfoComponent(unwindInfo);

if (targetArch != TargetArchitecture.X86)
else
{
bool isFilterFunclet = (_methodNode.FrameInfos[frameInfoIndex].Flags & FrameInfoFlags.Filter) != 0;
ISymbolNode personalityRoutine = (isFilterFunclet ? factory.FilterFuncletPersonalityRoutine : factory.PersonalityRoutine);
int codeDelta = 0;
if (targetArch == TargetArchitecture.ARM)
if (targetArch == TargetArchitecture.X64)
{
// THUMB_CODE
codeDelta = 1;
// On Amd64, patch the first byte of the unwind info by setting the flags to EHANDLER | UHANDLER
// as that's what CoreCLR does (zapcode.cpp, ZapUnwindData::Save).
unwindInfo[0] |= (byte)((UNW_FLAG_EHANDLER | UNW_FLAG_UHANDLER) << FlagsShift);
}
yield return new GCInfoComponent(personalityRoutine, codeDelta);
}

if (frameInfoIndex == 0 && _methodNode.GCInfo != null)
{
yield return new GCInfoComponent(_methodNode.GCInfo);
}
}
#if READYTORUN
if (_methodNode.GetColdCodeNode() != null)
{
for (int frameInfoIndex = 0; frameInfoIndex < _methodNode.ColdFrameInfos.Length; frameInfoIndex++)
{
byte[] unwindInfo = _methodNode.ColdFrameInfos[frameInfoIndex].BlobData;

if (unwindInfo == null)
else if ((targetArch == TargetArchitecture.ARM) || (targetArch == TargetArchitecture.ARM64) || (targetArch == TargetArchitecture.LoongArch64))
{
// Chain unwind info
byte[] header = new byte[4];
int i = 0;
header[i++] = 1 + (4 << 3); // Version = 1, UNW_FLAG_CHAININFO
header[i++] = 0; // SizeOfProlog = 0
header[i++] = 0; // CountOfCode = 0
header[i++] = _methodNode.FrameInfos[0].BlobData[3]; // Copying frame and frame offset from main function
yield return new GCInfoComponent(header);
yield return new GCInfoComponent(_methodNode, 0);
yield return new GCInfoComponent(_methodNode, _methodNode.Size);
// TODO: Is this correct?
yield return new GCInfoComponent(factory.RuntimeFunctionsGCInfo.StartSymbol, this.OffsetFromBeginningOfArray);
// Set the 'X' bit to indicate that there is a personality routine associated with this method
unwindInfo[2] |= 1 << 4;
}
else
{
if (targetArch == TargetArchitecture.X64)
{
// On Amd64, patch the first byte of the unwind info by setting the flags to EHANDLER | UHANDLER
// as that's what CoreCLR does (zapcode.cpp, ZapUnwindData::Save).
const byte UNW_FLAG_EHANDLER = 1;
const byte UNW_FLAG_UHANDLER = 2;
const byte FlagsShift = 3;

unwindInfo[0] |= (byte)((UNW_FLAG_EHANDLER | UNW_FLAG_UHANDLER) << FlagsShift);
}
else if ((targetArch == TargetArchitecture.ARM) || (targetArch == TargetArchitecture.ARM64))
{
// Set the 'X' bit to indicate that there is a personality routine associated with this method
unwindInfo[2] |= 1 << 4;
}
yield return new GCInfoComponent(unwindInfo);

yield return new GCInfoComponent(unwindInfo);
if (targetArch != TargetArchitecture.X86)
{
bool isFilterFunclet = (frameInfo.Flags & FrameInfoFlags.Filter) != 0;
ISymbolNode personalityRoutine = (isFilterFunclet ? factory.FilterFuncletPersonalityRoutine : factory.PersonalityRoutine);
yield return new GCInfoComponent(personalityRoutine, factory.Target.CodeDelta);
}

if (targetArch != TargetArchitecture.X86)
{
bool isFilterFunclet = (_methodNode.ColdFrameInfos[frameInfoIndex].Flags & FrameInfoFlags.Filter) != 0;
ISymbolNode personalityRoutine = (isFilterFunclet ? factory.FilterFuncletPersonalityRoutine : factory.PersonalityRoutine);
int codeDelta = 0;
if (targetArch == TargetArchitecture.ARM)
{
// THUMB_CODE
codeDelta = 1;
}
yield return new GCInfoComponent(personalityRoutine, codeDelta);
}
if (frameInfoIndex == 0 && _methodNode.GCInfo != null)
{
yield return new GCInfoComponent(_methodNode.GCInfo);
}
}
}
#endif
}

class MethodGCInfoNodeDeduplicatingComparer : IEqualityComparer<MethodGCInfoNode>
Expand Down
30 changes: 5 additions & 25 deletions ...ools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,8 @@ public class MethodWithGCInfo : ObjectNode, IMethodBodyNode, ISymbolDefinitionNo
private readonly MethodDesc _method;

private ObjectData _methodCode;
#if READYTORUN
private MethodColdCodeNode _methodColdCodeNode;
#endif
private FrameInfo[] _frameInfos;
#if READYTORUN
private FrameInfo[] _coldFrameInfos;
#endif
private byte[] _gcInfo;
private ObjectData _ehInfo;
private byte[] _debugLocInfos;
Expand All @@ -50,9 +45,7 @@ protected override void OnMarked(NodeFactory context)
{
SetCode(new ObjectNode.ObjectData(Array.Empty<byte>(), null, 1, Array.Empty<ISymbolDefinitionNode>()));
InitializeFrameInfos(Array.Empty<FrameInfo>());
#if READYTORUN
InitializeColdFrameInfos(Array.Empty<FrameInfo>());
#endif
}
_lateTriggeredCompilation = context.CompilationCurrentPhase != 0;
RegisterInlineeModuleIndices(context);
Expand Down Expand Up @@ -139,16 +132,15 @@ public int Compare(FixupCell a, FixupCell b)
}
}

public MethodColdCodeNode GetColdCodeNode() => _methodColdCodeNode;
public MethodColdCodeNode ColdCodeNode { get; set; }

public byte[] GetFixupBlob(NodeFactory factory)
{
Relocation[] relocations = GetData(factory, relocsOnly: true).Relocs;

#if READYTORUN
if (_methodColdCodeNode != null)
if (ColdCodeNode != null)
{
Relocation[] coldRelocations = _methodColdCodeNode.GetData(factory, relocsOnly: true).Relocs;
Relocation[] coldRelocations = ColdCodeNode.GetData(factory, relocsOnly: true).Relocs;
if (relocations == null)
{
relocations = coldRelocations;
Expand All @@ -158,7 +150,6 @@ public byte[] GetFixupBlob(NodeFactory factory)
relocations = Enumerable.Concat(relocations, coldRelocations).ToArray();
}
}
#endif

if (relocations == null)
{
Expand Down Expand Up @@ -271,9 +262,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
{
DependencyList dependencyList = new DependencyList(new DependencyListEntry[] { new DependencyListEntry(GCInfoNode, "Unwind & GC info") });

if (this.GetColdCodeNode() != null)
if (this.ColdCodeNode != null)
{
dependencyList.Add(this.GetColdCodeNode(), "cold");
dependencyList.Add(this.ColdCodeNode, "cold");
}

foreach (ISymbolNode node in _fixups)
Expand Down Expand Up @@ -312,9 +303,7 @@ public override ObjectNodeSection Section

public FrameInfo[] FrameInfos => _frameInfos;

#if READYTORUN
public FrameInfo[] ColdFrameInfos => _coldFrameInfos;
#endif

public byte[] GCInfo => _gcInfo;
public ObjectData EHInfo => _ehInfo;
Expand All @@ -337,14 +326,12 @@ public void InitializeFrameInfos(FrameInfo[] frameInfos)
}
}

#if READYTORUN
public void InitializeColdFrameInfos(FrameInfo[] coldFrameInfos)
{
Debug.Assert(_coldFrameInfos == null);
_coldFrameInfos = coldFrameInfos;
// TODO: x86 (see InitializeFrameInfos())
}
#endif

public void InitializeGCInfo(byte[] gcInfo)
{
Expand Down Expand Up @@ -403,12 +390,5 @@ public void InitializeInliningInfo(MethodDesc[] inlinedMethods, NodeFactory fact
public override bool ShouldSkipEmittingObjectNode(NodeFactory factory) => IsEmpty;

public override string ToString() => _method.ToString();

#if READYTORUN
public void SetColdCodeNode(MethodColdCodeNode methodColdCodeNode)
{
_methodColdCodeNode = methodColdCodeNode;
}
#endif
}
}
Loading

0 comments on commit f17547b

Please sign in to comment.