Skip to content

Commit a6e7fc1

Browse files
committed
Don't generate redundant unreachables in Flatten
This is to remove a code pattern that generates an additional `unreachable` after an unreachable expression. This happens when the unreachable expression is a direct child of a block/loop/try. Block/loop/try preserves its childrens' preludes within it, which means childrens' preludes are not gonna escape the structure. So this kind of code pattern is often generated by Flatten: ```wast (block (some unreachable expression) (unreachable) ;; unnecessary ) ``` This PR removes the unnecessary `unreachable`s generated. On the other hand, `if` does not satisfy that property because `if`'s condition's prelude can escape `if`. This is not for optimization (Flatten is not for optimization per se; it's more of an enabler of other optimization passes, so we don't need to optimize the code generated by Flatten because we have other passes after this); This is done in order to make for WebAssembly#2567 to work. (We are planning to disable Flatten for exceptions now, but that's because of `br_on_exn`; try handling part (WebAssembly#2567) can land now.)
1 parent 263d2d5 commit a6e7fc1

15 files changed

+413
-579
lines changed

scripts/fuzz_opt.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040

4141
INPUT_SIZE_LIMIT = 150 * 1024
4242

43-
LOG_LIMIT = 125
43+
LOG_LIMIT = 500
4444

4545

4646
# utilities

src/ir/flat.h

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,41 @@ inline bool isControlFlowStructure(Expression* curr) {
6969
curr->is<Try>();
7070
}
7171

72+
// Returns true if the given control flow structure preserves all its childrens'
73+
// preludes within it. For example, blocks satisfy this property, because when
74+
// the original block is in the form of
75+
// (block
76+
// (some expression)
77+
// ...
78+
// )
79+
// And we replaced (some expression) with a local.get whose prelude is (some
80+
// expression), the final block will be in the form of
81+
// (block
82+
// (some expression)
83+
// (local.get ...)
84+
// ...
85+
// )
86+
// So the block's children's preludes do not escape the boundary of the block.
87+
// 'if' does not satisfy this property, because the prelude of its condition
88+
// ends up preceding (= escaping) the if. For example, if the original 'if' is
89+
// in the form of
90+
// (if
91+
// (some expression)
92+
// ...
93+
// )
94+
// And (some expression) is replaced with a local.get whose prelude is (some
95+
// expression), the final 'if' will be something like
96+
// (some expression)
97+
// (if
98+
// (local.get ...)
99+
// ...
100+
// )
101+
// So 'if''s condition's preludes escapes the boundary of 'if'. Refer to Flatten
102+
// pass for detailed algorithms.
103+
inline bool containsChildrensPreludes(Expression* curr) {
104+
return curr->is<Block>() || curr->is<Loop>() || curr->is<Try>();
105+
}
106+
72107
inline void verifyFlatness(Function* func) {
73108
struct VerifyFlatness
74109
: public PostWalker<VerifyFlatness,

src/passes/Flatten.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,12 @@ struct Flatten
230230

231231
if (br->condition) {
232232
// the value must also flow out
233-
ourPreludes.push_back(br);
233+
Expression* parent = getParent();
234234
if (br->type.isConcrete()) {
235+
ourPreludes.push_back(br);
235236
replaceCurrent(builder.makeLocalGet(temp, type));
236-
} else {
237+
} else if (parent && !Flat::containsChildrensPreludes(parent)) {
238+
ourPreludes.push_back(br);
237239
assert(br->type == Type::unreachable);
238240
replaceCurrent(builder.makeUnreachable());
239241
}
@@ -277,7 +279,9 @@ struct Flatten
277279
curr = getCurrent(); // we may have replaced it
278280
// we have changed children
279281
ReFinalizeNode().visit(curr);
280-
if (curr->type == Type::unreachable) {
282+
Expression* parent = getParent();
283+
if (curr->type == Type::unreachable && !curr->is<Unreachable>() && parent &&
284+
!Flat::containsChildrensPreludes(parent)) {
281285
ourPreludes.push_back(curr);
282286
replaceCurrent(builder.makeUnreachable());
283287
} else if (curr->type.isConcrete()) {

test/passes/flatten.bin.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@
104104
(nop)
105105
(unreachable)
106106
)
107-
(unreachable)
108107
)
109108
(func $9 (; 9 ;) (param $0 i64) (param $1 f32) (param $2 f64) (param $3 i32) (param $4 i32) (result f64)
110109
(local $5 i64)

0 commit comments

Comments
 (0)