Skip to content

Commit f284d54

Browse files
kripkenkripken
authored
Vacuum: Do not turn unreachable into nop (#8387)
This allows propagation of unreachability to callers. Also verify that inlining propagates it.
1 parent 9ee4a25 commit f284d54

3 files changed

Lines changed: 147 additions & 8 deletions

File tree

‎src/passes/Vacuum.cpp‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <ir/branch-hints.h>
2323
#include <ir/drop.h>
2424
#include <ir/effects.h>
25+
#include <ir/find_all.h>
2526
#include <ir/intrinsics.h>
2627
#include <ir/iteration.h>
2728
#include <ir/literal-utils.h>
@@ -473,10 +474,40 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
473474
} else {
474475
ExpressionManipulator::nop(curr->body);
475476
}
476-
if (curr->getResults() == Type::none &&
477-
!EffectAnalyzer(getPassOptions(), *getModule(), curr)
478-
.hasUnremovableSideEffects()) {
479-
ExpressionManipulator::nop(curr->body);
477+
if (curr->getResults() == Type::none) {
478+
EffectAnalyzer effects(getPassOptions(), *getModule(), curr);
479+
if (!effects.hasUnremovableSideEffects()) {
480+
// We can remove these contents. However, there is one situation we want
481+
// to handle here: in trapsNeverHappen mode, we can remove traps, but
482+
// we don't want to remove an actual Unreachable - replacing an
483+
// Unreachable with a Nop is valid, but does not propagate to callers in
484+
// other passes.
485+
//
486+
// To avoid that situation, after finding we can remove the code, we
487+
// also require that no Unreachable exists. Note that this is unoptimal:
488+
// there may be a complex bundle of code whose only effect is to
489+
// potentially trap, and it happens to contain an Unreachable inside
490+
// somewhere, then that would prevent us from nopping the entire thing.
491+
// But we leave untangling such code for other passes.
492+
//
493+
// This is also unoptimal as it is a heuristic: some toolchain might
494+
// emit 0 / 0 for a logical trap, rather than an Unreachable. We would
495+
// remove that 0 / 0 if we saw it, and the trap would not propagate.
496+
// (But other passes would handle it, if they saw it first.)
497+
if (effects.trap) {
498+
// The code is removable, so the trap is the only effect it has, and
499+
// we are considering removing it because TNH is enabled.
500+
assert(getPassOptions().trapsNeverHappen);
501+
if (!FindAll<Unreachable>(curr->body).list.empty()) {
502+
return;
503+
}
504+
}
505+
// Either trapsNeverHappen and there is no Unreachable (so we are only
506+
// removing implicit traps, which is fine), or traps may happen in terms
507+
// of the flag, but not in this actual code. Either way, we can remove
508+
// all of this.
509+
ExpressionManipulator::nop(curr->body);
510+
}
480511
}
481512
}
482513
};

‎test/lit/passes/inlining_tnh.wat‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
2+
3+
;; RUN: foreach %s %t wasm-opt --inlining -S -o - | filecheck %s
4+
;; RUN: foreach %s %t wasm-opt --inlining -tnh -S -o - | filecheck %s
5+
6+
;; Check that with or without TrapsNeverHappen, we inline calls to trapping
7+
;; functions. That propagates the unreachability for other passes.
8+
9+
(module
10+
;; CHECK: (type $0 (func))
11+
12+
;; CHECK: (func $call-trap
13+
;; CHECK-NEXT: (if
14+
;; CHECK-NEXT: (i32.const 42)
15+
;; CHECK-NEXT: (then
16+
;; CHECK-NEXT: (block $__inlined_func$trap
17+
;; CHECK-NEXT: (unreachable)
18+
;; CHECK-NEXT: )
19+
;; CHECK-NEXT: )
20+
;; CHECK-NEXT: (else
21+
;; CHECK-NEXT: (block $__inlined_func$trap$1
22+
;; CHECK-NEXT: (unreachable)
23+
;; CHECK-NEXT: )
24+
;; CHECK-NEXT: )
25+
;; CHECK-NEXT: )
26+
;; CHECK-NEXT: )
27+
(func $call-trap
28+
;; Call twice to avoid the single-caller rules, which always inline.
29+
(if
30+
(i32.const 42)
31+
(then
32+
(call $trap)
33+
)
34+
(else
35+
(call $trap)
36+
)
37+
)
38+
)
39+
40+
(func $trap
41+
(unreachable)
42+
)
43+
44+
;; CHECK: (func $call-trap-value
45+
;; CHECK-NEXT: (drop
46+
;; CHECK-NEXT: (if
47+
;; CHECK-NEXT: (i32.const 42)
48+
;; CHECK-NEXT: (then
49+
;; CHECK-NEXT: (block $__inlined_func$trap-value$2
50+
;; CHECK-NEXT: (unreachable)
51+
;; CHECK-NEXT: )
52+
;; CHECK-NEXT: )
53+
;; CHECK-NEXT: (else
54+
;; CHECK-NEXT: (block $__inlined_func$trap-value$3
55+
;; CHECK-NEXT: (unreachable)
56+
;; CHECK-NEXT: )
57+
;; CHECK-NEXT: )
58+
;; CHECK-NEXT: )
59+
;; CHECK-NEXT: )
60+
;; CHECK-NEXT: )
61+
(func $call-trap-value
62+
(drop
63+
(if (result i32)
64+
(i32.const 42)
65+
(then
66+
(call $trap-value)
67+
)
68+
(else
69+
(call $trap-value)
70+
)
71+
)
72+
)
73+
)
74+
75+
(func $trap-value (result i32)
76+
(unreachable)
77+
)
78+
)

