summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/binaryen-c.cpp4
-rw-r--r--src/binaryen-c.h1
-rw-r--r--src/ir/effects.h30
-rw-r--r--src/js/binaryen.js-post.js1
-rw-r--r--src/passes/CodeFolding.cpp14
-rw-r--r--src/passes/SimplifyLocals.cpp8
-rw-r--r--test/binaryen.js/sideffects.js10
-rw-r--r--test/binaryen.js/sideffects.js.txt3
-rw-r--r--test/passes/simplify-locals_all-features.txt43
-rw-r--r--test/passes/simplify-locals_all-features.wast38
10 files changed, 139 insertions, 13 deletions
diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp
index c94867032..96edb4c21 100644
--- a/src/binaryen-c.cpp
+++ b/src/binaryen-c.cpp
@@ -2898,6 +2898,10 @@ BinaryenSideEffects BinaryenSideEffectIsAtomic(void) {
BinaryenSideEffects BinaryenSideEffectThrows(void) {
return static_cast<BinaryenSideEffects>(EffectAnalyzer::SideEffects::Throws);
}
+BinaryenSideEffects BinaryenSideEffectDanglingPop(void) {
+ return static_cast<BinaryenSideEffects>(
+ EffectAnalyzer::SideEffects::DanglingPop);
+}
BinaryenSideEffects BinaryenSideEffectAny(void) {
return static_cast<BinaryenSideEffects>(EffectAnalyzer::SideEffects::Any);
}
diff --git a/src/binaryen-c.h b/src/binaryen-c.h
index 53fe69b92..9fd08f968 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 d2afc8133..f6c87a4e5 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<Try>()) {
self->pushTask(doVisitTry, currp);
+ self->pushTask(doEndCatch, currp);
self->pushTask(scan, &curr->cast<Try>()->catchBody);
self->pushTask(doStartCatch, currp);
self->pushTask(scan, &curr->cast<Try>()->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 8a2d60a2f..2d7bd3f36 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 10255a670..04819a04d 100644
--- a/src/passes/CodeFolding.cpp
+++ b/src/passes/CodeFolding.cpp
@@ -305,12 +305,12 @@ private:
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<Pop>(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 @@ private:
// 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<Try>(outOf).list.empty()) {
+ if (effects.throws && !FindAll<Try>(outOf).list.empty()) {
return false;
}
}
diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp
index 2f269a00d..85c719d4c 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 76bc1db68..bfb5404c2 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 4aca0ac46..53ee474e5 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 4a003360d..0418e6bd1 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 bfe86f448..4fdedb2f9 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)
+ )
+ )
+ )
+ )
)