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.

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

JIT: profile checking through inlining #101834

Merged
merged 3 commits into from
May 4, 2024

Conversation

AndyAyersMS
Copy link
Member

Advance profile consistency check through inlining. Turns out there are five reasons why inlining may make profile data inconstent. Account for these and add metrics.

Also add separate metrics for consistency before and after inlining, since pre-inline phases are run on inlinees and so don't give us good insight into overall consistency rates. And add some metrics for inlining itself.

Contributes to #93020.

Advance profile consistency check through inlining. Turns out there are
five reasons why inlining may make profile data inconstent. Account
for these and add metrics.

Also add separate metrics for consistency before and after inlining,
since pre-inline phases are run on inlinees and so don't give us good
insight into overall consistency rates. And add some metrics for
inlining itself.

Contributes to dotnet#93020.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 3, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented May 3, 2024

@amanasifkhalid PTAL
cc @dotnet/jit-contrib

Some stats from ASP.NET (only for methods that have inlines). Roughly 80% consistent going in to inlining, 60% consistent coming out.

Count of Method full name 51654
Sum of ProfileConsistentBeforeInline 41952
Sum of ProfileConsistentAfterInline 33414
Sum of ProfileInconsistentInlinerBranchFold 859
Sum of ProfileInconsistentInlineeScale 0
Sum of ProfileInconsistentInlinee 6158
Sum of ProfileInconsistentNoReturnInlinee 23
Sum of ProfileInconsistentMayThrowInlinee 1498
Sum of InlineAttempt 779761
Sum of InlineCount 775969

Fair number of diffs in some collections. There are two spots where we alter profiles from base that are the likely causes -- I will spot check to make sure nothing else is going on.

  1. when the inlinee entry is a loop head, we were under-scaling the inlinee
  2. we now make profile adjustments when inliner folds a branch

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented May 3, 2024

osx-arm64 build issue is a crash in clang:

  clang: error: unable to execute command: Abort trap: 6
  clang: error: clang frontend command failed due to signal (use -v to see invocation)
  Apple clang version 14.0.0 (clang-1400.0.29.202)
  Target: arm64-apple-darwin21.6.0
  Thread model: posix
  InstalledDir: /Applications/Xcode_14.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
  [ 76%] Building CXX object vm/wks/CMakeFiles/cee_wks_core.dir/__/stackingallocator.cpp.o
  [ 76%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/jithashtable.cpp.o
  [ 76%] Building CXX object jit/CMakeFiles/clrjit.dir/emitarm64.cpp.o
  [ 76%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_arm64.dir/jitmetadata.cpp.o
  clang: note: diagnostic msg: 
  ********************
  
  PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
  Preprocessed source(s) and associated run script(s) are located at:
  clang: note: diagnostic msg: /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/codegenarmarch-0adb77.cpp
  clang: note: diagnostic msg: /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/codegenarmarch-0adb77.sh
  clang: note: diagnostic msg: Crash backtrace is located in
  clang: note: diagnostic msg: /Users/runner/Library/Logs/DiagnosticReports/clang_<YYYY-MM-DD-HHMMSS>_<hostname>.crash
  clang: note: diagnostic msg: (choose the .crash file that corresponds to your crash)
  clang: note: diagnostic msg: 
  
  ********************
  make[2]: *** [jit/CMakeFiles/clrjit_universal_arm64_arm64.dir/codegenarmarch.cpp.o] Error 254
  make[1]: *** [jit/CMakeFiles/clrjit_universal_arm64_arm64.dir/all] Error 2
  make[1]: *** Waiting for unfinished jobs...

Am going to rerun and hope this is a very intermittent failure, otherwise will try and repro this locally and file a bug.

@AndyAyersMS
Copy link
Member Author

Fyi @EgorBo per above we bail out of inlining late less than 1% of the time

Copy link
Member

@amanasifkhalid amanasifkhalid left a comment

Choose a reason for hiding this comment

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

LGTM, assuming CI looks good (hopefully that clang failure doesn't become a blocker). Thanks!

src/coreclr/jit/fgbasic.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/fginline.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/fgprofile.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/fgprofile.cpp Outdated Show resolved Hide resolved
AndyAyersMS and others added 2 commits May 3, 2024 10:42
Co-authored-by: Aman Khalid <amankhalid@microsoft.com>
@AndyAyersMS
Copy link
Member Author

SPMI failures are lagging collections after a GUID update. Am going to ignore.

@AndyAyersMS AndyAyersMS merged commit f82fb70 into dotnet:main May 4, 2024
100 of 107 checks passed
michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
Advance profile consistency check through inlining. Turns out there are
five reasons why inlining may make profile data inconsistent. Account
for these and add metrics.

Also add separate metrics for consistency before and after inlining,
since pre-inline phases are run on inlinees and so don't give us good
insight into overall consistency rates. And add some metrics for
inlining itself.

Contributes to dotnet#93020.

Co-authored-by: Aman Khalid <amankhalid@microsoft.com>
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
Advance profile consistency check through inlining. Turns out there are
five reasons why inlining may make profile data inconsistent. Account
for these and add metrics.

Also add separate metrics for consistency before and after inlining,
since pre-inline phases are run on inlinees and so don't give us good
insight into overall consistency rates. And add some metrics for
inlining itself.

Contributes to dotnet#93020.

Co-authored-by: Aman Khalid <amankhalid@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR 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.

2 participants