Skip to content

Inliner: Extend IL limit for profiled call-sites, allow inlining for switches. #55478

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

Merged
EgorBo merged 21 commits into from
Jul 14, 2021
Merged

Inliner: Extend IL limit for profiled call-sites, allow inlining for switches. #55478

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d711b0f
Improve Inliner, vol.3
EgorBo Jul 10, 2021
42dcb3d
Merge branch 'main' of https://github.com/dotnet/runtime into jit-inl…
EgorBo Jul 10, 2021
154bd99
Improvements
EgorBo Jul 10, 2021
6dd92ca
Tuning.
EgorBo Jul 11, 2021
4cb8387
Fix asserts
EgorBo Jul 11, 2021
8117d8f
Clean up
EgorBo Jul 11, 2021
5a3effa
Fix assert
EgorBo Jul 12, 2021
36f91ad
Merge branch 'main' of https://github.com/dotnet/runtime into jit-inl…
EgorBo Jul 12, 2021
40588ff
Tuning.
EgorBo Jul 12, 2021
e19f79f
add missing break;
EgorBo Jul 12, 2021
2240f34
Tuning for size.
EgorBo Jul 12, 2021
d8f4c45
Tuning.
EgorBo Jul 12, 2021
f6e7c13
Formatting
EgorBo Jul 12, 2021
970a2cd
Tuning.
EgorBo Jul 12, 2021
a7ecdf6
Revert some changes
EgorBo Jul 12, 2021
db0f3bc
Fix cases where "arg op cns" left "cns" on top of the pushedstack -- …
EgorBo Jul 12, 2021
d2bbe6d
Merge branch 'jit-inliner-3' of https://github.com/EgorBo/runtime-1 i…
EgorBo Jul 12, 2021
a382443
Merge branch 'main' of https://github.com/dotnet/runtime into jit-inl…
EgorBo Jul 12, 2021
4d0bf98
Fix an assert when we import a switch (in jitdump mode)
EgorBo Jul 12, 2021
e948d10
Disable over-ILsize for generic static pgo
EgorBo Jul 13, 2021
ab9f542
Update fgbasic.cpp
EgorBo Jul 13, 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
Prev Previous commit
Next Next commit
Formatting
  • Loading branch information
@EgorBo
EgorBo committed Jul 12, 2021
commit f6e7c1397e533c15f09177e1760919a8f131c474
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgprofile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ bool Compiler::fgHaveSufficientProfileData()

if ((fgFirstBB != nullptr) && (fgPgoSource == ICorJitInfo::PgoSource::Static))
{
const BasicBlock::weight_t sufficientSamples = 1000;
const BasicBlock::weight_t sufficientSamples = 5000;
Copy link
Member

@AndyAyersMS AndyAyersMS Jul 12, 2021

Choose a reason for hiding this comment

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

This used to be 1000 -- I assume you're increasing this to keep prejit image size small?

If so, you should add a comment describing how this value influences prejit size.
If not, you might comment on what the impact of changing this would be.

Copy link
Member

@jkotas jkotas Jul 12, 2021

Choose a reason for hiding this comment

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

1000 samples feels like a plenty of evidence that something is hot.

Copy link
Member Author

@EgorBo EgorBo Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Reverted to 1000. Yes, this changes was supposed to decrease the prejitted size, I was using this histogram:
image
(weights in SPC, the right column starts at 50000).

However, I don't need to save some space with it anymore as I've found an unrelated issue that bloated size for no reason (binary expressions like "arg op cns" used to leave "cns" on top of the pushed stack so there were lots of false-positive foldable-branches/switches).

return fgFirstBB->bbWeight > sufficientSamples;
}
return true;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19239,7 +19239,8 @@ void Compiler::impCheckCanInline(GenTreeCall* call,
}

// Profile data allows us to avoid early "too many IL bytes" outs.
pParam->result->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE, pParam->pThis->fgHaveSufficientProfileData());
pParam->result->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE,
pParam->pThis->fgHaveSufficientProfileData());

bool forceInline;
forceInline = !!(pParam->methAttr & CORINFO_FLG_FORCEINLINE);
Expand Down