diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index c94867032c8..96edb4c21cd 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -2898,6 +2898,10 @@ BinaryenSideEffects BinaryenSideEffectIsAtomic(void) { BinaryenSideEffects BinaryenSideEffectThrows(void) { return static_cast(EffectAnalyzer::SideEffects::Throws); } +BinaryenSideEffects BinaryenSideEffectDanglingPop(void) { + return static_cast( + EffectAnalyzer::SideEffects::DanglingPop); +} BinaryenSideEffects BinaryenSideEffectAny(void) { return static_cast(EffectAnalyzer::SideEffects::Any); } diff --git a/src/binaryen-c.h b/src/binaryen-c.h index 53fe69b9252..9fd08f968cb 100644 --- a/src/binaryen-c.h +++ b/src/binaryen-c.h @@ -1580,6 +1580,7 @@ BINARYEN_API BinaryenSideEffects BinaryenSideEffectWritesMemory(void); BINARYEN_API BinaryenSideEffects BinaryenSideEffectImplicitTrap(void); BINARYEN_API BinaryenSideEffects BinaryenSideEffectIsAtomic(void); BINARYEN_API BinaryenSideEffects BinaryenSideEffectThrows(void); +BINARYEN_API BinaryenSideEffects BinaryenSideEffectDanglingPop(void); BINARYEN_API BinaryenSideEffects BinaryenSideEffectAny(void); BINARYEN_API BinaryenSideEffects BinaryenExpressionGetSideEffects( diff --git a/src/ir/effects.h b/src/ir/effects.h index d2afc813301..f6c87a4e581 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -76,6 +76,11 @@ struct EffectAnalyzer // inner try, we don't mark it as 'throws', because it will be caught by an // inner catch. size_t tryDepth = 0; + // The nested depth of catch. This is necessary to track danglng pops. + size_t catchDepth = 0; + // If this expression contains 'exnref.pop's that are not enclosed in 'catch' + // body. For example, (drop (exnref.pop)) should set this to true. + bool danglingPop = false; static void scan(EffectAnalyzer* self, Expression** currp) { Expression* curr = *currp; @@ -83,6 +88,7 @@ struct EffectAnalyzer // separately if (curr->is()) { self->pushTask(doVisitTry, currp); + self->pushTask(doEndCatch, currp); self->pushTask(scan, &curr->cast()->catchBody); self->pushTask(doStartCatch, currp); self->pushTask(scan, &curr->cast()->body); @@ -100,6 +106,12 @@ struct EffectAnalyzer static void doStartCatch(EffectAnalyzer* self, Expression** currp) { assert(self->tryDepth > 0 && "try depth cannot be negative"); self->tryDepth--; + self->catchDepth++; + } + + static void doEndCatch(EffectAnalyzer* self, Expression** currp) { + assert(self->catchDepth > 0 && "catch depth cannot be negative"); + self->catchDepth--; } // Helper functions to check for various effect types @@ -128,7 +140,7 @@ struct EffectAnalyzer } bool hasSideEffects() const { return hasGlobalSideEffects() || localsWritten.size() > 0 || - transfersControlFlow() || implicitTrap; + transfersControlFlow() || implicitTrap || danglingPop; } bool hasAnything() const { return hasSideEffects() || accessesLocal() || readsMemory || @@ -148,7 +160,8 @@ struct EffectAnalyzer if ((transfersControlFlow() && other.hasSideEffects()) || (other.transfersControlFlow() && hasSideEffects()) || ((writesMemory || calls) && other.accessesMemory()) || - (accessesMemory() && (other.writesMemory || other.calls))) { + (accessesMemory() && (other.writesMemory || other.calls)) || + (danglingPop || other.danglingPop)) { return true; } // All atomics are sequentially consistent for now, and ordered wrt other @@ -203,6 +216,7 @@ struct EffectAnalyzer implicitTrap = implicitTrap || other.implicitTrap; isAtomic = isAtomic || other.isAtomic; throws = throws || other.throws; + danglingPop = danglingPop || other.danglingPop; for (auto i : other.localsRead) { localsRead.insert(i); } @@ -470,7 +484,11 @@ struct EffectAnalyzer } void visitNop(Nop* curr) {} void visitUnreachable(Unreachable* curr) { branchesOut = true; } - void visitPop(Pop* curr) { calls = true; } + void visitPop(Pop* curr) { + if (catchDepth == 0) { + danglingPop = true; + } + } void visitTupleMake(TupleMake* curr) {} void visitTupleExtract(TupleExtract* curr) {} @@ -500,7 +518,8 @@ struct EffectAnalyzer ImplicitTrap = 1 << 8, IsAtomic = 1 << 9, Throws = 1 << 10, - Any = (1 << 11) - 1 + DanglingPop = 1 << 11, + Any = (1 << 12) - 1 }; uint32_t getSideEffects() const { uint32_t effects = 0; @@ -537,6 +556,9 @@ struct EffectAnalyzer if (throws) { effects |= SideEffects::Throws; } + if (danglingPop) { + effects |= SideEffects::DanglingPop; + } return effects; } diff --git a/src/js/binaryen.js-post.js b/src/js/binaryen.js-post.js index 8a2d60a2fea..2d7bd3f3613 100644 --- a/src/js/binaryen.js-post.js +++ b/src/js/binaryen.js-post.js @@ -485,6 +485,7 @@ function initializeConstants() { 'ImplicitTrap', 'IsAtomic', 'Throws', + 'DanglingPop', 'Any' ].forEach(function(name) { Module['SideEffects'][name] = Module['_BinaryenSideEffect' + name](); diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp index 10255a67009..04819a04d00 100644 --- a/src/passes/CodeFolding.cpp +++ b/src/passes/CodeFolding.cpp @@ -305,12 +305,12 @@ struct CodeFolding : public WalkerPass> { return false; } if (getModule()->features.hasExceptionHandling()) { + EffectAnalyzer effects(getPassOptions(), getModule()->features, item); // Currently pop instructions are only used for exnref.pop, which is a - // pseudo instruction following a catch. We check if the current - // expression has a pop child. This can be overly conservative, because - // this can also exclude whole try-catches that contain a pop within - // them. - if (!FindAll(item).list.empty()) { + // pseudo instruction following a catch. We cannot move expressions + // containing pops if they are not enclosed in a 'catch' body, because a + // pop instruction should follow right after 'catch'. + if (effects.danglingPop) { return false; } // When an expression can throw and it is within a try scope, taking it @@ -321,9 +321,7 @@ struct CodeFolding : public WalkerPass> { // conservative approximation because there can be cases that 'try' is // within the expression that may throw so it is safe to take the // expression out. - if (EffectAnalyzer(getPassOptions(), getModule()->features, item) - .throws && - !FindAll(outOf).list.empty()) { + if (effects.throws && !FindAll(outOf).list.empty()) { return false; } } diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index 2f269a00daf..85c719d4c33 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -411,6 +411,14 @@ struct SimplifyLocals if (set->isTee()) { return false; } + // We cannot move expressions containing exnref.pops that are not enclosed + // in 'catch', because 'exnref.pop' should follow right after 'catch'. + FeatureSet features = this->getModule()->features; + if (features.hasExceptionHandling() && + EffectAnalyzer(this->getPassOptions(), features, set->value) + .danglingPop) { + return false; + } // if in the first cycle, or not allowing tees, then we cannot sink if >1 // use as that would make a tee if ((firstCycle || !allowTee) && getCounter.num[set->index] > 1) { diff --git a/test/binaryen.js/sideffects.js b/test/binaryen.js/sideffects.js index 76bc1db68dd..bfb5404c2ac 100644 --- a/test/binaryen.js/sideffects.js +++ b/test/binaryen.js/sideffects.js @@ -10,6 +10,7 @@ console.log("SideEffects.WritesMemory=" + binaryen.SideEffects.WritesMemory); console.log("SideEffects.ImplicitTrap=" + binaryen.SideEffects.ImplicitTrap); console.log("SideEffects.IsAtomic=" + binaryen.SideEffects.IsAtomic); console.log("SideEffects.Throws=" + binaryen.SideEffects.Throws); +console.log("SideEffects.DanglingPop=" + binaryen.SideEffects.DanglingPop); console.log("SideEffects.Any=" + binaryen.SideEffects.Any); var module = new binaryen.Module(); @@ -104,3 +105,12 @@ assert( == binaryen.SideEffects.Calls | binaryen.SideEffects.Throws ); + +assert( + binaryen.getSideEffects( + module.drop(module.exnref.pop()), + module.getFeatures() + ) + == + binaryen.SideEffects.DanglingPop +); diff --git a/test/binaryen.js/sideffects.js.txt b/test/binaryen.js/sideffects.js.txt index 4aca0ac4694..53ee474e5eb 100644 --- a/test/binaryen.js/sideffects.js.txt +++ b/test/binaryen.js/sideffects.js.txt @@ -10,4 +10,5 @@ SideEffects.WritesMemory=128 SideEffects.ImplicitTrap=256 SideEffects.IsAtomic=512 SideEffects.Throws=1024 -SideEffects.Any=2047 +SideEffects.DanglingPop=2048 +SideEffects.Any=4095 diff --git a/test/passes/simplify-locals_all-features.txt b/test/passes/simplify-locals_all-features.txt index 4a003360d17..0418e6bd1fd 100644 --- a/test/passes/simplify-locals_all-features.txt +++ b/test/passes/simplify-locals_all-features.txt @@ -1894,6 +1894,7 @@ ) (module (type $none_=>_none (func)) + (type $i32_exnref_=>_none (func (param i32 exnref))) (type $exnref_=>_none (func (param exnref))) (type $none_=>_exnref (func (result exnref))) (event $event$0 (attr 0) (param)) @@ -1937,4 +1938,46 @@ ) ) ) + (func $foo (param $0 i32) (param $1 exnref) + (nop) + ) + (func $pop-cannot-be-sinked + (local $0 exnref) + (try + (do + (nop) + ) + (catch + (local.set $0 + (exnref.pop) + ) + (call $foo + (i32.const 3) + (local.get $0) + ) + ) + ) + ) + (func $pop-within-catch-can-be-sinked + (local $0 exnref) + (try + (do + (nop) + ) + (catch + (nop) + (call $foo + (i32.const 3) + (try (result exnref) + (do + (ref.null) + ) + (catch + (exnref.pop) + ) + ) + ) + ) + ) + ) ) diff --git a/test/passes/simplify-locals_all-features.wast b/test/passes/simplify-locals_all-features.wast index bfe86f44873..4fdedb2f9f7 100644 --- a/test/passes/simplify-locals_all-features.wast +++ b/test/passes/simplify-locals_all-features.wast @@ -1711,4 +1711,42 @@ ) ) ) + + (func $foo (param i32 exnref)) + (func $pop-cannot-be-sinked (local $0 exnref) + (try + (do) + (catch + ;; This (local.set $0) of (exnref.pop) cannot be sinked to + ;; (local.get $0) below, because exnref.pop should follow right after + ;; 'catch'. + (local.set $0 (exnref.pop)) + (call $foo + (i32.const 3) + (local.get $0) + ) + ) + ) + ) + + (func $pop-within-catch-can-be-sinked (local $0 exnref) + (try + (do) + (catch + ;; This whole 'try' body can be sinked to eliminate local.set / + ;; local.get. Even though it contains a pop, it is enclosed within + ;; try-catch, so it is OK. + (local.set $0 + (try (result exnref) + (do (ref.null)) + (catch (exnref.pop)) + ) + ) + (call $foo + (i32.const 3) + (local.get $0) + ) + ) + ) + ) )