-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Fold CMP and CBR instructions #2615
Conversation
// TODO: Experiment with putting combine-constants and simplify-cfg | ||
// in a loop, but per function. |
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.
Or really, we need to create a pass manager in sway-ir
. It could initially just call all the passes consecutively and repeatedly until nothing changes (if that isn't too expensive) but it doesn't belong here in lib.rs
any more.
Apart from the nits, all good! I'm glad it's already making a difference. |
This change also replaces expressions such as the one below
with
and it looks like, when there are no further removal of dead blocks in that program, the code size actually increases a little bit. There are no other changes apart from these in the IR that I inspected, for the tests which have a slight code size increase. Full results:
|
So the files that are larger are just because |
I didn't check them all, but the ones I manually inspected, yes. And yes, we should do this optimization at some point. |
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.
Awesome metrics!
This enables more opportunities for
simplify-cfg
, which in-turn enables morecmp
andcbr
folding (as some intermediate PHIs get removed bysimplify-cfg
).I haven't yet done a full benchmarking, but for the
basic_storage
test, which uses a lot of code fromstd::storage
, which in-turn has code of the formif !is_reference_type...
(which is statically evaluatable), I saw the bytecode size reduce from 20k to about 11.7k.edit: numbers on the full testsuite is added as comment down below.