-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remove extra blocks for return statements #645
Conversation
58d823e
to
59f285c
Compare
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.
Seems fine to me!
.fuze_up() | ||
.return_early() | ||
.return_early() | ||
.collapse_empty_blocks() |
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.
collapse_empty also needs to be run to fixpoint? We cant do it in a single pass?
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.
If there are two empty blocks, you need to skip both of them. This pass only does it once per time it runs
} | ||
|
||
/// Find cases where a block jumps directly to another block A -> B where B | ||
/// Converts footers into bril expressions to simplify this pass | ||
fn remove_footers(mut self) -> SimpleCfgFunction { |
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.
what is a footer?
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.
Footers are used by the RVSDG translation to track new stuff added to the block
We don't really care about footers vs instructions so we convert them here
This PR adds a small optimization that moves return statements up into parent blocks