-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Add budget check to optReachable #118787
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
Add budget check to optReachable #118787
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
PTAL @jakobbotsch @dotnet/jit-contrib A few diffs, only from the fast-path check |
There was a problem hiding this comment.
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 addresses a performance issue in the JIT compiler's if-conversion optimization by adding budget checks to prevent excessive computation during reachability analysis. The change reduces crossgen compilation time from 60 seconds to 6 seconds for the reported test case.
Key changes:
- Added a budget parameter to
optReachable
to limit the number of blocks examined during reachability analysis - Modified if-conversion logic to use the budget and skip expensive checks when appropriate
- Optimized debug checking routines to avoid expensive operations on large basic block counts
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/coreclr/jit/compiler.h | Added optional budget parameter to optReachable method signature |
src/coreclr/jit/redundantbranchopts.cpp | Implemented budget checking logic in optReachable with two code paths for budgeted vs non-budgeted operation |
src/coreclr/jit/ifconversion.cpp | Added reachability budget field and logic to use budgeted reachability checks only when backward jumps are present |
src/coreclr/jit/fgdiagnostic.cpp | Optimized debug checks to skip expensive operations when basic block count exceeds 10,000 |
@dotnet/samsung Could you please take a look? These changes may be related to riscv64. |
Presumably not related. |
Co-authored-by: Copilot <[email protected]>
Do you know whether this also reduces peak memory usage during compilation? I can't tell if the allocation of all the huge bitvecs is suppressed by the budget system or not. Improved compile time is good but my understanding is that the test failure in #106675 is caused by the oom killer, so being faster won't necessarily fix it even if it is fantastic for it to be faster. |
Hm.. no, that is something else (I'm seeing 2GB peak memory usage while compiling the repro), this PR only fixes the compilation duration issue, the phase that I'm touching here is not responsible for the working set size. |
ping @jakobbotsch |
Fixes #106675
Reduces crossgen compilation time from 60s to 6s for that test.