Repository navigation
[AbstractTypeRefining] Skip unreachable casts #9358
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 all commits
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 | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -371,6 +371,13 @@ struct AbstractTypeRefining : public Pass { | |||||||||||||||||||||||||||||||||||||||||
| // This might cause the casts to incorrectly start succeeding. To avoid | ||||||||||||||||||||||||||||||||||||||||||
| // that, optimize them out first. | ||||||||||||||||||||||||||||||||||||||||||
| void visitRefCast(RefCast* curr) { | ||||||||||||||||||||||||||||||||||||||||||
| if (curr->ref->type == Type::unreachable || | ||||||||||||||||||||||||||||||||||||||||||
| (curr->desc && curr->desc->type == Type::unreachable)) { | ||||||||||||||||||||||||||||||||||||||||||
| // A child may have become unreachable earlier in Preoptimizer, before | ||||||||||||||||||||||||||||||||||||||||||
| // ReFinalize has run. Propagate unreachability and skip optimizing. | ||||||||||||||||||||||||||||||||||||||||||
| curr->type = Type::unreachable; | ||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| auto optimized = getOptimized(curr->type); | ||||||||||||||||||||||||||||||||||||||||||
| if (!optimized) { | ||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -402,6 +409,11 @@ struct AbstractTypeRefining : public Pass { | |||||||||||||||||||||||||||||||||||||||||
| // If we optimize away a descriptor type, then we must fix up any | ||||||||||||||||||||||||||||||||||||||||||
| // ref.get_desc of it, as ReFinalize would fix us up to return it. | ||||||||||||||||||||||||||||||||||||||||||
| void visitRefGetDesc(RefGetDesc* curr) { | ||||||||||||||||||||||||||||||||||||||||||
| if (curr->ref->type == Type::unreachable) { | ||||||||||||||||||||||||||||||||||||||||||
| // As in visitRefCast, propagate unreachability and skip optimizing. | ||||||||||||||||||||||||||||||||||||||||||
| curr->type = Type::unreachable; | ||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| auto optimized = getOptimized(curr->type); | ||||||||||||||||||||||||||||||||||||||||||
| if (!optimized) { | ||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -430,6 +442,12 @@ struct AbstractTypeRefining : public Pass { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| void visitBrOn(BrOn* curr) { | ||||||||||||||||||||||||||||||||||||||||||
| if (curr->ref->type == Type::unreachable || | ||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [HIGH | Confidence: 5/5 | BUG] The unreachable check in
Suggested change
If the If this To ensure consistent type propagation matching
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't quite get the comment here, but it at least makes sense to me to move the check up like it suggests? Although maybe refinalize would do this anyway
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, seems reasonable. |
||||||||||||||||||||||||||||||||||||||||||
| (curr->desc && curr->desc->type == Type::unreachable)) { | ||||||||||||||||||||||||||||||||||||||||||
| // As in visitRefCast, propagate unreachability and skip optimizing. | ||||||||||||||||||||||||||||||||||||||||||
| curr->type = Type::unreachable; | ||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if (curr->op == BrOnNull || curr->op == BrOnNonNull) { | ||||||||||||||||||||||||||||||||||||||||||
| 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.
Why is this needed? Did we modify the code at this point but not call refinalize yet? If so it seems worth a comment as in general this should not be necessary.
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.
Added comments.