Refactor ConstantInterval #8179
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and use it in Monotonic.cpp
Also explicit ignore lossless_cast bugs that will be fixed in #8155
Also, plumb the cache through the recursive calls
Member
Author
|
@rootjalex I think you would be ideal to review this (Not urgently - once you're out from under your current crunch) |
Member
|
Will take a look next week! |
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 19, 2024
rootjalex
reviewed
Apr 21, 2024
rootjalex
reviewed
Apr 21, 2024
rootjalex
reviewed
Apr 21, 2024
rootjalex
reviewed
Apr 21, 2024
Member
Author
|
I believe I have addressed review comments, PTAAL (mostly for operator<<) |
rootjalex
reviewed
Apr 22, 2024
rootjalex
reviewed
Apr 22, 2024
Member
Author
|
Well now there's a fuzz test failure, so something's probably not right with the new operator<<. Will investigate. |
Member
Author
|
The fuzzer failure was just lossless_negate being wrong, which is to be fixed in #8155 |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is the bulk of #8155, pulled out for separate review.
This PR pulls the ConstantInterval class out of Interval.h, and the arithmetic helpers for it out of Monotonic.cpp. It then refactors Monotonic.cpp to use it, including using a new bounds inference routine to use instead of find_constant_bounds. It adds fuzz tests for both the new bounds inference routine and the interval arithmetic itself.
The fuzz test shows a lot of errors due to the lossless_cast casting bugs revealed in #8155, so those are simply reported for now, though I did fix the most egregious bug in lossless_cast to reduce the noise. The fuzz test also found another error to fix in FindIntrinsics.cpp
After this, the next step is to refactor the simplifier to use this ConstantInterval instead of rolling its own thing, and then actually fixing the lossless_cast bug, which has trickle-on effects in instruction selection.