-
Notifications
You must be signed in to change notification settings - Fork 830
Cast optimization improvement: also handle local.tee #6507
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
Conversation
Converts the following:
(some.operation
(ref.cast .. (local.tee $ref ..))
(local.get $ref)
)
into:
(some.operation
(local.tee $temp
(ref.cast .. (local.tee $ref ..))
)
(local.get $temp)
)
This removes close to 10% of the casts in a test program (from 38113
casts down to 36119).
I also tried to improve the optimization that moves more refined casts
earlier, but this does not seem very effective (it only eliminated 9
further casts), so I'm not sure it is worth it.
kripken
left a comment
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.
Good idea!
| // blocks (TODO 2, though RedundantSetElimination does that as well). | ||
| // However, we should consider whether improving those other passes | ||
| // might make more sense (as it would help more than casts, if we could | ||
| // make them operate "backwards" and/or past basic 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.
The first TODO is still relevant, isn't it?
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.
Oh, it is indeed only partially done (with connectAdjacentBlocks set to true).
src/passes/OptimizeCasts.cpp
Outdated
| void handleRefinement(Expression* curr) { | ||
| auto* teeFallthrough = Properties::getFallthrough( | ||
| curr, options, *getModule(), Properties::FallthroughBehavior::NoTeeBrIf); | ||
| if (auto* set = teeFallthrough->dynCast<LocalSet>()) { |
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 (auto* set = teeFallthrough->dynCast<LocalSet>()) { | |
| if (auto* tee = teeFallthrough->dynCast<LocalSet>()) { |
Also the isTee check below is not needed, as a fallthrough LocalSet must be a tee (because only a tee flows out a value that can fall through to some place).
src/passes/OptimizeCasts.cpp
Outdated
| return; | ||
| } | ||
| updateBestCast(curr, get->index); | ||
| return; |
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.
| return; |
src/passes/OptimizeCasts.cpp
Outdated
| updateBestCast(curr, set->index); | ||
| } | ||
| } | ||
| auto* fallthrough = Properties::getFallthrough(curr, options, *getModule()); |
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.
| auto* fallthrough = Properties::getFallthrough(curr, options, *getModule()); | |
| auto* fallthrough = Properties::getFallthrough(teeFallthrough, options, *getModule()); |
This saves a little work: we looked a little the first time, and the second time we don't need to re-do that work, and just continue from there.
|
I tested this on a large Java program and it removed 3.2% of casts. Nice work @vouillon ! |
This reverts commit 25315fb4d3231ecd0fdc07c57adec59f3e697dc3. No longer useful with WebAssembly/binaryen#6507
Converts the following:
into:
This removes close to 10% of the casts in a test program (from 38113 casts down to 36119).
I also tried to improve the optimization that moves more refined casts earlier, but this does not seem very effective (it only eliminated 9 further casts), so I'm not sure it is worth it.