diff options
| -rw-r--r-- | src/ir/effects.h | 52 | ||||
| -rw-r--r-- | src/ir/properties.h | 2 | ||||
| -rw-r--r-- | src/passes/CodeFolding.cpp | 2 | ||||
| -rw-r--r-- | src/passes/LoopInvariantCodeMotion.cpp | 2 | ||||
| -rw-r--r-- | src/passes/SimplifyLocals.cpp | 2 | ||||
| -rw-r--r-- | src/passes/Vacuum.cpp | 2 | ||||
| -rw-r--r-- | test/lit/passes/code-pushing-eh.wast | 132 | ||||
| -rw-r--r-- | test/lit/passes/vacuum-eh.wast | 51 |
8 files changed, 216 insertions, 29 deletions
diff --git a/src/ir/effects.h b/src/ir/effects.h index ced8e63cb..808693e95 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -111,9 +111,9 @@ public: // An atomic load/store/RMW/Cmpxchg or an operator that has a defined ordering // wrt atomics (e.g. memory.grow) bool isAtomic = false; - bool throws = false; + bool throws_ = false; // The nested depth of try-catch_all. If an instruction that may throw is - // inside an inner try-catch_all, we don't mark it as 'throws', because it + // inside an inner try-catch_all, we don't mark it as 'throws_', because it // will be caught by an inner catch_all. We only count 'try's with a // 'catch_all' because instructions within a 'try' without a 'catch_all' can // still throw outside of the try. @@ -144,6 +144,7 @@ public: return accessesMutableStruct() || readsImmutableStruct; } bool accessesArray() const { return calls || readsArray || writesArray; } + bool throws() const { return throws_ || !delegateTargets.empty(); } // Check whether this may transfer control flow to somewhere outside of this // expression (aside from just flowing out normally). That includes a break // or a throw (if the throw is not known to be caught inside this expression; @@ -152,7 +153,7 @@ public: // caught in the function at all, which would mean control flow cannot be // transferred inside the function, but this expression does not know that). bool transfersControlFlow() const { - return branchesOut || throws || hasExternalBreakTargets(); + return branchesOut || throws() || hasExternalBreakTargets(); } // Changes something in globally-stored state. @@ -167,7 +168,7 @@ public: bool hasNonTrapSideEffects() const { return localsWritten.size() > 0 || danglingPop || writesGlobalState() || - throws || transfersControlFlow(); + throws() || transfersControlFlow(); } bool hasSideEffects() const { return trap || hasNonTrapSideEffects(); } @@ -261,7 +262,7 @@ public: // function, so transfersControlFlow would be true) - while we allow the // reordering of traps with each other, we do not reorder exceptions with // anything. - assert(!((trap && other.throws) || (throws && other.trap))); + assert(!((trap && other.throws()) || (throws() && other.trap))); // We can't reorder an implicit trap in a way that could alter what global // state is modified. if ((trap && other.writesGlobalState()) || @@ -287,7 +288,7 @@ public: implicitTrap = implicitTrap || other.implicitTrap; trapsNeverHappen = trapsNeverHappen || other.trapsNeverHappen; isAtomic = isAtomic || other.isAtomic; - throws = throws || other.throws; + throws_ = throws_ || other.throws_; danglingPop = danglingPop || other.danglingPop; for (auto i : other.localsRead) { localsRead.insert(i); @@ -307,6 +308,9 @@ public: for (auto i : other.breakTargets) { breakTargets.insert(i); } + for (auto i : other.delegateTargets) { + delegateTargets.insert(i); + } } // the checks above happen after the node's children were processed, in the @@ -329,6 +333,7 @@ public: } std::set<Name> breakTargets; + std::set<Name> delegateTargets; private: struct InternalAnalyzer @@ -369,6 +374,18 @@ private: static void doStartCatch(InternalAnalyzer* self, Expression** currp) { Try* curr = (*currp)->cast<Try>(); + // This is conservative. When an inner try-delegate targets the current + // expression, even if the try-delegate's body can't throw, we consider + // the current expression can throw for simplicity, unless the current + // expression is not inside a try-catch_all. It is hard to figure out + // whether the original try-delegate's body throws or not at this point. + if (curr->name.is()) { + if (self->parent.delegateTargets.count(curr->name) && + self->parent.tryDepth == 0) { + self->parent.throws_ = true; + } + self->parent.delegateTargets.erase(curr->name); + } // We only count 'try's with a 'catch_all' because instructions within a // 'try' without a 'catch_all' can still throw outside of the try. if (curr->hasCatchAll()) { @@ -423,7 +440,7 @@ private: parent.calls = true; // When EH is enabled, any call can throw. if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws = true; + parent.throws_ = true; } if (curr->isReturn) { parent.branchesOut = true; @@ -438,7 +455,7 @@ private: void visitCallIndirect(CallIndirect* curr) { parent.calls = true; if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws = true; + parent.throws_ = true; } if (curr->isReturn) { parent.branchesOut = true; @@ -631,15 +648,19 @@ private: parent.readsTable = true; parent.writesTable = true; } - void visitTry(Try* curr) {} + void visitTry(Try* curr) { + if (curr->delegateTarget.is()) { + parent.delegateTargets.insert(curr->delegateTarget); + } + } void visitThrow(Throw* curr) { if (parent.tryDepth == 0) { - parent.throws = true; + parent.throws_ = true; } } void visitRethrow(Rethrow* curr) { if (parent.tryDepth == 0) { - parent.throws = true; + parent.throws_ = true; } // traps when the arg is null parent.implicitTrap = true; @@ -658,7 +679,7 @@ private: void visitCallRef(CallRef* curr) { parent.calls = true; if (parent.features.hasExceptionHandling() && parent.tryDepth == 0) { - parent.throws = true; + parent.throws_ = true; } if (curr->isReturn) { parent.branchesOut = true; @@ -811,7 +832,7 @@ public: if (isAtomic) { effects |= SideEffects::IsAtomic; } - if (throws) { + if (throws_) { effects |= SideEffects::Throws; } if (danglingPop) { @@ -826,7 +847,10 @@ public: } private: - void pre() { breakTargets.clear(); } + void pre() { + breakTargets.clear(); + delegateTargets.clear(); + } void post() { assert(tryDepth == 0); diff --git a/src/ir/properties.h b/src/ir/properties.h index 8800f1049..d2c5affd2 100644 --- a/src/ir/properties.h +++ b/src/ir/properties.h @@ -283,7 +283,7 @@ inline Expression* getImmediateFallthrough(Expression* curr, return br->value; } } else if (auto* tryy = curr->dynCast<Try>()) { - if (!EffectAnalyzer(passOptions, module, tryy->body).throws) { + if (!EffectAnalyzer(passOptions, module, tryy->body).throws()) { return tryy->body; } } else if (auto* as = curr->dynCast<RefCast>()) { diff --git a/src/passes/CodeFolding.cpp b/src/passes/CodeFolding.cpp index 7a492dbea..0323223e1 100644 --- a/src/passes/CodeFolding.cpp +++ b/src/passes/CodeFolding.cpp @@ -319,7 +319,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 (effects.throws && !FindAll<Try>(outOf).list.empty()) { + if (effects.throws() && !FindAll<Try>(outOf).list.empty()) { return false; } } diff --git a/src/passes/LoopInvariantCodeMotion.cpp b/src/passes/LoopInvariantCodeMotion.cpp index f339f8126..7fbd3ef55 100644 --- a/src/passes/LoopInvariantCodeMotion.cpp +++ b/src/passes/LoopInvariantCodeMotion.cpp @@ -125,7 +125,7 @@ struct LoopInvariantCodeMotion (effects.readsMutableGlobalState() && loopEffects.writesGlobalState()); // TODO: look into optimizing this with exceptions. for now, disallow - if (effects.throws || loopEffects.throws) { + if (effects.throws() || loopEffects.throws()) { unsafeToMove = true; } if (!unsafeToMove) { diff --git a/src/passes/SimplifyLocals.cpp b/src/passes/SimplifyLocals.cpp index 8ed822774..35980f503 100644 --- a/src/passes/SimplifyLocals.cpp +++ b/src/passes/SimplifyLocals.cpp @@ -304,7 +304,7 @@ struct SimplifyLocals if (curr->is<Try>()) { std::vector<Index> invalidated; for (auto& [index, info] : self->sinkables) { - if (info.effects.throws) { + if (info.effects.throws()) { invalidated.push_back(index); } } diff --git a/src/passes/Vacuum.cpp b/src/passes/Vacuum.cpp index abd54f334..6adab04e9 100644 --- a/src/passes/Vacuum.cpp +++ b/src/passes/Vacuum.cpp @@ -360,7 +360,7 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> { void visitTry(Try* curr) { // If try's body does not throw, the whole try-catch can be replaced with // the try's body. - if (!EffectAnalyzer(getPassOptions(), *getModule(), curr->body).throws) { + if (!EffectAnalyzer(getPassOptions(), *getModule(), curr->body).throws()) { replaceCurrent(curr->body); for (auto* catchBody : curr->catchBodies) { typeUpdater.noteRecursiveRemoval(catchBody); diff --git a/test/lit/passes/code-pushing-eh.wast b/test/lit/passes/code-pushing-eh.wast index a18b10dae..c428d757d 100644 --- a/test/lit/passes/code-pushing-eh.wast +++ b/test/lit/passes/code-pushing-eh.wast @@ -1,17 +1,19 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. ;; RUN: wasm-opt %s --code-pushing -all -S -o - | filecheck %s +;; The tests in this file test EffectAnalyzer, which is used by CodePushing. + (module ;; CHECK: (tag $e (param i32)) (tag $e (param i32)) - ;; CHECK: (func $cant-push-past-call + ;; CHECK: (func $cannot-push-past-call ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (local.set $x ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (call $cant-push-past-call) + ;; CHECK-NEXT: (call $cannot-push-past-call) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) @@ -23,19 +25,19 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $cant-push-past-call + (func $cannot-push-past-call (local $x i32) (block $out ;; This local.set cannot be pushed down, because the call below can throw (local.set $x (i32.const 1)) - (call $cant-push-past-call) + (call $cannot-push-past-call) (drop (i32.const 1)) (br_if $out (i32.const 2)) (drop (local.get $x)) ) ) - ;; CHECK: (func $cant-push-past-throw + ;; CHECK: (func $cannot-push-past-throw ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (local.set $x @@ -55,7 +57,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $cant-push-past-throw + (func $cannot-push-past-throw (local $x i32) (block $out ;; This local.set cannot be pushed down, because there is 'throw' below @@ -117,7 +119,7 @@ ;; CHECK-NEXT: ) (func $foo) - ;; CHECK: (func $cant-push-past-try + ;; CHECK: (func $cannot-push-past-try ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (local.set $x @@ -144,7 +146,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $cant-push-past-try + (func $cannot-push-past-try (local $x i32) (block $out ;; This local.set cannot be pushed down, because the exception thrown by @@ -164,7 +166,7 @@ ) ) - ;; CHECK: (func $cant-push-past-rethrow-within-catch + ;; CHECK: (func $cannot-push-past-rethrow-within-catch ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (block $out ;; CHECK-NEXT: (local.set $x @@ -191,7 +193,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $cant-push-past-rethrow-within-catch + (func $cannot-push-past-rethrow-within-catch (local $x i32) (block $out ;; This local.set cannot be pushed down, because there is 'rethrow' within @@ -210,4 +212,114 @@ (drop (local.get $x)) ) ) + + ;; CHECK: (func $can-push-past-try-delegate + ;; CHECK-NEXT: (local $x i32) + ;; CHECK-NEXT: (block $out + ;; CHECK-NEXT: (try $l + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (try $try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (throw $e + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (delegate $l) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch_all + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br_if $out + ;; CHECK-NEXT: (i32.const 2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $can-push-past-try-delegate + (local $x i32) + (block $out + ;; This local.set can be pushed down, because the 'throw' below is going + ;; to be caught by the catch_all + (local.set $x (i32.const 1)) + (try $l + (do + (try + (do + (throw $e (i32.const 0)) + ) + (delegate $l) + ) + ) + (catch_all) + ) + (drop (i32.const 1)) + (br_if $out (i32.const 2)) + (drop (local.get $x)) + ) + ) + + ;; CHECK: (func $cannot-push-past-try-delegate + ;; CHECK-NEXT: (local $x i32) + ;; CHECK-NEXT: (block $out + ;; CHECK-NEXT: (local.set $x + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (try $l + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (try $try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (throw $e + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (delegate 2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch_all + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (br_if $out + ;; CHECK-NEXT: (i32.const 2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $x) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $cannot-push-past-try-delegate + (local $x i32) + (block $out + ;; This local.set cannot be pushed down, because the 'delegate' bypasses + ;; the catch_all, making the whole 'try' throwable. + (local.set $x (i32.const 1)) + (try $l + (do + (try + (do + (throw $e (i32.const 0)) + ) + (delegate 2) + ) + ) + (catch_all) + ) + (drop (i32.const 1)) + (br_if $out (i32.const 2)) + (drop (local.get $x)) + ) + ) ) diff --git a/test/lit/passes/vacuum-eh.wast b/test/lit/passes/vacuum-eh.wast index ef6b6ff5b..574a525bc 100644 --- a/test/lit/passes/vacuum-eh.wast +++ b/test/lit/passes/vacuum-eh.wast @@ -125,4 +125,55 @@ ) ) ) + + ;; CHECK: (func $try-delegate-outer-target + ;; CHECK-NEXT: (local $0 i32) + ;; CHECK-NEXT: (try $label$0 + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (try $try + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (try $try2 + ;; CHECK-NEXT: (do + ;; CHECK-NEXT: (throw $e + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (delegate $label$0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (catch_all + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $try-delegate-outer-target + (local $0 i32) + (try $label$0 ;; outer try + (do + ;; If it were not for the inner (delegate $label0), this middle try + ;; cannot throw even if there is a throw in the inner try, because this + ;; try has a catch_all. And Vacuum can replace the outer try-catch with + ;; the try's body if the body doesn't throw. + ;; + ;; But because the inner try has a delegate that targets the outer try, + ;; this middle try can throw, and we can't do the optimization for + ;; the outer try. + (try ;; middle try + (do + (try ;; inner try + (do + (throw $e + (i32.const 0) + ) + ) + (delegate $label$0) + ) + ) + (catch_all) + ) + ) + ) + ) ) |