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

For x86/x64, call a stack probe helper for all frame sizes equal or over one page #44664

Merged

Conversation

echesakov
Copy link
Contributor

@echesakov echesakov commented Nov 13, 2020

This is to avoid accessing stack below rsp in a prolog. This is how the probing is done in MSVC++ implementation for frames larger than PAGE_SIZE.

@echesakov echesakov added arch-x86 os-linux Linux OS (any supported distro) os-windows arch-x64 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Nov 13, 2020
@echesakov echesakov self-assigned this Nov 13, 2020
…lper for larger frame sizes in codegenxarch.cpp
@echesakov echesakov force-pushed the Jit-Always-Use-Stack-Probe-Helper branch from 4000867 to 15b7556 Compare November 13, 2020 21:42
@echesakov echesakov marked this pull request as ready for review November 17, 2020 03:09
@echesakov
Copy link
Contributor Author

@janvorli @BruceForstall PTAL - I ran dotnet/performance benchmarks on top of these changes.
There is no measurable performance impact - if a benchmark was Slower/Faster during the first run, it was showing the Same results for the second run.

With help from @DrewScoggins we ran the same changes on our performance machines and, although there are some benchmarks that are slower with the change, looking at the historical data I can't reliably whether it's going make it's slower. Please let me know if you want me to share the data with you.

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM. btw, the title, "Always call a stack probe helper", is a too ambition. Instead, I would say "For x86/x64, call a stack probe helper for all frame sizes equal or over one page." The only difference here is we don't inline probe 2 and 3 page frames, which was an optimization someone added long, long ago (before my time) that has persisted.

@BruceForstall
Copy link
Member

btw, I think we need a detailed section the in the CLR ABI document, https://github.com/dotnet/runtime/blob/master/docs/design/coreclr/botr/clr-abi.md, describing all the rules and restrictions and design constraints around stack probing on all the different platforms. (Or maybe it deserves its own document.)

@echesakov
Copy link
Contributor Author

btw, I think we need a detailed section the in the CLR ABI document, https://github.com/dotnet/runtime/blob/master/docs/design/coreclr/botr/clr-abi.md, describing all the rules and restrictions and design constraints around stack probing on all the different platforms. (Or maybe it deserves its own document.)

Yes, I do remember that I need to update the document (or create a new one as suggested). I am still in the process of deciding what should be done on Arm64. After that, I can go and describe all the implementations.

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, as far as I can tell given my limited knowledge of JIT internals.

@echesakov echesakov changed the title Always call a stack probe helper For x86/x64, call a stack probe helper for all frame sizes equal or over one page Nov 19, 2020
@echesakov echesakov merged commit 999785e into dotnet:master Nov 19, 2020
@echesakov echesakov deleted the Jit-Always-Use-Stack-Probe-Helper branch November 19, 2020 01:39
@ghost ghost locked as resolved and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-linux Linux OS (any supported distro) os-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants