-
Notifications
You must be signed in to change notification settings - Fork 42
Add function return propagation pass (try2) #2228
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
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #2228 will improve performances by 12.42%Comparing Summary
Benchmarks breakdown
|
d13a715 to
7d54fa5
Compare
|
I just rebased this on #2229 which makes the size benchmark only use size (and no lookahead), which means this pass removes those. That ends up being a 10-12% improvement over current Note that I changed it to be different so that the lookahead and size benchmarks test different things. I noticed both removed the same parameters, and every other time I've seen them they've moved in unison, so I think it's fair to separate their concerns a bit. It's just a bit nice to see bigger number too :) EDIT: And honestly I don't think the 6% improvement in lookahead is shabby either! end-to-end improvement in Zeek with the SSL analyzer: If I remember right the Spicy parts were about 1/2 the total runtime so this is about the same as the lookahead benchmark improvements |
After 5197e79, we replace some declarations with the RHS of the assignment so that we don't remove method calls. The same problem applies to assignments generally, so add cases for those. This also makes some code stick around (particularly when paired with constant propagation), so we need more aggressive removal of expressions whose result is unused and who don't have side effects.
This is necessary to be separate from the statement/declaration tracking since there are more cases of just "floating" expression statements, like `(a, b, c, d)` after some constant propagation and dead code elimination.
Since the two benchmarks used the same `Inner`, they would get the same interprocedural optimizations. This doesn't give the full picture about how something changes, so separate them so that we can get a better picture. This definitely makes the benchmarks more "micro," but I believe that we see more people using *either* lookahead *or* size, not both on the same unit.
This will propagate the `result` variable in cases like: ``` result = my_method_call(); return result; ``` into: ``` return my_method_call(); ```
f30f2e6 to
9b53517
Compare
Supersedes #2173 (see #2173 (comment))
Closes #2031
Draft to get benchmark run, then I'll clean it up and fix bugs/tests if it's worth it. and add the explanation for all of the weirdness in here
EDIT: Seems like the perf improvement will be worth it, I'll clean this up.