-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Refactor subrange assertion generation and propagation #55186
Merged
AndyAyersMS
merged 16 commits into
dotnet:main
from
SingleAccretion:Improve-Subrange-Assertion-Generation
Sep 20, 2021
Merged
Refactor subrange assertion generation and propagation #55186
AndyAyersMS
merged 16 commits into
dotnet:main
from
SingleAccretion:Improve-Subrange-Assertion-Generation
Sep 20, 2021
Commits on Sep 17, 2021
-
Move it into its own function. No diffs.
Configuration menu - View commit details
-
Copy full SHA for 2ddaefa - Browse repository at this point
Copy the full SHA 2ddaefaView commit details -
Stop generating assertions for TYP_U/INT ranges
They are not useful: they do not tell us anything about the value. They can be harmful: code operating on the assertions can wrongly remove a checked cast because of an assertion for a non-checked one.
Configuration menu - View commit details
-
Copy full SHA for d6b7073 - Browse repository at this point
Copy the full SHA d6b7073View commit details -
Configuration menu - View commit details
-
Copy full SHA for 0f24634 - Browse repository at this point
Copy the full SHA 0f24634View commit details -
This change completely refactors the way subrange assertions are generated and consumed. It rationalizes and generalizes the model for subrange assertions which are generated and propagated for casts, makes AssertionDsc::Range into an actual abstraction, eliminates bugs related to wrong assertions generated for TYP_BOOL (without CQ loss), enables propagation for checked casts to non-small types both on 64 and 32 bit, all the while making the AssertionDsc structure smaller. There are quite a few moving parts, but at the heart of this change are following items: 1. Make ranges symbolic, i. e. made up of symbolic upper and lower bounds, represented by the new SymbolicIntegerValue enum class. This is necessary to not regress memory consumption and throughput of operations on 32 bit while handling long subranges. 2. Split the concept of "input" and "output" for a cast node. To properly handle checked cast and make the code "obviously correct", it is necessary to split the two values, one the cast node produces and one it consumes, apart. The old code only handles the "easy" case of the "input" and "output" ranges matching (such is the case for small types), the new code must be, and is, robust in reasoning about, e. g. CAST_OVF(uint <- ulong) vs CAST_OVF(int <- long). The ranges that represent these values are consistently in the "signed" format, making handling them easy. 3. Tweak the propagation code to not remove representation-changing casts, make it explicit and obvious in what cases (and why) it retypes the local under the cast. As a side-effect of the above changes, TYP_BOOL is now treated as TYP_UBYTE in propagation, as it should be. Actually, the old code didn't remove redundant casts to TYP_BOOL, the new code does, which is also where the bulk of the positive diffs come from. The old code also generated "useless" ranges like [INT_MIN..INT_MAX], the new code only generates ranges that could be used in subsequent propagation. Also, some miscellaneous cleanup, such as adding function headers, was performed.
Configuration menu - View commit details
-
Copy full SHA for fd3db65 - Browse repository at this point
Copy the full SHA fd3db65View commit details -
Use correct ranges for global subrange assertions
In global assertion propagation, tentative assertions are created for casts, with the expectation that proving them via implication will allow for removing the cast. The old code did this through the common path with local assertions, however, this cannot be done the same way anymore because the two types of assertions are fundamnetally different. In local assertion propagation, subranges are derived from ASGs, where the cast is on the RHS and the local - LHS, so the "output" range for the cast must be used - local is assigned value the cast **produces**. In contrast, global subrange assertions say that the value the cast **consumes** is within the correct bounds for the cast to become a no-op or non-overflowing. Because of this fundamnetal distinction, I chose to make global assertions for casts in a new method instead of plumbing it through "optCreateAssertion". Note that it would be possible to do it via "optCreateAssertion" as well - global propagation always passes OAK_SUBRANGE to it, while local - OAK_EQUAL, but I decided against that option for the reason that it seems unintuitive to me that it would work the way it would. These tentative assertions are a very special case, so I think it is better to treat them that way. This leads to some minor code duplication, but I think that's ok. No diffs for this commit, though that's expected as "proving" the generated assertion is quite rare.
Configuration menu - View commit details
-
Copy full SHA for 9d47aac - Browse repository at this point
Copy the full SHA 9d47aacView commit details -
Refactor AssertionDsc::Range into its own class
Make IntegralRange the owner of the logic for various methods relating to it and decouple it from AssertionDsc. IntegralRange will be useful in the future for other code that deals with cast removal, e. g. in fgMorphCast. Also some miscellaneous formatting fixups.
Configuration menu - View commit details
-
Copy full SHA for 5a4434c - Browse repository at this point
Copy the full SHA 5a4434cView commit details -
Change the implementation of SymbolicToRealValue
Using a static array results in smaller and more efficient code on all compilers.
Configuration menu - View commit details
-
Copy full SHA for 452a4a2 - Browse repository at this point
Copy the full SHA 452a4a2View commit details -
FP & GC castees in IntegralRange::ForCastOutput
This change is for completeness, since "fgMorphCast" decomposes casts from FP types to small types today and as such assertions are generated for them anyway. No diffs for this commit as expected.
Configuration menu - View commit details
-
Copy full SHA for 9b35f21 - Browse repository at this point
Copy the full SHA 9b35f21View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9412c7e - Browse repository at this point
Copy the full SHA 9412c7eView commit details -
Fix the input ranges for non-overflowing casts
They did not handle "long" cases and as such were too conservative.
Configuration menu - View commit details
-
Copy full SHA for 4b658dc - Browse repository at this point
Copy the full SHA 4b658dcView commit details -
Configuration menu - View commit details
-
Copy full SHA for 723da4d - Browse repository at this point
Copy the full SHA 723da4dView commit details -
Configuration menu - View commit details
-
Copy full SHA for b43f07b - Browse repository at this point
Copy the full SHA b43f07bView commit details -
Configuration menu - View commit details
-
Copy full SHA for a697d02 - Browse repository at this point
Copy the full SHA a697d02View commit details -
Configuration menu - View commit details
-
Copy full SHA for 763a95f - Browse repository at this point
Copy the full SHA 763a95fView commit details -
Configuration menu - View commit details
-
Copy full SHA for c513b9a - Browse repository at this point
Copy the full SHA c513b9aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 995f0f8 - Browse repository at this point
Copy the full SHA 995f0f8View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.