diff options
author | Alon Zakai <azakai@google.com> | 2023-05-10 12:36:08 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-10 12:36:08 -0700 |
commit | c5c223943770412b2ebd7d9f23fce8c11cf5982e (patch) | |
tree | 4a4038c2a6caee19d0dc0f803535a57fa06a9d01 | |
parent | ee738ac1f838a090cac74ba8981e2104b6c02d44 (diff) | |
download | binaryen-c5c223943770412b2ebd7d9f23fce8c11cf5982e.tar.gz binaryen-c5c223943770412b2ebd7d9f23fce8c11cf5982e.tar.bz2 binaryen-c5c223943770412b2ebd7d9f23fce8c11cf5982e.zip |
Add a "mayNotReturn" effect (#5711)
This changes loops from having the effect "may trap (timeout)" to having
"may not return." The only noticeable difference is in TrapsNeverHappen mode,
which ignores the former but not the latter. So after this PR, in TNH mode we do
not optimize away an infinite loop that seems to have no other side effects. We
may also use this for other things in the future, like continuations/stack switching.
There are upsides and downsides to allowing the optimizer to remove infinite
loops (the C and C++ communities have had interesting discussions on that topic
over the years...) but it seems safer to not optimize them out for now, to let the
most code work properly. If a need comes up to optimize such code, we can look
at various options then (like a flag to ignore infinite loops).
See discussion in #5228
-rw-r--r-- | src/ir/effects.h | 16 | ||||
-rw-r--r-- | test/lit/passes/vacuum-tnh.wast | 13 | ||||
-rw-r--r-- | test/wasm2js/br_table_to_loop.2asm.js.opt | 4 |
3 files changed, 16 insertions, 17 deletions
diff --git a/src/ir/effects.h b/src/ir/effects.h index fe2520299..893ff2cae 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -122,10 +122,6 @@ public: // *do* mark a potentially infinite number of allocations as trapping, as all // VMs would trap eventually, and the same for potentially infinite recursion, // etc. - // * We assume that VMs will timeout eventually, so any loop that we cannot - // prove terminates is considered to trap. (Some VMs might not have - // such timeouts, but even they will error before the heat death of the - // universe, which is a kind of trap.) bool trap = false; // A trap from an instruction like a load or div/rem, which may trap on corner // cases. If we do not ignore implicit traps then these are counted as a trap. @@ -145,6 +141,9 @@ public: // If this expression contains 'pop's that are not enclosed in 'catch' body. // For example, (drop (pop i32)) should set this to true. bool danglingPop = false; + // Whether this code may "hang" and not eventually complete. An infinite loop, + // or a continuation that is never continued, are examples of that. + bool mayNotReturn = false; // Helper functions to check for various effect types @@ -184,7 +183,7 @@ public: bool hasNonTrapSideEffects() const { return localsWritten.size() > 0 || danglingPop || writesGlobalState() || - throws() || transfersControlFlow(); + throws() || transfersControlFlow() || mayNotReturn; } bool hasSideEffects() const { return trap || hasNonTrapSideEffects(); } @@ -419,12 +418,7 @@ private: void visitIf(If* curr) {} void visitLoop(Loop* curr) { if (curr->name.is() && parent.breakTargets.erase(curr->name) > 0) { - // Breaks to this loop exist, which we just removed as they do not have - // further effect outside of this loop. One additional thing we need to - // take into account is infinite looping, which is a noticeable side - // effect we can't normally remove - eventually the VM will time out and - // error (see more details in the comment on trapping above). - parent.implicitTrap = true; + parent.mayNotReturn = true; } } void visitBreak(Break* curr) { parent.breakTargets.insert(curr->name); } diff --git a/test/lit/passes/vacuum-tnh.wast b/test/lit/passes/vacuum-tnh.wast index 6b0907989..e4ce9765d 100644 --- a/test/lit/passes/vacuum-tnh.wast +++ b/test/lit/passes/vacuum-tnh.wast @@ -186,7 +186,14 @@ ) ;; YESTNH: (func $drop-loop (type $none_=>_none) - ;; YESTNH-NEXT: (nop) + ;; YESTNH-NEXT: (drop + ;; YESTNH-NEXT: (loop $loop (result i32) + ;; YESTNH-NEXT: (br_if $loop + ;; YESTNH-NEXT: (i32.const 1) + ;; YESTNH-NEXT: ) + ;; YESTNH-NEXT: (i32.const 10) + ;; YESTNH-NEXT: ) + ;; YESTNH-NEXT: ) ;; YESTNH-NEXT: ) ;; NO_TNH: (func $drop-loop (type $none_=>_none) ;; NO_TNH-NEXT: (drop @@ -199,8 +206,8 @@ ;; NO_TNH-NEXT: ) ;; NO_TNH-NEXT: ) (func $drop-loop - ;; A loop has effects, since it might infinite loop (and hit a timeout trap - ;; eventually), so we do not vacuum it out unless we are ignoring traps. + ;; A loop has the effect of potentially being infinite. Even in TNH mode we + ;; do not optimize out such loops. (drop (loop $loop (result i32) (br_if $loop diff --git a/test/wasm2js/br_table_to_loop.2asm.js.opt b/test/wasm2js/br_table_to_loop.2asm.js.opt index 5c6fa5e7f..4c9f97311 100644 --- a/test/wasm2js/br_table_to_loop.2asm.js.opt +++ b/test/wasm2js/br_table_to_loop.2asm.js.opt @@ -1,6 +1,4 @@ -function wasm2js_trap() { throw new Error('abort'); } - function asmFunc(imports) { var Math_imul = Math.imul; var Math_fround = Math.fround; @@ -13,7 +11,7 @@ function asmFunc(imports) { var Math_trunc = Math.trunc; var Math_sqrt = Math.sqrt; function $0() { - wasm2js_trap(); + while (1) continue; } function $1() { |