diff --git a/src/ir/effects.h b/src/ir/effects.h index 44cc8031f45..a14bfc419ef 100644 --- a/src/ir/effects.h +++ b/src/ir/effects.h @@ -716,6 +716,10 @@ class EffectAnalyzer { } void visitCall(Call* curr) { + if (curr->isReturn) { + parent.branchesOut = true; + } + // call.without.effects has no effects. if (Intrinsics(parent.module).isCallWithoutEffects(curr)) { return; @@ -730,36 +734,19 @@ class EffectAnalyzer { if (auto* target = parent.module.getFunctionOrNull(curr->target)) { targetEffects = target->effects.get(); } + if (targetEffects) { + populateEffectsFromGlobalEffects(*targetEffects, curr); + return; + } if (curr->isReturn) { parent.branchesOut = true; // When EH is enabled, any call can throw. - if (parent.features.hasExceptionHandling() && - (!targetEffects || targetEffects->throws())) { + if (parent.features.hasExceptionHandling()) { parent.hasReturnCallThrow = true; } } - if (targetEffects) { - // We have effect information for this call target, and can just use - // that. The one change we may want to make is to remove throws_, if the - // target function throws and we know that will be caught anyhow, the - // same as the code below for the general path. We can always filter out - // throws for return calls because they are already more precisely - // captured by `branchesOut`, which models the return, and - // `hasReturnCallThrow`, which models the throw that will happen after - // the return. - if (targetEffects->throws_ && (parent.tryDepth > 0 || curr->isReturn)) { - auto filteredEffects = *targetEffects; - filteredEffects.throws_ = false; - parent.mergeIn(filteredEffects); - } else { - // Just merge in all the effects. - parent.mergeIn(*targetEffects); - } - return; - } - parent.calls = true; // When EH is enabled, any call can throw. Skip this for return calls // because the throw is already more precisely captured by the combination @@ -770,16 +757,34 @@ class EffectAnalyzer { } } void visitCallIndirect(CallIndirect* curr) { - parent.calls = true; + auto* table = parent.module.getTable(curr->table); + if (!Type::isSubType(Type(curr->heapType, Nullability::Nullable), table->type)) { + parent.trap = true; + return; + } + if (table->type.isNullable()) { + parent.implicitTrap = true; + } if (curr->isReturn) { parent.branchesOut = true; - if (parent.features.hasExceptionHandling()) { + } + + if (auto it = parent.module.typeEffects.find(curr->heapType); + it != parent.module.typeEffects.end() && it->second) { + populateEffectsFromGlobalEffects(*it->second, curr); + return; + } + + parent.calls = true; + // If EH is enabled and we don't have global effects information, + // assume that the call body may throw. + if (parent.features.hasExceptionHandling()) { + if (curr->isReturn) { parent.hasReturnCallThrow = true; } - } - if (parent.features.hasExceptionHandling() && - (parent.tryDepth == 0 && !curr->isReturn)) { - parent.throws_ = true; + if (parent.tryDepth == 0 && !curr->isReturn) { + parent.throws_ = true; + } } } void visitLocalGet(LocalGet* curr) { @@ -1042,16 +1047,29 @@ class EffectAnalyzer { if (trapOnNull(curr->target)) { return; } + if (curr->isReturn) { parent.branchesOut = true; - if (parent.features.hasExceptionHandling()) { - parent.hasReturnCallThrow = true; - } + } + + if (auto it = + parent.module.typeEffects.find(curr->target->type.getHeapType()); + it != parent.module.typeEffects.end() && it->second) { + populateEffectsFromGlobalEffects(*it->second, curr); + return; } parent.calls = true; - if (parent.features.hasExceptionHandling() && - (parent.tryDepth == 0 && !curr->isReturn)) { - parent.throws_ = true; + + // If EH is enabled and we don't have global effects information, + // assume that the call body may throw. + if (parent.features.hasExceptionHandling()) { + if (curr->isReturn) { + parent.hasReturnCallThrow = true; + } + + if (parent.tryDepth == 0 && !curr->isReturn) { + parent.throws_ = true; + } } } void visitRefTest(RefTest* curr) {} @@ -1335,6 +1353,25 @@ class EffectAnalyzer { parent.throws_ = true; } } + + private: + template + void populateEffectsFromGlobalEffects(const EffectAnalyzer& effects, const CallType* curr) { + if (curr->isReturn) { + if (effects.throws()) { + parent.hasReturnCallThrow = true; + } + } + + if (effects.throws_ && (parent.tryDepth > 0 || curr->isReturn)) { + auto filteredEffects = effects; + filteredEffects.throws_ = false; + parent.mergeIn(filteredEffects); + } else { + // Just merge in all the effects. + parent.mergeIn(effects); + } + } }; public: diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 69f29101c86..563d110f6da 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -324,6 +324,25 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) { for (auto& tag : wasm.tags) { tag->type = updater.getNew(tag->type); } + + // Update type effects. + std::unordered_map> + newTypeEffects; + for (auto& [oldType, effects] : wasm.typeEffects) { + if (!effects) { + continue; + } + auto newType = updater.getNew(oldType); + auto& targetEffects = newTypeEffects[newType]; + if (!targetEffects) { + targetEffects = effects; + } else { + auto merged = std::make_shared(*targetEffects); + merged->mergeIn(*effects); + targetEffects = merged; + } + } + wasm.typeEffects = std::move(newTypeEffects); } void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) { diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index ca82b2b3aea..d7285496893 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -24,6 +24,7 @@ #include "pass.h" #include "support/graph_traversal.h" #include "support/strongly_connected_components.h" +#include "support/utilities.h" #include "wasm.h" namespace wasm { @@ -225,10 +226,13 @@ void mergeMaybeEffects(std::optional& dest, // - Merge all of the effects of functions within the CC // - Also merge the (already computed) effects of each callee CC // - Add trap effects for potentially recursive call chains -void propagateEffects(const Module& module, - const PassOptions& passOptions, - std::map& funcInfos, - const CallGraph& callGraph) { +void propagateEffects( + const Module& module, + const PassOptions& passOptions, + std::map& funcInfos, + std::unordered_map>& + typeEffects, + const CallGraph& callGraph) { // We only care about Functions that are roots, not types. // A type would be a root if a function exists with that type, but no-one // indirect calls the type. @@ -317,12 +321,21 @@ void propagateEffects(const Module& module, } // Assign each function's effects to its CC effects. - for (Function* f : ccFuncs) { - if (!ccEffects) { - funcInfos.at(f).effects = UnknownEffects; - } else { - funcInfos.at(f).effects.emplace(*ccEffects); - } + for (auto node : cc) { + std::visit(overloaded{[&](HeapType type) { + if (ccEffects != UnknownEffects) { + typeEffects[type] = + std::make_shared(*ccEffects); + } + }, + [&](Function* f) { + if (!ccEffects) { + funcInfos.at(f).effects = UnknownEffects; + } else { + funcInfos.at(f).effects.emplace(*ccEffects); + } + }}, + node); } } } @@ -346,7 +359,8 @@ struct GenerateGlobalEffects : public Pass { auto callGraph = buildCallGraph(*module, funcInfos, getPassOptions().closedWorld); - propagateEffects(*module, getPassOptions(), funcInfos, callGraph); + propagateEffects( + *module, getPassOptions(), funcInfos, module->typeEffects, callGraph); copyEffectsToFunctions(funcInfos); } diff --git a/src/support/utilities.h b/src/support/utilities.h index 3f40111c451..272488e18f8 100644 --- a/src/support/utilities.h +++ b/src/support/utilities.h @@ -94,6 +94,17 @@ class Fatal { #define WASM_UNREACHABLE(msg) wasm::handle_unreachable() #endif +// Helper to create an invocable with an overloaded operator(), for use with +// std::visit e.g. +// std::visit( +// overloaded{ +// [](const A& a) { ... }, +// [](const B& b) { ... }}, +// variant) +template struct overloaded : Ts... { + using Ts::operator()...; +}; + } // namespace wasm #endif // wasm_support_utilities_h diff --git a/src/wasm.h b/src/wasm.h index df0c19669d3..a14416ad222 100644 --- a/src/wasm.h +++ b/src/wasm.h @@ -2722,6 +2722,12 @@ class Module { std::unordered_map typeNames; std::unordered_map typeIndices; + // Potential effects for bodies of indirect calls to this type. + // TODO: Use Type instead of HeapType to account for nullability and + // exactness. + std::unordered_map> + typeEffects; + MixedArena allocator; private: diff --git a/test/lit/passes/global-effects-closed-world-tnh.wast b/test/lit/passes/global-effects-closed-world-tnh.wast index 4c4558f8f95..64aeab8879d 100644 --- a/test/lit/passes/global-effects-closed-world-tnh.wast +++ b/test/lit/passes/global-effects-closed-world-tnh.wast @@ -16,22 +16,9 @@ ) ;; CHECK: (func $calls-nop-via-nullable-ref (type $1) (param $ref (ref null $nopType)) - ;; CHECK-NEXT: (call_ref $nopType - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) (func $calls-nop-via-nullable-ref (param $ref (ref null $nopType)) (call_ref $nopType (i32.const 1) (local.get $ref)) ) - - ;; CHECK: (func $f (type $1) (param $ref (ref null $nopType)) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref null $nopType)) - ;; The only possible implementation of $nopType has no effects. - ;; $calls-nop-via-nullable-ref may trap from a null reference, but - ;; --traps-never-happen is enabled, so we're free to optimize this out. - (call $calls-nop-via-nullable-ref (local.get $ref)) - ) ) diff --git a/test/lit/passes/global-effects-closed-world.wast b/test/lit/passes/global-effects-closed-world.wast index 77484c63d6d..e521bcee807 100644 --- a/test/lit/passes/global-effects-closed-world.wast +++ b/test/lit/passes/global-effects-closed-world.wast @@ -17,18 +17,10 @@ ) ;; CHECK: (func $calls-nop-via-ref (type $1) (param $ref (ref $nopType)) - ;; CHECK-NEXT: (call_ref $nopType - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) (func $calls-nop-via-ref (param $ref (ref $nopType)) ;; This can only possibly be a nop in closed-world. - ;; Ideally vacuum could optimize this out but we don't have a way to share - ;; this information with other passes today. - ;; For now, we can at least annotate that the call to this function in $f - ;; has no effects. - ;; TODO: This call_ref could be marked as having no effects, like the call below. (call_ref $nopType (i32.const 1) (local.get $ref)) ) @@ -41,27 +33,6 @@ (func $calls-nop-via-nullable-ref (param $ref (ref null $nopType)) (call_ref $nopType (i32.const 1) (local.get $ref)) ) - - - ;; CHECK: (func $f (type $1) (param $ref (ref $nopType)) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref $nopType)) - ;; $calls-nop-via-ref has no effects because we determined that it can only - ;; call $nop. We can optimize this call out. - (call $calls-nop-via-ref (local.get $ref)) - ) - - ;; CHECK: (func $g (type $2) (param $ref (ref null $nopType)) - ;; CHECK-NEXT: (call $calls-nop-via-nullable-ref - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $g (param $ref (ref null $nopType)) - ;; Similar to $f, but we may still trap here because the ref is null, so we - ;; don't optimize. - (call $calls-nop-via-nullable-ref (local.get $ref)) - ) ) ;; Same as the above but with call_indirect @@ -79,29 +50,11 @@ ) ;; CHECK: (func $calls-nop-via-ref (type $1) - ;; CHECK-NEXT: (call_indirect $0 (type $nopType) - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (i32.const 0) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) (func $calls-nop-via-ref - ;; This can only possibly be a nop in closed-world. - ;; Ideally vacuum could optimize this out but we don't have a way to share - ;; this information with other passes today. - ;; For now, we can at least annotate that the call to this function in $f - ;; has no effects. - ;; TODO: This call_ref could be marked as having no effects, like the call below. (call_indirect (type $nopType) (i32.const 1) (i32.const 0)) ) - - ;; CHECK: (func $f (type $1) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - (func $f - ;; $calls-nop-via-ref has no effects because we determined that it can only - ;; call $nop. We can optimize this call out. - (call $calls-nop-via-ref) - ) ) (module @@ -129,18 +82,9 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $calls-effectful-function-via-ref (param $ref (ref $maybe-has-effects)) - (call_ref $maybe-has-effects (i32.const 1) (local.get $ref)) - ) - - ;; CHECK: (func $f (type $1) (param $ref (ref $maybe-has-effects)) - ;; CHECK-NEXT: (call $calls-effectful-function-via-ref - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref $maybe-has-effects)) - ;; This may be a nop or it may trap depending on the ref. + ;; This may be a nop or it may trap depending on the ref ;; We don't know so don't optimize it out. - (call $calls-effectful-function-via-ref (local.get $ref)) + (call_ref $maybe-has-effects (i32.const 1) (local.get $ref)) ) ) @@ -172,16 +116,9 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $calls-effectful-function-via-ref - (call_indirect (type $maybe-has-effects) (i32.const 1) (i32.const 1)) - ) - - ;; CHECK: (func $f (type $1) - ;; CHECK-NEXT: (call $calls-effectful-function-via-ref) - ;; CHECK-NEXT: ) - (func $f ;; This may be a nop or it may trap depending on the ref. ;; We don't know so don't optimize it out. - (call $calls-effectful-function-via-ref) + (call_indirect (type $maybe-has-effects) (i32.const 1) (i32.const 1)) ) ) @@ -190,13 +127,12 @@ (type $uninhabited (func (param i32))) ;; CHECK: (func $calls-uninhabited (type $1) (param $ref (ref $uninhabited)) - ;; CHECK-NEXT: (call_ref $uninhabited - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) (func $calls-uninhabited (param $ref (ref $uninhabited)) - ;; It's impossible to create a ref to call this function with. + ;; There's no function with this type, so it's impossible to create a ref to + ;; call this function with and there are no effects to aggregate. + ;; Remove this call. ;; TODO: Optimize this to (unreachable). (call_ref $uninhabited (i32.const 1) (local.get $ref)) ) @@ -212,28 +148,6 @@ ;; TODO: Optimize this to (unreachable). (call_ref $uninhabited (i32.const 1) (local.get $ref)) ) - - - ;; CHECK: (func $f (type $1) (param $ref (ref $uninhabited)) - ;; CHECK-NEXT: (nop) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref $uninhabited)) - ;; There's no function with this type, so it's impossible to create a ref to - ;; call this function with and there are no effects to aggregate. - ;; Remove this call. - (call $calls-uninhabited (local.get $ref)) - ) - - ;; CHECK: (func $g (type $2) (param $ref (ref null $uninhabited)) - ;; CHECK-NEXT: (call $calls-nullable-uninhabited - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $g (param $ref (ref null $uninhabited)) - ;; Similar to above but we have a nullable reference, so we may trap and - ;; can't optimize the call out. - (call $calls-nullable-uninhabited (local.get $ref)) - ) ) (module @@ -256,7 +170,7 @@ (unreachable) ) - ;; CHECK: (func $calls-ref-with-supertype (type $1) (param $func (ref $super)) + ;; CHECK: (func $calls-ref-with-supertype (type $2) (param $func (ref $super)) ;; CHECK-NEXT: (call_ref $super ;; CHECK-NEXT: (local.get $func) ;; CHECK-NEXT: ) @@ -265,7 +179,7 @@ (call_ref $super (local.get $func)) ) - ;; CHECK: (func $calls-ref-with-exact-supertype (type $2) (param $func (ref (exact $super))) + ;; CHECK: (func $calls-ref-with-exact-supertype (type $3) (param $func (ref (exact $super))) ;; CHECK-NEXT: (call_ref $super ;; CHECK-NEXT: (local.get $func) ;; CHECK-NEXT: ) @@ -273,32 +187,6 @@ (func $calls-ref-with-exact-supertype (param $func (ref (exact $super))) (call_ref $super (local.get $func)) ) - - ;; CHECK: (func $f (type $1) (param $func (ref $super)) - ;; CHECK-NEXT: (call $calls-ref-with-supertype - ;; CHECK-NEXT: (local.get $func) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $f (param $func (ref $super)) - ;; Check that we account for subtyping correctly. - ;; $super has no effects (i.e. the union of all effects of functions with - ;; this type is empty). However, $sub does have effects, and we can call_ref - ;; with that subtype, so we need to include the unreachable effect and we - ;; can't optimize out this call. - (call $calls-ref-with-supertype (local.get $func)) - ) - - ;; CHECK: (func $g (type $2) (param $func (ref (exact $super))) - ;; CHECK-NEXT: (call $calls-ref-with-exact-supertype - ;; CHECK-NEXT: (local.get $func) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $g (param $func (ref (exact $super))) - ;; Same as above but this time our reference is the exact supertype - ;; so we know not to aggregate effects from the subtype. - ;; TODO: this case doesn't optimize today. Add exact ref support in the pass. - (call $calls-ref-with-exact-supertype (local.get $func)) - ) ) (module @@ -325,21 +213,11 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $calls-type-with-effects-but-not-addressable (param $ref (ref $only-has-effects-in-not-addressable-function)) - (call_ref $only-has-effects-in-not-addressable-function (i32.const 1) (local.get $ref)) - ) - - ;; CHECK: (func $f (type $1) (param $ref (ref $only-has-effects-in-not-addressable-function)) - ;; CHECK-NEXT: (call $calls-type-with-effects-but-not-addressable - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref $only-has-effects-in-not-addressable-function)) ;; The type $has-effects-but-not-exported doesn't have an address because ;; it's not exported and it's never the target of a ref.func. - ;; We should be able to determine that $ref can only point to $nop. - ;; TODO: Only aggregate effects from functions that are addressed. - (call $calls-type-with-effects-but-not-addressable (local.get $ref)) - ) + ;; So the call_ref has no potential targets and thus no effects. + (call_ref $only-has-effects-in-not-addressable-function (i32.const 1) (local.get $ref)) + ) ) (module @@ -406,18 +284,9 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $indirect-calls (param $ref (ref $t)) - (call_ref $t (i32.const 1) (local.get $ref)) - ) - - ;; CHECK: (func $f (type $1) (param $ref (ref $t)) - ;; CHECK-NEXT: (call $indirect-calls - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) - ;; CHECK-NEXT: ) - (func $f (param $ref (ref $t)) ;; $indirect-calls might end up calling an imported function, ;; so we don't know anything about effects here - (call $indirect-calls (local.get $ref)) + (call_ref $t (i32.const 1) (local.get $ref)) ) ) @@ -435,15 +304,8 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $calls-unreachable (export "calls-unreachable") - (call_ref $t (unreachable)) - ) - - ;; CHECK: (func $f (type $0) - ;; CHECK-NEXT: (call $calls-unreachable) - ;; CHECK-NEXT: ) - (func $f ;; $t looks like it has no effects, but unreachable is passed in, ;; so preserve the trap. - (call $calls-unreachable) + (call_ref $t (unreachable)) ) ) diff --git a/test/lit/passes/global-effects-indirect-merge.wast b/test/lit/passes/global-effects-indirect-merge.wast new file mode 100644 index 00000000000..ab087b23628 --- /dev/null +++ b/test/lit/passes/global-effects-indirect-merge.wast @@ -0,0 +1,44 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited. +;; RUN: foreach %s %t wasm-opt --all-features --closed-world --generate-global-effects --minimize-rec-groups --vacuum --ignore-implicit-traps -S -o - | filecheck %s + +(module + (rec (type $A (func (result i32)))) + (rec (type $B (func (result i32)))) + (table 2 2 funcref) + (elem (i32.const 0) $pure_A) + (elem (i32.const 1) $pure_B) + + ;; CHECK: (func $pure_A (type $A) (result i32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + (func $pure_A (type $A) + (unreachable) + ) + ;; CHECK: (func $pure_B (type $A) (result i32) + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + (func $pure_B (type $B) + (i32.const 0) + ) + + ;; CHECK: (func $test (type $1) (param $idx i32) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call_indirect $0 (type $A) + ;; CHECK-NEXT: (local.get $idx) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (call_indirect $0 (type $A) + ;; CHECK-NEXT: (local.get $idx) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (export "test") (param $idx i32) + (drop + (call_indirect (type $B) (local.get $idx)) + ) + (drop + (call_indirect (type $A) (local.get $idx)) + ) + ) +)