-
Notifications
You must be signed in to change notification settings - Fork 294
Rework relooper algorithm for fun and profit #1558
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
Open
randomPoison
wants to merge
127
commits into
master
Choose a base branch
from
legare/relooper-experiment
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
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
- Try to create a loop if there's a single entry. - Attempt to create a multiple if there are any handled entries/blocks. - Fallback to a loop only if we can't create a multiple. This brings us more in line with the original paper as well, and already fixes the disjoint loops case. Also make a bunch of not-strictly-necessary changes to make things more organized and easier to work with. These should be split out into a separate PR before the functional refactoring.
Because apparently I have a different compiler on my laptop and it's mad about the missing statements, even though on my computer at home it works lolsob
Also differentiate between breaks and continues when relooping.
... by not generating a break if there's only one entry following a simple shape.
And also make the json output pretty.
Contributor
Author
|
Oh joy, we have a subtle difference in behavior between platforms. Looks like the CFG labels that we generate on macos are slightly different than the labels we generate on Ubuntu, likely because the compiler we're using is different and is generating a slightly different CFG. |
This was referenced Jan 22, 2026
Contributor
Author
|
Okay the tests should be clean and ready for review as well. |
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 PR reworks the relooper algorithm to replace most usages of
current_blockwith labeled blocks/breaks. Addresses #330.The bulk of the changes are in
relooper.rsandstructures.rs:relooper.rsthe core logic is in therelooperfunction, which takes the unstructured CFG and processes it into the "structured CFG".structures.rswe have the logic that takes the structured CFG and turns it into the "strucured AST", which is an intermediate AST that just represents the structured control flow we're translating.structured_cfg_helpwith a newprocess_cfgfunction that is responsible for generating the labeled blocks/breaks.cleanup_labelsthat removes redundant labels and extra labeled blocks. This allows the logic inprocess_cfgto be simpler by conservatively labeling all blocks and breaks.Don't try to review this commit-by-commit, the commit history is NOT clean and shows all of the experimentation and iteration I went through to get to this point. I only have all the commit history intact because I haven't had a need to squash yet, and we should squash this when merging to avoid polluting the commit history.
I'd also recommend not worrying too much about the diff here, and instead just review the new code as if it were all entirely fresh. Many of the parts here have been heavily reworked, and since I'm the only person deeply familiar with what the code was doing before it probably doesn't make sense to worry about how the new code differs from the old version.
NOTE: I still need to do a cleanup pass on the unit tests, since a lot of those were slapped together during experimentation. Snapshot tests are all clean, but I may want to add more as I cleanup the unit tests.