-
Notifications
You must be signed in to change notification settings - Fork 836
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -92,22 +92,6 @@ | |||||
| // Note that right now, we only consider RefAs with op RefAsNonNull as a cast. | ||||||
| // RefAs with ExternInternalize and ExternExternalize are not considered casts | ||||||
| // when obtaining fallthroughs, and so are ignored. | ||||||
| // | ||||||
| // TODO: 1. Look past individual basic blocks? This may be worth considering | ||||||
| // given the pattern of a cast appearing in an if condition that is | ||||||
| // then used in an if arm, for example, where simple dominance shows | ||||||
| // the cast can be reused. | ||||||
| // TODO: 2. Look at LocalSet as well and not just Get. That would add some | ||||||
| // overlap with the other passes mentioned above (SimplifyLocals and | ||||||
| // RedundantSetElimination also track sets and can switch a get to use | ||||||
| // a better set's index when that refines the type). But once we do the | ||||||
| // first two TODOs above then we'd be adding some novel things here, | ||||||
| // as we could optimize "backwards" as well (TODO 1) and past basic | ||||||
| // 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). | ||||||
| // | ||||||
|
|
||||||
| #include "ir/effects.h" | ||||||
| #include "ir/linear-execution.h" | ||||||
|
|
@@ -453,20 +437,32 @@ struct BestCastFinder : public LinearExecutionWalker<BestCastFinder> { | |||||
| void visitRefCast(RefCast* curr) { handleRefinement(curr); } | ||||||
|
|
||||||
| void handleRefinement(Expression* curr) { | ||||||
| auto* teeFallthrough = Properties::getFallthrough( | ||||||
| curr, options, *getModule(), Properties::FallthroughBehavior::NoTeeBrIf); | ||||||
| if (auto* set = teeFallthrough->dynCast<LocalSet>()) { | ||||||
|
||||||
| 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).
Outdated
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.
Outdated
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; |
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
connectAdjacentBlocksset to true).