From c27618f978c9d61c01c9eacbe04099d1756a71d4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 15 Sep 2026 14:04:47 -0700 Subject: [PATCH 1/2] JIT: Fix truncated histogram likelihoods Truncated histograms assigned omitted probability to the first entry, artificially inflating its likelihood. Only normalize rounding error when the full histogram is returned. Fixes #133973 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/likelyclass.cpp | 29 ++- src/tests/JIT/PGO/LikelyClass/LikelyClass.cs | 188 ++++++++++++++++++ .../JIT/PGO/LikelyClass/LikelyClass.csproj | 16 ++ 3 files changed, 222 insertions(+), 11 deletions(-) create mode 100644 src/tests/JIT/PGO/LikelyClass/LikelyClass.cs create mode 100644 src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj diff --git a/src/coreclr/jit/likelyclass.cpp b/src/coreclr/jit/likelyclass.cpp index b3b86804f55093..130fd59f85bc49 100644 --- a/src/coreclr/jit/likelyclass.cpp +++ b/src/coreclr/jit/likelyclass.cpp @@ -287,9 +287,8 @@ static unsigned getLikelyClassesOrMethods(LikelyClassMethodRecord* assert(totalLikelihood <= 100); - // Distribute the rounding error and just apply it to the first entry. - // Assume that there is no error If we have unknown handles. - if (!containsUnknownHandles) + // Distribute the rounding error only if the returned entries represent the entire histogram. + if ((numberOfClasses == knownHandles) && !containsUnknownHandles) { assert(numberOfClasses > 0); assert(totalLikelihood > 0); @@ -326,10 +325,11 @@ static unsigned getLikelyClassesOrMethods(LikelyClassMethodRecord* // ilOffset - il offset of the callvirt // // Returns: -// Estimated number of classes seen at runtime +// Number of likely class records written to pLikelyClasses // // Notes: // A "monomorphic" call site will return likelihood 100 and number of entries = 1. +// Returned likelihoods reflect the observed proportions and are not guaranteed to sum to 100. // // This is used by the devirtualization logic below, and by crossgen2 when producing // the R2R image (to reduce the sizecost of carrying the type histogram) @@ -372,14 +372,17 @@ extern "C" DLLEXPORT UINT32 WINAPI getLikelyMethods(LikelyClassMethodRecord* // at least of 'maxLikelyValues' (next argument) length. // The array consists of pairs "value - likelihood" ordered by likelihood // (descending) where likelihood can be any value in [0..100] range. -// maxLikelyValues - limit for likely classes to output +// maxLikelyValues - limit for likely values to output // schema - profile schema // countSchemaItems - number of items in the schema // pInstrumentationData - associated data // ilOffset - il offset of the node of interest // // Returns: -// Estimated number of different constants seen at runtime +// Number of likely value records written to pLikelyValues +// +// Notes: +// Returned likelihoods reflect the observed proportions and are not guaranteed to sum to 100. // extern "C" DLLEXPORT UINT32 WINAPI getLikelyValues(LikelyValueRecord* pLikelyValues, UINT32 maxLikelyValues, @@ -446,11 +449,15 @@ extern "C" DLLEXPORT UINT32 WINAPI getLikelyValues(LikelyValueRecord* assert(totalLikelihood <= 100); - // Distribute the rounding error and just apply it to the first entry. - assert(numberOfLikelyConst > 0); - assert(totalLikelihood > 0); - pLikelyValues[0].likelihood += 100 - totalLikelihood; - assert(pLikelyValues[0].likelihood <= 100); + // Distribute the rounding error only if the returned entries represent the entire histogram. + if (numberOfLikelyConst == h.countHistogramElements) + { + assert(numberOfLikelyConst > 0); + assert(totalLikelihood > 0); + pLikelyValues[0].likelihood += 100 - totalLikelihood; + assert(pLikelyValues[0].likelihood <= 100); + } + return numberOfLikelyConst; } } diff --git a/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs b/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs new file mode 100644 index 00000000000000..38784459a9d855 --- /dev/null +++ b/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs @@ -0,0 +1,188 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +public unsafe class LikelyClassTests +{ + private const int ILOffset = 42; + private const string JitLibrary = "clrjit"; + + public static int Main() + { + foreach (bool types in new[] { true, false }) + { + TruncatedHistogramReportsActualLikelihoods(types); + SingleRecordReportsActualLikelihood(types); + CompleteHistogramNormalizesRoundingError(types); + UnknownHandlesKeepResidualLikelihood(types); + } + + return 100; + } + + private static void TruncatedHistogramReportsActualLikelihoods(bool types) + { + nint[] histogram = new nint[10]; + for (int i = 0; i < histogram.Length; i++) + { + histogram[i] = 100 + i; + } + + LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[5]; + + AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); + for (int i = 0; i < records.Length; i++) + { + AssertEqual(10u, records[i].Likelihood); + } + } + + private static void SingleRecordReportsActualLikelihood(bool types) + { + nint[] histogram = [100, 100, 200, 200, 300, 300, 400, 400]; + LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[1]; + + AssertEqual(1u, GetLikelyRecords(types, histogram, records)); + AssertEqual(25u, records[0].Likelihood); + } + + private static void CompleteHistogramNormalizesRoundingError(bool types) + { + nint[] histogram = [100, 100, 100, 200, 200, 300]; + LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[3]; + + AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); + AssertEqual((nint)100, records[0].Handle); + AssertEqual((nint)200, records[1].Handle); + AssertEqual((nint)300, records[2].Handle); + AssertEqual(51u, records[0].Likelihood); + AssertEqual(33u, records[1].Likelihood); + AssertEqual(16u, records[2].Likelihood); + } + + private static void UnknownHandlesKeepResidualLikelihood(bool types) + { + nint[] histogram = [100, 100, 100, 200, 200, 1]; + LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[2]; + + AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); + AssertEqual((nint)100, records[0].Handle); + AssertEqual((nint)200, records[1].Handle); + AssertEqual(50u, records[0].Likelihood); + AssertEqual(33u, records[1].Likelihood); + } + + private static void AssertEqual(uint expected, uint actual) + { + if (actual != expected) + { + throw new InvalidOperationException($"Expected {expected}, actual {actual}."); + } + } + + private static void AssertEqual(nint expected, nint actual) + { + if (actual != expected) + { + throw new InvalidOperationException($"Expected {expected}, actual {actual}."); + } + } + + private static uint GetLikelyRecords( + bool types, + nint[] histogram, + LikelyClassMethodRecord[] records) + { + PgoInstrumentationKind histogramKind = types + ? PgoInstrumentationKind.HandleHistogramTypes + : PgoInstrumentationKind.HandleHistogramMethods; + PgoInstrumentationSchema[] schema = + [ + new() + { + InstrumentationKind = PgoInstrumentationKind.HandleHistogramIntCount, + ILOffset = ILOffset, + Count = 1, + }, + new() + { + InstrumentationKind = histogramKind, + ILOffset = ILOffset, + Count = histogram.Length, + }, + ]; + + fixed (nint* pHistogram = histogram) + fixed (LikelyClassMethodRecord* pRecords = records) + fixed (PgoInstrumentationSchema* pSchema = schema) + { + if (types) + { + return GetLikelyClasses( + pRecords, + (uint)records.Length, + pSchema, + (uint)schema.Length, + (byte*)pHistogram, + ILOffset); + } + + return GetLikelyMethods( + pRecords, + (uint)records.Length, + pSchema, + (uint)schema.Length, + (byte*)pHistogram, + ILOffset); + } + } + + [DllImport(JitLibrary, EntryPoint = "getLikelyClasses")] + private static extern uint GetLikelyClasses( + LikelyClassMethodRecord* records, + uint maxRecords, + PgoInstrumentationSchema* schema, + uint schemaCount, + byte* instrumentationData, + int ilOffset); + + [DllImport(JitLibrary, EntryPoint = "getLikelyMethods")] + private static extern uint GetLikelyMethods( + LikelyClassMethodRecord* records, + uint maxRecords, + PgoInstrumentationSchema* schema, + uint schemaCount, + byte* instrumentationData, + int ilOffset); + + private enum PgoInstrumentationKind + { + FourByte = 1, + TypeHandle = 3, + MethodHandle = 4, + AlignPointer = 0x30, + DescriptorMin = 0x40, + HandleHistogramIntCount = (DescriptorMin * 2) | FourByte | AlignPointer, + HandleHistogramTypes = (DescriptorMin * 3) | TypeHandle, + HandleHistogramMethods = (DescriptorMin * 3) | MethodHandle, + } + + [StructLayout(LayoutKind.Sequential)] + private struct PgoInstrumentationSchema + { + public nuint Offset; + public PgoInstrumentationKind InstrumentationKind; + public int ILOffset; + public int Count; + public int Other; + } + + [StructLayout(LayoutKind.Sequential)] + private struct LikelyClassMethodRecord + { + public nint Handle; + public uint Likelihood; + } +} diff --git a/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj b/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj new file mode 100644 index 00000000000000..6b76bbc2de9659 --- /dev/null +++ b/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj @@ -0,0 +1,16 @@ + + + true + 1 + true + true + true + true + Exe + false + true + + + + + From 33fa00f1e42a27456d1d7f642a9c80beda7e80d4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 15 Sep 2026 14:21:32 -0700 Subject: [PATCH 2/2] JIT: Remove fragile histogram test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/tests/JIT/PGO/LikelyClass/LikelyClass.cs | 188 ------------------ .../JIT/PGO/LikelyClass/LikelyClass.csproj | 16 -- 2 files changed, 204 deletions(-) delete mode 100644 src/tests/JIT/PGO/LikelyClass/LikelyClass.cs delete mode 100644 src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj diff --git a/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs b/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs deleted file mode 100644 index 38784459a9d855..00000000000000 --- a/src/tests/JIT/PGO/LikelyClass/LikelyClass.cs +++ /dev/null @@ -1,188 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime.InteropServices; - -public unsafe class LikelyClassTests -{ - private const int ILOffset = 42; - private const string JitLibrary = "clrjit"; - - public static int Main() - { - foreach (bool types in new[] { true, false }) - { - TruncatedHistogramReportsActualLikelihoods(types); - SingleRecordReportsActualLikelihood(types); - CompleteHistogramNormalizesRoundingError(types); - UnknownHandlesKeepResidualLikelihood(types); - } - - return 100; - } - - private static void TruncatedHistogramReportsActualLikelihoods(bool types) - { - nint[] histogram = new nint[10]; - for (int i = 0; i < histogram.Length; i++) - { - histogram[i] = 100 + i; - } - - LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[5]; - - AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); - for (int i = 0; i < records.Length; i++) - { - AssertEqual(10u, records[i].Likelihood); - } - } - - private static void SingleRecordReportsActualLikelihood(bool types) - { - nint[] histogram = [100, 100, 200, 200, 300, 300, 400, 400]; - LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[1]; - - AssertEqual(1u, GetLikelyRecords(types, histogram, records)); - AssertEqual(25u, records[0].Likelihood); - } - - private static void CompleteHistogramNormalizesRoundingError(bool types) - { - nint[] histogram = [100, 100, 100, 200, 200, 300]; - LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[3]; - - AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); - AssertEqual((nint)100, records[0].Handle); - AssertEqual((nint)200, records[1].Handle); - AssertEqual((nint)300, records[2].Handle); - AssertEqual(51u, records[0].Likelihood); - AssertEqual(33u, records[1].Likelihood); - AssertEqual(16u, records[2].Likelihood); - } - - private static void UnknownHandlesKeepResidualLikelihood(bool types) - { - nint[] histogram = [100, 100, 100, 200, 200, 1]; - LikelyClassMethodRecord[] records = new LikelyClassMethodRecord[2]; - - AssertEqual((uint)records.Length, GetLikelyRecords(types, histogram, records)); - AssertEqual((nint)100, records[0].Handle); - AssertEqual((nint)200, records[1].Handle); - AssertEqual(50u, records[0].Likelihood); - AssertEqual(33u, records[1].Likelihood); - } - - private static void AssertEqual(uint expected, uint actual) - { - if (actual != expected) - { - throw new InvalidOperationException($"Expected {expected}, actual {actual}."); - } - } - - private static void AssertEqual(nint expected, nint actual) - { - if (actual != expected) - { - throw new InvalidOperationException($"Expected {expected}, actual {actual}."); - } - } - - private static uint GetLikelyRecords( - bool types, - nint[] histogram, - LikelyClassMethodRecord[] records) - { - PgoInstrumentationKind histogramKind = types - ? PgoInstrumentationKind.HandleHistogramTypes - : PgoInstrumentationKind.HandleHistogramMethods; - PgoInstrumentationSchema[] schema = - [ - new() - { - InstrumentationKind = PgoInstrumentationKind.HandleHistogramIntCount, - ILOffset = ILOffset, - Count = 1, - }, - new() - { - InstrumentationKind = histogramKind, - ILOffset = ILOffset, - Count = histogram.Length, - }, - ]; - - fixed (nint* pHistogram = histogram) - fixed (LikelyClassMethodRecord* pRecords = records) - fixed (PgoInstrumentationSchema* pSchema = schema) - { - if (types) - { - return GetLikelyClasses( - pRecords, - (uint)records.Length, - pSchema, - (uint)schema.Length, - (byte*)pHistogram, - ILOffset); - } - - return GetLikelyMethods( - pRecords, - (uint)records.Length, - pSchema, - (uint)schema.Length, - (byte*)pHistogram, - ILOffset); - } - } - - [DllImport(JitLibrary, EntryPoint = "getLikelyClasses")] - private static extern uint GetLikelyClasses( - LikelyClassMethodRecord* records, - uint maxRecords, - PgoInstrumentationSchema* schema, - uint schemaCount, - byte* instrumentationData, - int ilOffset); - - [DllImport(JitLibrary, EntryPoint = "getLikelyMethods")] - private static extern uint GetLikelyMethods( - LikelyClassMethodRecord* records, - uint maxRecords, - PgoInstrumentationSchema* schema, - uint schemaCount, - byte* instrumentationData, - int ilOffset); - - private enum PgoInstrumentationKind - { - FourByte = 1, - TypeHandle = 3, - MethodHandle = 4, - AlignPointer = 0x30, - DescriptorMin = 0x40, - HandleHistogramIntCount = (DescriptorMin * 2) | FourByte | AlignPointer, - HandleHistogramTypes = (DescriptorMin * 3) | TypeHandle, - HandleHistogramMethods = (DescriptorMin * 3) | MethodHandle, - } - - [StructLayout(LayoutKind.Sequential)] - private struct PgoInstrumentationSchema - { - public nuint Offset; - public PgoInstrumentationKind InstrumentationKind; - public int ILOffset; - public int Count; - public int Other; - } - - [StructLayout(LayoutKind.Sequential)] - private struct LikelyClassMethodRecord - { - public nint Handle; - public uint Likelihood; - } -} diff --git a/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj b/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj deleted file mode 100644 index 6b76bbc2de9659..00000000000000 --- a/src/tests/JIT/PGO/LikelyClass/LikelyClass.csproj +++ /dev/null @@ -1,16 +0,0 @@ - - - true - 1 - true - true - true - true - Exe - false - true - - - - -