Skip to content

Handle Block and If in BCodeBodyBuilder.genCond(). #15081

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

Merged
merged 2 commits into from
May 2, 2022

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented May 2, 2022

These are my last suggestions for a better codegen by construction. The first one, for Block, is always a win. The second one is sometimes a bit worse, as mentioned in the commit message.

For reference, the whole scala3-compiler.jar jardiff for the second commit can be seen here:
https://gist.github.com/sjrd/9ec0f9f53308fb8c8c747f4bd0e480f7

sjrd added 2 commits May 2, 2022 10:47
For `Block`s, we can push `genCond` inside the final `expr`,
reducing the number of cases where we need to go through
`loadAndTestBoolean`.

This is particularly effective for the shape of `do..while` loops,
as shown in the changed test.
This is more for completeness of `genCond()`. It can now push down
the jumps inside all the control structures for which it makes
sense to do so. It typically applies when the condition of an `if`
or `while` is an `if..else` expression.

It is however not always a win. If the result of the nested `if`s
must be converted to a boolean anyway, doing this will result in
duplication of the conversion to boolean.

A jardiff on the bootstrapped `scala3-compiler.jar` shows 191
additions and 287 deletions, suggesting that, on average, this is
good to have.
@sjrd sjrd requested a review from lrytz May 2, 2022 09:02
@sjrd sjrd merged commit 9585868 into scala:main May 2, 2022
@sjrd sjrd deleted the codegen-better-gencond branch May 2, 2022 12:05
@sjrd
Copy link
Member Author

sjrd commented May 2, 2022

Thanks for the reviews. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants