diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index 0bcb3a387e6..2b5ec3f191b 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -473,10 +474,40 @@ struct Vacuum : public WalkerPass> { } else { ExpressionManipulator::nop(curr->body); } - if (curr->getResults() == Type::none && - !EffectAnalyzer(getPassOptions(), *getModule(), curr) - .hasUnremovableSideEffects()) { - ExpressionManipulator::nop(curr->body); + if (curr->getResults() == Type::none) { + EffectAnalyzer effects(getPassOptions(), *getModule(), curr); + if (!effects.hasUnremovableSideEffects()) { + // We can remove these contents. However, there is one situation we want + // to handle here: in trapsNeverHappen mode, we can remove traps, but + // we don't want to remove an actual Unreachable - replacing an + // Unreachable with a Nop is valid, but does not propagate to callers in + // other passes. + // + // To avoid that situation, after finding we can remove the code, we + // also require that no Unreachable exists. Note that this is unoptimal: + // there may be a complex bundle of code whose only effect is to + // potentially trap, and it happens to contain an Unreachable inside + // somewhere, then that would prevent us from nopping the entire thing. + // But we leave untangling such code for other passes. + // + // This is also unoptimal as it is a heuristic: some toolchain might + // emit 0 / 0 for a logical trap, rather than an Unreachable. We would + // remove that 0 / 0 if we saw it, and the trap would not propagate. + // (But other passes would handle it, if they saw it first.) + if (effects.trap) { + // The code is removable, so the trap is the only effect it has, and + // we are considering removing it because TNH is enabled. + assert(getPassOptions().trapsNeverHappen); + if (!FindAll(curr->body).list.empty()) { + return; + } + } + // Either trapsNeverHappen and there is no Unreachable (so we are only + // removing implicit traps, which is fine), or traps may happen in terms + // of the flag, but not in this actual code. Either way, we can remove + // all of this. + ExpressionManipulator::nop(curr->body); + } } } }; diff --git a/test/lit/passes/inlining_tnh.wat b/test/lit/passes/inlining_tnh.wat new file mode 100644 index 00000000000..f8a52502d99 --- /dev/null +++ b/test/lit/passes/inlining_tnh.wat @@ -0,0 +1,78 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt --inlining -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt --inlining -tnh -S -o - | filecheck %s + +;; Check that with or without TrapsNeverHappen, we inline calls to trapping +;; functions. That propagates the unreachability for other passes. + +(module + ;; CHECK: (type $0 (func)) + + ;; CHECK: (func $call-trap + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (block $__inlined_func$trap + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (block $__inlined_func$trap$1 + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-trap + ;; Call twice to avoid the single-caller rules, which always inline. + (if + (i32.const 42) + (then + (call $trap) + ) + (else + (call $trap) + ) + ) + ) + + (func $trap + (unreachable) + ) + + ;; CHECK: (func $call-trap-value + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (if + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: (then + ;; CHECK-NEXT: (block $__inlined_func$trap-value$2 + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (else + ;; CHECK-NEXT: (block $__inlined_func$trap-value$3 + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $call-trap-value + (drop + (if (result i32) + (i32.const 42) + (then + (call $trap-value) + ) + (else + (call $trap-value) + ) + ) + ) + ) + + (func $trap-value (result i32) + (unreachable) + ) +) diff --git a/test/lit/passes/vacuum-tnh.wast b/test/lit/passes/vacuum-tnh.wast index 8bfcfd95170..6203c2f3420 100644 --- a/test/lit/passes/vacuum-tnh.wast +++ b/test/lit/passes/vacuum-tnh.wast @@ -19,7 +19,9 @@ (type $struct (struct (field (mut i32)))) ;; YESTNH: (func $drop (type $4) (param $x i32) (param $y anyref) - ;; YESTNH-NEXT: (nop) + ;; YESTNH-NEXT: (drop + ;; YESTNH-NEXT: (unreachable) + ;; YESTNH-NEXT: ) ;; YESTNH-NEXT: ) ;; NO_TNH: (func $drop (type $4) (param $x i32) (param $y anyref) ;; NO_TNH-NEXT: (drop @@ -62,7 +64,9 @@ ) ) - ;; Ignore unreachable code. + ;; Ignore unreachable code. Note that we leave this dropped unreachable in + ;; the final output - in TNH it is valid to turn it into a nop, but we do + ;; not remove unreachables, to let them propagate to callers. (drop (unreachable) ) @@ -180,17 +184,43 @@ ) ;; YESTNH: (func $toplevel (type $0) - ;; YESTNH-NEXT: (nop) + ;; YESTNH-NEXT: (unreachable) ;; YESTNH-NEXT: ) ;; NO_TNH: (func $toplevel (type $0) ;; NO_TNH-NEXT: (unreachable) ;; NO_TNH-NEXT: ) (func $toplevel ;; A removable side effect at the top level of a function. We can turn this - ;; into a nop. + ;; into a nop, but leave it as unreachable even in TNH, so that it can + ;; propagate to callers. (unreachable) ) + ;; YESTNH: (func $toplevel-might-trap (type $0) + ;; YESTNH-NEXT: (local $0 i32) + ;; YESTNH-NEXT: (nop) + ;; YESTNH-NEXT: ) + ;; NO_TNH: (func $toplevel-might-trap (type $0) + ;; NO_TNH-NEXT: (local $0 i32) + ;; NO_TNH-NEXT: (local.set $0 + ;; NO_TNH-NEXT: (i32.load + ;; NO_TNH-NEXT: (i32.const 0) + ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: ) + ;; NO_TNH-NEXT: ) + (func $toplevel-might-trap + ;; This might trap, but we can still remove it all in TNH mode: the implicit + ;; trap does not inhibit us from removing this code. (If we saw an explicit + ;; unreachable, we would not remove it, as tested above.) + (local $0 i32) + (local.set $0 + (i32.load + (i32.const 0) + ) + ) + ) + + ;; YESTNH: (func $drop-loop (type $0) ;; YESTNH-NEXT: (drop ;; YESTNH-NEXT: (loop $loop (result i32)