‎test/lit/passes/vacuum-tnh.wast‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
(type $struct (struct (field (mut i32))))
2020

2121
;; YESTNH: (func $drop (type $4) (param $x i32) (param $y anyref)
22-
;; YESTNH-NEXT: (nop)
22+
;; YESTNH-NEXT: (drop
23+
;; YESTNH-NEXT: (unreachable)
24+
;; YESTNH-NEXT: )
2325
;; YESTNH-NEXT: )
2426
;; NO_TNH: (func $drop (type $4) (param $x i32) (param $y anyref)
2527
;; NO_TNH-NEXT: (drop
@@ -62,7 +64,9 @@
6264
)
6365
)
6466

65-
;; Ignore unreachable code.
67+
;; Ignore unreachable code. Note that we leave this dropped unreachable in
68+
;; the final output - in TNH it is valid to turn it into a nop, but we do
69+
;; not remove unreachables, to let them propagate to callers.
6670
(drop
6771
(unreachable)
6872
)
@@ -180,17 +184,43 @@
180184
)
181185

182186
;; YESTNH: (func $toplevel (type $0)
183-
;; YESTNH-NEXT: (nop)
187+
;; YESTNH-NEXT: (unreachable)
184188
;; YESTNH-NEXT: )
185189
;; NO_TNH: (func $toplevel (type $0)
186190
;; NO_TNH-NEXT: (unreachable)
187191
;; NO_TNH-NEXT: )
188192
(func $toplevel
189193
;; A removable side effect at the top level of a function. We can turn this
190-
;; into a nop.
194+
;; into a nop, but leave it as unreachable even in TNH, so that it can
195+
;; propagate to callers.
191196
(unreachable)
192197
)
193198

199+
;; YESTNH: (func $toplevel-might-trap (type $0)
200+
;; YESTNH-NEXT: (local $0 i32)
201+
;; YESTNH-NEXT: (nop)
202+
;; YESTNH-NEXT: )
203+
;; NO_TNH: (func $toplevel-might-trap (type $0)
204+
;; NO_TNH-NEXT: (local $0 i32)
205+
;; NO_TNH-NEXT: (local.set $0
206+
;; NO_TNH-NEXT: (i32.load
207+
;; NO_TNH-NEXT: (i32.const 0)
208+
;; NO_TNH-NEXT: )
209+
;; NO_TNH-NEXT: )
210+
;; NO_TNH-NEXT: )
211+
(func $toplevel-might-trap
212+
;; This might trap, but we can still remove it all in TNH mode: the implicit
213+
;; trap does not inhibit us from removing this code. (If we saw an explicit
214+
;; unreachable, we would not remove it, as tested above.)
215+
(local $0 i32)
216+
(local.set $0
217+
(i32.load
218+
(i32.const 0)
219+
)
220+
)
221+
)
222+
223+
194224
;; YESTNH: (func $drop-loop (type $0)
195225
;; YESTNH-NEXT: (drop
196226
;; YESTNH-NEXT: (loop $loop (result i32)

0 commit comments

Comments
 (0)