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

Fix logic error in PGO schema comparison #52822

Merged
merged 3 commits into from
May 17, 2021
+11 −25
Merged

Fix logic error in PGO schema comparison #52822

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3a977c
Fix logic error in PGO schema comparison
AndyAyersMS May 16, 2021
da339c9
Revert "Fix logic error in PGO schema comparison"
AndyAyersMS May 17, 2021
2b6e816
Fix merging of edge counts
jakobbotsch May 17, 2021
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
36 changes: 11 additions & 25 deletions src/coreclr/tools/Common/Pgo/PgoFormat.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public enum PgoInstrumentationKind
AlignMask = 0x30,

DescriptorMin = 0x40,
DescriptorMask = ~(MarshalMask | AlignMask),

Done = None, // All instrumentation schemas must end with a record which is "Done"
BasicBlockIntCount = (DescriptorMin * 1) | FourByte, // basic block counter using unsigned 4 byte int
Expand Down Expand Up @@ -474,29 +475,22 @@ private class PgoSchemaMergeComparer : IComparer<PgoSchemaElem>, IEqualityCompar
{
public static PgoSchemaMergeComparer Singleton = new PgoSchemaMergeComparer();

private static bool SchemaMergesItemsWithDifferentOtherFields(PgoInstrumentationKind kind)
{
switch (kind)
{
//
default:
// All non-specified kinds are not distinguishable by Other field
return false;
}
}

public int Compare(PgoSchemaElem x, PgoSchemaElem y)
{
if (x.ILOffset != y.ILOffset)
{
return x.ILOffset.CompareTo(y.ILOffset);
}
if (x.InstrumentationKind != y.InstrumentationKind)
PgoInstrumentationKind xdescr = x.InstrumentationKind & PgoInstrumentationKind.DescriptorMask;
PgoInstrumentationKind ydescr = y.InstrumentationKind & PgoInstrumentationKind.DescriptorMask;
if (xdescr != ydescr)
{
return x.InstrumentationKind.CompareTo(y.InstrumentationKind);
return xdescr.CompareTo(ydescr);
}
// Some InstrumentationKinds may be compared based on the Other field, some may not
if (x.Other != y.Other && SchemaMergesItemsWithDifferentOtherFields(x.InstrumentationKind))
// We usually merge the Other field, except for edges, where we take care only to merge
// edges with equal ILOffset _and_ equal Other fields.
if ((x.InstrumentationKind == PgoInstrumentationKind.EdgeIntCount || x.InstrumentationKind == PgoInstrumentationKind.EdgeLongCount)
&& x.Other != y.Other)
{
return x.Other.CompareTo(y.Other);
}
Expand All @@ -505,16 +499,8 @@ public int Compare(PgoSchemaElem x, PgoSchemaElem y)
}

public bool Equals(PgoSchemaElem x, PgoSchemaElem y)
{
if (x.ILOffset != y.ILOffset)
return false;
if (x.InstrumentationKind != y.InstrumentationKind)
return false;
if (x.InstrumentationKind != y.InstrumentationKind && SchemaMergesItemsWithDifferentOtherFields(x.InstrumentationKind))
return false;
return true;
}
int IEqualityComparer<PgoSchemaElem>.GetHashCode(PgoSchemaElem obj) => obj.ILOffset ^ ((int)obj.InstrumentationKind << 20);
=> Compare(x, y) == 0;
int IEqualityComparer<PgoSchemaElem>.GetHashCode(PgoSchemaElem obj) => obj.ILOffset ^ ((int)(obj.InstrumentationKind & PgoInstrumentationKind.DescriptorMask) << 20);
}

public static PgoSchemaElem[] Merge<TType>(ReadOnlySpan<PgoSchemaElem[]> schemasToMerge)
Expand Down