diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index 720752372aa..525e0c87e52 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -44,6 +44,53 @@ struct FuncInfo { std::unordered_set indirectCalledTypes; }; +// Only funcs that are 'addressed' may be the target of an indirect call. A +// function is addressed if: +// - It appears in a ref.func expression +// - It appears in an `elem` segment (note that we already ignore `elem declare` +// statements in our IR, but we check separately for funcs that appear in +// `ref.func`). +// - It's exported, because it may flow back to us as a reference. +// - It's imported, which implies it can be addressed (see +// https://github.com/WebAssembly/spec/issues/2072). +// +// If a function doesn't meet any of these criteria, it can't be the target of +// an indirect call and we don't need to include its effects in indirect calls. +std::unordered_set getAddressedFuncs(Module& module) { + struct AddressedFuncsWalker : WalkerPass> { + std::unordered_set& addressedFuncs; + + AddressedFuncsWalker(std::unordered_set& addressedFuncs) + : addressedFuncs(addressedFuncs) {} + + bool isFunctionParallel() override { return true; } + + void visitRefFunc(RefFunc* refFunc) { + addressedFuncs.insert(getModule()->getFunction(refFunc->func)); + } + }; + + std::unordered_set addressedFuncs; + AddressedFuncsWalker walker(addressedFuncs); + walker.walkModuleCode(&module); + walker.walkModule(&module); + + ModuleUtils::iterImportedFunctions( + module, [&addressedFuncs, &module](Function* import) { + addressedFuncs.insert(module.getFunction(import->name)); + }); + + for (const auto& export_ : module.exports) { + if (export_->kind != ExternalKind::Function) { + continue; + } + + addressedFuncs.insert(module.getFunction(*export_->getInternalName())); + } + + return addressedFuncs; +} + std::map analyzeFuncs(Module& module, const PassOptions& passOptions) { ModuleUtils::ParallelFunctionAnalysis analysis( @@ -144,6 +191,7 @@ using CallGraph = CallGraph buildCallGraph(const Module& module, const std::map& funcInfos, + const std::unordered_set& addressedFuncs, WorldMode worldMode) { CallGraph callGraph; if (worldMode == WorldMode::Open) { @@ -179,16 +227,18 @@ CallGraph buildCallGraph(const Module& module, } // Type -> Function - callGraph[caller->type.getHeapType()].insert(caller); + if (addressedFuncs.contains(caller)) { + callGraph[caller->type.getHeapType()].insert(caller); + } } // Type -> Type - // Do a DFS up the type heirarchy for all function implementations. + // Do a DFS up the type hierarchy for all function implementations. // We are essentially walking up each supertype chain and adding edges from // super -> subtype, but doing it via DFS to avoid repeated work. Graph superTypeGraph(allFunctionTypes.begin(), allFunctionTypes.end(), - [&callGraph](auto&& push, HeapType t) { + [&callGraph](const auto& push, HeapType t) { // Not needed except that during lookup we expect the // key to exist. callGraph[t]; @@ -350,8 +400,10 @@ struct GenerateGlobalEffects : public Pass { std::map funcInfos = analyzeFuncs(*module, getPassOptions()); - auto callGraph = - buildCallGraph(*module, funcInfos, getPassOptions().worldMode); + auto addressedFuncs = getAddressedFuncs(*module); + + auto callGraph = buildCallGraph( + *module, funcInfos, addressedFuncs, getPassOptions().worldMode); propagateEffects(*module, getPassOptions(), diff --git a/test/lit/passes/global-effects-closed-world-simplify-locals.wast b/test/lit/passes/global-effects-closed-world-simplify-locals.wast index 534e6476172..0def74034c8 100644 --- a/test/lit/passes/global-effects-closed-world-simplify-locals.wast +++ b/test/lit/passes/global-effects-closed-world-simplify-locals.wast @@ -4,6 +4,8 @@ ;; Tests for aggregating effects from indirect calls in GlobalEffects when ;; --closed-world is true. Continued from global-effects-closed-world.wast. +;; Test that effects are aggregated from both $indirect-type-super and +;; $indirect-type-sub for indirect calls to $indirect-type-super. (module ;; CHECK: (type $indirect-type-super (sub (func (param i32)))) (type $indirect-type-super (sub (func (param i32)))) @@ -13,6 +15,8 @@ ;; CHECK: (type $2 (func (param (ref $indirect-type-super)))) + ;; CHECK: (type $3 (func (param (ref $indirect-type-sub)))) + ;; CHECK: (global $g1 (mut i32) (i32.const 0)) (global $g1 (mut i32) (i32.const 0)) ;; CHECK: (global $g2 (mut i32) (i32.const 0)) @@ -42,7 +46,7 @@ (global.set $g2 (local.get $i32)) ) - ;; CHECK: (func $merges-multiple-effects (type $2) (param $ref (ref $indirect-type-super)) + ;; CHECK: (func $merges-effects-from-super-and-sub (type $2) (param $ref (ref $indirect-type-super)) ;; CHECK-NEXT: (local $x i32) ;; CHECK-NEXT: (local $y i32) ;; CHECK-NEXT: (local $z i32) @@ -67,7 +71,7 @@ ;; CHECK-NEXT: (global.get $g3) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $merges-multiple-effects (param $ref (ref $indirect-type-super)) + (func $merges-effects-from-super-and-sub (param $ref (ref $indirect-type-super)) (local $x i32) (local $y i32) (local $z i32) @@ -85,4 +89,222 @@ (drop (local.get $y)) (drop (local.get $z)) ) + + ;; CHECK: (func $merges-effects-from-sub-only (type $3) (param $ref (ref $indirect-type-sub)) + ;; CHECK-NEXT: (local $x i32) + ;; CHECK-NEXT: (local $y i32) + ;; CHECK-NEXT: (local $z i32) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (local.set $y + ;; CHECK-NEXT: (global.get $g2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call_ref $indirect-type-sub + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.get $g1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $y) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.get $g3) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $merges-effects-from-sub-only (param $ref (ref $indirect-type-sub)) + (local $x i32) + (local $y i32) + (local $z i32) + + (local.set $x (global.get $g1)) + (local.set $y (global.get $g2)) + (local.set $z (global.get $g3)) + + ;; Similar to above but here it's impossible to reach $impl1 + ;; (the supertype), so $x can safely be optimized out. + (call_ref $indirect-type-sub (i32.const 1) (local.get $ref)) + + (drop (local.get $x)) + (drop (local.get $y)) + (drop (local.get $z)) + ) +) + +;; Test different ways of referencing functions to ensure that they're included +;; in indirect effects analysis. A function is considered 'addressed' if it's: +;; - imported (tested in the next test) +;; - exported +;; - referenced in a ref.func +;; - contained in an `elem` segment +;; Imported functions are tested in the next module to avoid +;; confounding this test because imports are assumed to have all possible +;; effects. +(module + ;; CHECK: (type $indirect-type (func (param i32))) + (type $indirect-type (func (param i32))) + + ;; CHECK: (type $1 (func)) + + ;; CHECK: (type $2 (func (param (ref $indirect-type)))) + + ;; CHECK: (global $g1 (mut i32) (i32.const 0)) + (global $g1 (mut i32) (i32.const 0)) + ;; CHECK: (global $g2 (mut i32) (i32.const 0)) + (global $g2 (mut i32) (i32.const 0)) + ;; CHECK: (global $g3 (mut i32) (i32.const 0)) + (global $g3 (mut i32) (i32.const 0)) + ;; CHECK: (global $g4 (mut i32) (i32.const 0)) + (global $g4 (mut i32) (i32.const 0)) + + (table 1 1 funcref) + + ;; CHECK: (table $0 1 1 funcref) + + ;; CHECK: (elem $0 (i32.const 0) $f3) + + ;; CHECK: (elem declare func $f2) + + ;; CHECK: (export "f1" (func $f1)) + + ;; CHECK: (func $f1 (type $indirect-type) (param $i32 i32) + ;; CHECK-NEXT: (global.set $g1 + ;; CHECK-NEXT: (local.get $i32) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $f1 (export "f1") (type $indirect-type) (param $i32 i32) + (global.set $g1 (local.get $i32)) + ) + + ;; CHECK: (func $f2 (type $indirect-type) (param $i32 i32) + ;; CHECK-NEXT: (global.set $g2 + ;; CHECK-NEXT: (local.get $i32) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $f2 (type $indirect-type) (param $i32 i32) + (global.set $g2 (local.get $i32)) + ) + ;; CHECK: (func $reference-f2 (type $1) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.func $f2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $reference-f2 + (drop (ref.func $f2)) + ) + + ;; CHECK: (func $f3 (type $indirect-type) (param $i32 i32) + ;; CHECK-NEXT: (global.set $g3 + ;; CHECK-NEXT: (local.get $i32) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $f3 (type $indirect-type) (param $i32 i32) + (global.set $g3 (local.get $i32)) + ) + (elem (i32.const 0) $f3) + + ;; CHECK: (func $merges-multiple-effects (type $2) (param $ref (ref $indirect-type)) + ;; CHECK-NEXT: (local $l1 i32) + ;; CHECK-NEXT: (local $l2 i32) + ;; CHECK-NEXT: (local $l3 i32) + ;; CHECK-NEXT: (local $l4 i32) + ;; CHECK-NEXT: (local.set $l1 + ;; CHECK-NEXT: (global.get $g1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $l2 + ;; CHECK-NEXT: (global.get $g2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $l3 + ;; CHECK-NEXT: (global.get $g3) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: (call_ref $indirect-type + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $l1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $l2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $l3) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (global.get $g4) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $merges-multiple-effects (param $ref (ref $indirect-type)) + (local $l1 i32) + (local $l2 i32) + (local $l3 i32) + (local $l4 i32) + + (local.set $l1 (global.get $g1)) + (local.set $l2 (global.get $g2)) + (local.set $l3 (global.get $g3)) + (local.set $l4 (global.get $g4)) + + ;; This acts as a barrier for $l1, $l2, and $l3 but not $l4. + ;; $ref may write to $g1 via $f1, or $g2 via $f2, $g3 via $f3 but not $g4. + ;; $l4 is optimized out and the others are left alone. + (call_ref $indirect-type (i32.const 1) (local.get $ref)) + + (drop (local.get $l1)) + (drop (local.get $l2)) + (drop (local.get $l3)) + (drop (local.get $l4)) + ) +) + +(module + ;; CHECK: (type $indirect-type (func (param i32))) + (type $indirect-type (func (param i32))) + + ;; CHECK: (type $1 (func (param (ref $indirect-type)))) + + ;; CHECK: (import "" "" (func $imported-func (type $indirect-type) (param i32))) + (import "" "" (func $imported-func (type $indirect-type))) + + ;; CHECK: (global $g1 (mut i32) (i32.const 0)) + (global $g1 (mut i32) (i32.const 0)) + ;; CHECK: (global $g2 (mut i32) (i32.const 0)) + (global $g2 (mut i32) (i32.const 0)) + + ;; CHECK: (func $merges-multiple-effects (type $1) (param $ref (ref $indirect-type)) + ;; CHECK-NEXT: (local $l1 i32) + ;; CHECK-NEXT: (local $l2 i32) + ;; CHECK-NEXT: (local.set $l1 + ;; CHECK-NEXT: (global.get $g1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $l2 + ;; CHECK-NEXT: (global.get $g2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (call_ref $indirect-type + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $l1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $l2) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $merges-multiple-effects (param $ref (ref $indirect-type)) + (local $l1 i32) + (local $l2 i32) + + (local.set $l1 (global.get $g1)) + (local.set $l2 (global.get $g2)) + + ;; This can flow to an import, so we have to assume that $g1 and $g2 could + ;; be mutated, and nothing can be optimized. + (call_ref $indirect-type (i32.const 1) (local.get $ref)) + + (drop (local.get $l1)) + (drop (local.get $l2)) + ) ) diff --git a/test/lit/passes/global-effects-closed-world.wast b/test/lit/passes/global-effects-closed-world.wast index 7b1945fc3a0..774a72d054f 100644 --- a/test/lit/passes/global-effects-closed-world.wast +++ b/test/lit/passes/global-effects-closed-world.wast @@ -197,16 +197,12 @@ ) ;; CHECK: (func $calls-type-with-effects-but-not-addressable (type $1) (param $ref (ref $only-has-effects-in-not-addressable-function)) - ;; CHECK-NEXT: (call_ref $only-has-effects-in-not-addressable-function - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (local.get $ref) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) (func $calls-type-with-effects-but-not-addressable (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. + ;; 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. + ;; So the call_ref's only potential target is $nop which has no effects. (call_ref $only-has-effects-in-not-addressable-function (i32.const 1) (local.get $ref)) ) ) diff --git a/test/lit/passes/simplify-locals-global-effects-eh.wast b/test/lit/passes/simplify-locals-global-effects-eh.wast index f616208a9b1..2fed91f128a 100644 --- a/test/lit/passes/simplify-locals-global-effects-eh.wast +++ b/test/lit/passes/simplify-locals-global-effects-eh.wast @@ -75,21 +75,19 @@ ;; CHECK: (func $const (type $const-type) (result f32) ;; CHECK-NEXT: (f32.const 1) ;; CHECK-NEXT: ) - (func $const (type $const-type) + (func $const (export "const") (type $const-type) (f32.const 1) ) - (elem declare $const) ;; CHECK: (func $throws (type $throw-type) (result f64) ;; CHECK-NEXT: (throw $t) ;; CHECK-NEXT: (f64.const 1) ;; CHECK-NEXT: ) - (func $throws (type $throw-type) + (func $throws (export "throws") (type $throw-type) (throw $t) (f64.const 1) ) - (elem declare $throws) ;; CHECK: (func $read-g-with-nop-call-ref (type $5) (param $ref (ref null $const-type)) (result i32) ;; CHECK-NEXT: (local $x i32)