-
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
JIT: fix case where implied subrange assertions can get lost in morph #112020
Conversation
If a method used short bit vectors during morph, modifications to the assertion set were being made to a copy, not to the original. Also, restrict implied boolean subrange assertions to integer vars.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
@EgorBo PTAL Follow up from some things I noticed here #111980 (comment) Some diffs in small methods. |
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.
Our guidelines dislike non-constant reference arguments https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/clr-jit-coding-conventions.md#1535-reference-arguments but looks like we do it in a quite a few places
Once this goes in I will experiment with expanding the implied assertion set. |
Also not a big fan of using typedefs to hide implementation details, |
Failures all look like infra issues so far. |
* main: JIT: fix case where implied subrange assertions can get lost in morph (dotnet#112020) Propose new async API (dotnet#110420) Move the diagnostic print for stack overflow test failure (dotnet#112001) JIT: Fold more nullchecks (dotnet#111985) Baseline more pri1 tests (dotnet#111068)
If a method used short bit vectors during morph, modifications to the assertion set were being made to a copy, not to the original.
Also, restrict implied boolean subrange assertions to integer vars.