avm2: Optimizer improvements #15105
Merged
avm2: Optimizer improvements #15105
Conversation
This file contains 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
Lord-McSweeney
added
A-avm2
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
a74dcc4 to
ef23851
Compare
adrian17
reviewed
Feb 3, 2024
adrian17
reviewed
Feb 3, 2024
adrian17
reviewed
Feb 4, 2024
adrian17
reviewed
Feb 4, 2024
core/src/avm2/verify.rs
Outdated
| stack.pop(); | ||
| stack.push_int(); | ||
| } | ||
| _ => { |
There was a problem hiding this comment.
as mentioned on discord, I think we should just have all of these checked in one go - otherwise this can't be used for verification, and can randomly inhibit optimizations.
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
bbcc1f4 to
464274a
Compare
Aaron1011
reviewed
Feb 4, 2024
adrian17
reviewed
Feb 7, 2024
| @@ -2243,23 +2249,23 @@ impl<'a, 'gc> Activation<'a, 'gc> { | |||
| let value2 = self.pop_stack().coerce_to_i32(self)?; | |||
| let value1 = self.pop_stack().coerce_to_i32(self)?; | |||
|
|
|||
| self.push_stack(value1.wrapping_mul(value2)); | |||
| self.push_raw(value1.wrapping_mul(value2)); | |||
There was a problem hiding this comment.
I think this could also be applied to some more ops: add, subtract, swap, astype, astypelate? For the last three, since other ops guarantee that stack never has PrimitiveObject, then they can also never encounter a PrimitiveObject.
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
deaf921 to
06cbbe4
Compare
torokati44
force-pushed
the
avm2-abstract-optimizer
branch
from
7787587 to
5af94c9
Compare
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
5af94c9 to
6bdca88
Compare
added 12 commits
February 24, 2024 12:43
…e than one `ClassObject` had been constructed for the class
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
6bdca88 to
d2971e8
Compare
adrian17
approved these changes
Feb 24, 2024
evilpie
reviewed
Feb 25, 2024
evilpie
reviewed
Feb 25, 2024
| } | ||
|
|
||
| fn popn(&mut self, count: u32) { | ||
| for _ in 0..count { |
There was a problem hiding this comment.
I wonder if something like self.0.truncate(self.0.len() - count) would be more efficient?
There was a problem hiding this comment.
I don't think we need to be too worried about performance here
evilpie
reviewed
Feb 25, 2024
Lord-McSweeney
force-pushed
the
avm2-abstract-optimizer
branch
from
890c690 to
dce4238
Compare
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.
Based off @adrian17's
abstract-optimizerbranch.Testing on the Box2D demo SWF shows that:
55% of
GetPropertycalls are converted toGetSlotorCallMethodcalls36% of
SetPropertycalls are converted toSetSlotcalls74% of
InitPropertycalls are converted toSetSlotcalls25% of
CallPropertycalls are converted toCallMethodcallsTesting on Sniper Team shows that
PropertyMap::get_for_multinameis pushed down significantly. HoweverActivation::run_actionsis still too high for it to make a noticeable difference.Fixes #15227
Fixes #15273
Fixes #15309
TODO:
Constant pooling fixesFor laterCallPropertytoCallMethodAlso support setters forFor laterSetPropertyandInitPropertyRemoveFor laterNops (requires recalculating branch and jump targets)Use it for determining type returned byFor laterfindproperty/findpropstrictImplement more opsFor later