Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Inliner: improve arg observations #6337

Merged
merged 1 commit into from
Jul 20, 2016

Conversation

AndyAyersMS
Copy link
Member

Two related changes to better capture cases where args or constant args
are seen at inline call sites.

On the observation side, the inliner's stack modelling of the callee is
crude and will often overestimate the evaluation stack depth. So when
looking at an opcode that takes just one stack operand (eg BRFALSE) the
inliner's check should be whether the stack has at least one element
instead of checking whether the stack has exactly one element. For
compatibility reasons, the check for exactly one element is still used
when the inline is evaluated via the LegacyPolicy.

On the policy side, instead of keeping a bool flag to note that there
were interesting arg cases, count the number of observations.
LegacyPolicy continues to act as before, checking if count is zero or
nonzero instead of whether the flag was false or true. The count is
available for use in other heuristics and is reported in inline data.

Two related changes to better capture cases where args or constant args
are seen at inline call sites.

On the observation side, the inliner's stack modelling of the callee is
crude and will often overestimate the evaluation stack depth. So when
looking at an opcode that takes just one stack operand (eg BRFALSE) the
inliner's check should be whether the stack has at least one element
instead of checking whether the stack has exactly one element. For
compatibility reasons, the check for exactly one element is still used
when the inline is evaluated via the LegacyPolicy.

On the policy side, instead of keeping a bool flag to note that there
were interesting arg cases, count the number of observations.
LegacyPolicy continues to act as before, checking if count is zero or
nonzero instead of whether the flag was false or true. The count is
available for use in other heuristics and is reported in inline data.
@AndyAyersMS
Copy link
Member Author

@kyulee PTAL
cc @dotnet/jit-contrib

@AndyAyersMS
Copy link
Member Author

@mikedn If this goes in before your change we'll want to update the new policy you're adding to override IsLegacyPolicy and return false, so this new logic kicks in...

@kyulee1
Copy link

kyulee1 commented Jul 19, 2016

LGTM

@AndyAyersMS
Copy link
Member Author

ARM failures are known issues; Ubuntu tests passed but the leg failed at the very end in the post processing. So will ignore all these.

@AndyAyersMS
Copy link
Member Author

@dotnet-bot retest Ubuntu x64 Checked Build and Test

@AndyAyersMS
Copy link
Member Author

Retrying Ubuntu to see if the netci.groovy reversion in #6336 really fixed the leg.

@AndyAyersMS AndyAyersMS merged commit 7288deb into dotnet:master Jul 20, 2016
@AndyAyersMS AndyAyersMS deleted the CountObservations branch July 20, 2016 06:31
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ions

Inliner: improve arg observations

Commit migrated from dotnet/coreclr@7288deb
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants