Skip to content

JIT: enable instrumentation for inlinees - #119658

Merged
AndyAyersMS merged 5 commits into
dotnet:mainfrom
AndyAyersMS:InstrumentInlinees
Sep 22, 2025
Merged

JIT: enable instrumentation for inlinees#119658
AndyAyersMS merged 5 commits into
dotnet:mainfrom
AndyAyersMS:InstrumentInlinees

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Sep 12, 2025

Copy link
Copy Markdown
Member

If we are doing an optimized+instrumented jit pass (like we do for R2R methods) allow the inlinees to be instrumented. This gives us a chance to collect profile data for methods that are always inlined.

All inlinees currently share the same profile data segment with each other and with a root compilation of the method (if any). So this profile is "context-free".

If there is a schema mismatch (say there is a stale pre-existing R2R schema) then we disregard the old schema and use the new one.

This is mostly just removing assertions and bailouts and bad assumptions. However if the inlinee has an instrumentable call in its return expression we need to temporarily insert it as normal IR so it can get instrumented like anything else. We then undo this after instrumentation. Any residual impact from instrumentation will be left either in prior statements or commas inside the return expression.

Closes#44372. Closes#91938.

If we are doing an optimized+instrumented jit pass (like we do for R2R methods)
allow the inlinees to be instrumented. This gives us a chance to collect profile
data for methods that are always inlined.
All inlinees currently share the same profile data segment with each other and
with a root compilation of the method (if any). So this profile is "context-free".
If there is a schema mismatch (say there is a stale pre-existing R2R schema) then
subsequent instrumentation will fail. This is something we need to keep an eye on.
Right now we can't distinguish this failure from other kinds of schema allocation
failures.
Closesdotnet#44372
@github-actionsgithub-actionsBot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 12, 2025
@AndyAyersMS

AndyAyersMS commented Sep 12, 2025

Copy link
Copy Markdown
MemberAuthor

This should address the long-standing regression in #91938. Local runs show it possibly helps but doesn't completely fix the regression. Still need to verify it is actually getting instrumentation data for the key inlinees.

Generally speaking, this should help reduce the gap between running with R2R enabled (where we can lose profile data, or optimize based on stale data) and R2R disabled.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

I have a bunch more fixes on top of this, will PR them soon.

One issue I'm seeing is IL mismatches. Some of it I've tracked down to IL differences between the PGO schema embedded into corelib and the current corelib IL; that's somewhat understandable as the PGO data likely goes stale, and should be (generally) a non-product issue.

Would still be good to flag this case and allow the dynamic PGO to take over.

I also suspect inlinee schemas may diverge from root schemas even for the same IL, but need to do more work to track this down.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Still some failures to sort out. Also my local perf runs aren't as promising as I'd hoped.

@AndyAyersMS

AndyAyersMS commented Sep 18, 2025

Copy link
Copy Markdown
MemberAuthor

Ok, think I tracked down the last few issues.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Local run. Regression fixed. TC now matches no-R2R perf here.

MethodRuntimeOptionsMeanErrorStdDevMedianMinMaxRatioRatioSDAllocatedAlloc Ratio
LastIndexOf_Word_NotFoundPR(en-US, OrdinalIgnoreCase, False)574.6 ns9.34 ns8.74 ns573.9 ns562.4 ns588.8 ns0.890.02-NA
LastIndexOf_Word_NotFound.NET 10.0(en-US, OrdinalIgnoreCase, False)784.9 ns13.04 ns12.19 ns785.1 ns770.8 ns807.0 ns1.210.02-NA
LastIndexOf_Word_NotFound.NET 7.0(en-US, OrdinalIgnoreCase, False)647.1 ns9.97 ns9.33 ns645.9 ns632.7 ns666.0 ns1.000.02-NA

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

@EgorBot --intel --arm --filter System.Globalization.Tests.StringSearch.LastIndexOf_Word_NotFound(Options: (en-US, OrdinalIgnoreCase, False))

@EgorBo

Copy link
Copy Markdown
Member

@EgorBot --intel --arm --filter System.Globalization.Tests.StringSearch.LastIndexOf_Word_NotFound(Options: (en-US, OrdinalIgnoreCase, False))

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

@EgorBot --intel --arm --filter System.Globalization.Tests.StringSearch.LastIndexOf_Word_NotFound

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr libraries-pgo, runtime-coreclr pgostress, runtime-coreclr pgo

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

@AndyAyersMS
AndyAyersMS marked this pull request as ready for review September 18, 2025 19:01
CopilotAI review requested due to automatic review settings September 18, 2025 19:01

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables instrumentation for inlined methods (inlinees) when performing optimized+instrumented JIT passes, such as R2R methods. This allows collection of profile data for methods that are always inlined, providing "context-free" profile data.

Key changes:

  • Removes assertions and bailouts that prevented inlinee instrumentation
  • Handles schema mismatches by discarding stale data and using new schemas
  • Adds special handling for inlinee return expressions during instrumentation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/coreclr/vm/pgo.cppUpdates PGO allocation to handle schema mismatches by removing stale data and relaying schemas
src/coreclr/vm/jitinterface.cppChanges PGO allocation to use method handle instead of method being compiled
src/coreclr/jit/jitconfigvalues.hAdds JitInstrumentIfOptimizing configuration option
src/coreclr/jit/importercalls.cppRemoves inlining restrictions for instrumentation and tail call handling
src/coreclr/jit/fgprofile.cppRemoves inlining restrictions and adds wrapper for inlinee return expression handling
src/coreclr/jit/fginline.cppRemoves clearing of instrumentation flags for inlinees
src/coreclr/jit/compiler.hAdds fgInstrumentMethodCore method declaration
src/coreclr/jit/compiler.cppRemoves inlining assertions and adds debug instrumentation forcing
src/coreclr/inc/pgo_formatprocessing.hChanges schema compatibility check to return match count instead of boolean

Comment threadsrc/coreclr/inc/pgo_formatprocessing.h
Comment threadsrc/coreclr/jit/fgprofile.cpp Outdated
Comment threadsrc/coreclr/jit/fgprofile.cpp Outdated
@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

@dotnet/jit-contrib PTAL

There will be a sizable code size / tp increase in some collections.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Libraries-pgo failure looks like a GC hole but suspect it is something pre-existing that we're just uncovering here.

Pgo failure is in stackoverflow tester.

Diffs. As promised large size increases, though the bulk of that is from libraries tests and (not surprisingly) mostly in Tier1-instr.

A decent fraction of context misses too, so the accuracy of SPMI is a bit questionable. I am not really sure how to do a fair assessment here as the new data gathered by this instrumentation should have a big effect on Tier1.

@EgorBo

Copy link
Copy Markdown
Member

@EgorBot -amd -arm

usingSystem;usingSystem.Runtime.CompilerServices;usingSystem.Runtime.InteropServices;usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Running;publicclassProg{staticvoidMain(string[]args){BenchmarkSwitcher.FromAssembly(typeof(Prog).Assembly).Run(args);}byte[]Src=newbyte[32];byte[]Dst=newbyte[32];[Benchmark]publicvoidCopy()=>Src.AsSpan().CopyTo(Dst);}

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

@EgorBo PTAL

@EgorBoEgorBo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment threadsrc/coreclr/inc/pgo_formatprocessing.h
@AndyAyersMS
AndyAyersMS merged commit ad22abb into dotnet:mainSep 22, 2025
172 checks passed
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Oct 22, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regressions in System.Globalization.Tests.StringSearch Support profile instrumentation for inlinees

3 participants

@AndyAyersMS@EgorBo