-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
When min major R2R version moves forward, consider some cleanups #98871
Comments
Some helpers can be removed too, #98858 (comment) |
Right. Let's keep this as a holder for cleanups, conditional on revving min version. |
Likewise there are some ARM helpers that are now obsolete. We marked them with a comment in the appropriate source code. |
The delegate method pointers (which are being swapped in #99200 to avoid R2R bump) can be swapped back on the next bump for some minor perf improvements. |
I now have a PR #100310 which represents the cleanups. I've talked with @VSadov, and unfortunately, it looks like the V3 GCInfo isn't happening due to an issue with our codegen in RyuJit that won't get fixed this release. @MichalPetryka I know you've done some work on the math helpers, I can see that we only want to remove the floating point rounding helpers instead of the 4 helpers you initially proposed as making ready for removal. Since #99200 is not yet in, I haven't put together the changes to re-order the fields. |
The issue might be fixable. I am still looking at how it may be dealt with. There are some obvious solutions like placing a NOP at the end of every basic block that ends on a method call, but I'd prefer a solution that is less intrusive (which I did not figure yet). |
GCInfo version v3 is backward-compatible with v2. That is intentional to not require moving forward the R2R min major version.
However, if the min major version is incremented for other reasons, consider the following (not backward-compatible) changes -
Once v2 is no longer in use, both GCInfo and hijacking logic could be simplified.
AreSafePointsInterruptible
could be removed, except when needed to support x86.(i.e. anything for RISC architectures can stop supporting noninterruptible call sites)
DOTNET_InterruptibleCallSites
knob at that timeThese suggestions are not by themselves enough to bump the min major version, but the version moves forward for other reasons, please consider doing these as well.
The text was updated successfully, but these errors were encountered: