From dfc82c81d133456e76b4070b1790bef121b0edec Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Sun, 19 Apr 2026 16:31:24 +0000 Subject: [PATCH 1/3] Account for addressed funcs --- src/passes/GlobalEffects.cpp | 72 ++++++++++++++++--- .../passes/global-effects-closed-world.wast | 7 +- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index 1625b551322..91d8551fe40 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -22,6 +22,7 @@ #include #include "ir/effects.h" +#include "ir/element-utils.h" #include "ir/module-utils.h" #include "pass.h" #include "support/graph_traversal.h" @@ -47,8 +48,55 @@ struct FuncInfo { std::unordered_set indirectCalledTypes; }; +std::unordered_set getAddressedFuncs(Module& module) { + struct AddressedFuncsWalker + : PostWalker> { + const Module& module; + std::unordered_set& addressedFuncs; + + AddressedFuncsWalker(const Module& module, + std::unordered_set& addressedFuncs) + : module(module), addressedFuncs(addressedFuncs) {} + + void visitExpression(Expression* curr) { + if (auto* refFunc = curr->dynCast()) { + addressedFuncs.insert(module.getFunction(refFunc->func)); + } + } + }; + + std::unordered_set addressedFuncs; + AddressedFuncsWalker walker(module, addressedFuncs); + 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; + } + + // TODO: internal or external? I think internal + // This might be why we failed to lookup the function earlier + // Maybe we can use Function* after all + addressedFuncs.insert(module.getFunction(*export_->getInternalName())); + } + + ElementUtils::iterAllElementFunctionNames( + &module, [&addressedFuncs, &module](Name func) { + addressedFuncs.insert(module.getFunction(func)); + }); + + return addressedFuncs; +} + std::map analyzeFuncs(Module& module, const PassOptions& passOptions) { + std::unordered_set addressedFuncs; ModuleUtils::ParallelFunctionAnalysis analysis( module, [&](Function* func, FuncInfo& funcInfo) { if (func->imported()) { @@ -79,10 +127,14 @@ std::map analyzeFuncs(Module& module, const PassOptions& options; FuncInfo& funcInfo; + std::unordered_set& addressedFuncs; + CallScanner(Module& wasm, const PassOptions& options, - FuncInfo& funcInfo) - : wasm(wasm), options(options), funcInfo(funcInfo) {} + FuncInfo& funcInfo, + std::unordered_set& addressedFuncs) + : wasm(wasm), options(options), funcInfo(funcInfo), + addressedFuncs(addressedFuncs) {} void visitExpression(Expression* curr) { ShallowEffectAnalyzer effects(options, wasm, curr); @@ -116,7 +168,7 @@ std::map analyzeFuncs(Module& module, } } }; - CallScanner scanner(module, passOptions, funcInfo); + CallScanner scanner(module, passOptions, funcInfo, addressedFuncs); scanner.walkFunction(func); } }); @@ -146,6 +198,7 @@ using CallGraph = CallGraph buildCallGraph(const Module& module, const std::map& funcInfos, + const std::unordered_set addressedFuncs, bool closedWorld) { CallGraph callGraph; if (!closedWorld) { @@ -181,7 +234,9 @@ 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 @@ -342,11 +397,12 @@ void copyEffectsToFunctions(const std::map& funcInfos) { struct GenerateGlobalEffects : public Pass { void run(Module* module) override { - std::map funcInfos = - analyzeFuncs(*module, getPassOptions()); + auto funcInfos = analyzeFuncs(*module, getPassOptions()); + + auto addressedFuncs = getAddressedFuncs(*module); - auto callGraph = - buildCallGraph(*module, funcInfos, getPassOptions().closedWorld); + auto callGraph = buildCallGraph( + *module, funcInfos, addressedFuncs, getPassOptions().closedWorld); propagateEffects(*module, getPassOptions(), funcInfos, callGraph); diff --git a/test/lit/passes/global-effects-closed-world.wast b/test/lit/passes/global-effects-closed-world.wast index 77484c63d6d..586b0025538 100644 --- a/test/lit/passes/global-effects-closed-world.wast +++ b/test/lit/passes/global-effects-closed-world.wast @@ -329,15 +329,12 @@ ) ;; 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: (nop) ;; 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. + ;; So the call_ref has no potential targets and thus no effects. (call $calls-type-with-effects-but-not-addressable (local.get $ref)) ) ) From f670a0da07b70aa62003d445cdce46e024b8b92c Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Mon, 20 Apr 2026 21:32:19 +0000 Subject: [PATCH 2/3] PR updates --- src/passes/GlobalEffects.cpp | 50 ++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index 91d8551fe40..c2c4a4ed722 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -48,26 +48,33 @@ 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 declare` statement (note that we already ignore `elem` + 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 is `elem declare`d. + + 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 - : PostWalker> { - const Module& module; + struct AddressedFuncsWalker : PostWalker { std::unordered_set& addressedFuncs; - AddressedFuncsWalker(const Module& module, - std::unordered_set& addressedFuncs) - : module(module), addressedFuncs(addressedFuncs) {} + AddressedFuncsWalker(std::unordered_set& addressedFuncs) + : addressedFuncs(addressedFuncs) {} - void visitExpression(Expression* curr) { - if (auto* refFunc = curr->dynCast()) { - addressedFuncs.insert(module.getFunction(refFunc->func)); - } + void visitRefFunc(RefFunc* refFunc) { + addressedFuncs.insert(getModule()->getFunction(refFunc->func)); } }; std::unordered_set addressedFuncs; - AddressedFuncsWalker walker(module, addressedFuncs); + AddressedFuncsWalker walker(addressedFuncs); walker.walkModule(&module); ModuleUtils::iterImportedFunctions( @@ -80,9 +87,6 @@ std::unordered_set getAddressedFuncs(Module& module) { continue; } - // TODO: internal or external? I think internal - // This might be why we failed to lookup the function earlier - // Maybe we can use Function* after all addressedFuncs.insert(module.getFunction(*export_->getInternalName())); } @@ -96,7 +100,6 @@ std::unordered_set getAddressedFuncs(Module& module) { std::map analyzeFuncs(Module& module, const PassOptions& passOptions) { - std::unordered_set addressedFuncs; ModuleUtils::ParallelFunctionAnalysis analysis( module, [&](Function* func, FuncInfo& funcInfo) { if (func->imported()) { @@ -127,14 +130,10 @@ std::map analyzeFuncs(Module& module, const PassOptions& options; FuncInfo& funcInfo; - std::unordered_set& addressedFuncs; - CallScanner(Module& wasm, const PassOptions& options, - FuncInfo& funcInfo, - std::unordered_set& addressedFuncs) - : wasm(wasm), options(options), funcInfo(funcInfo), - addressedFuncs(addressedFuncs) {} + FuncInfo& funcInfo) + : wasm(wasm), options(options), funcInfo(funcInfo) {} void visitExpression(Expression* curr) { ShallowEffectAnalyzer effects(options, wasm, curr); @@ -168,7 +167,7 @@ std::map analyzeFuncs(Module& module, } } }; - CallScanner scanner(module, passOptions, funcInfo, addressedFuncs); + CallScanner scanner(module, passOptions, funcInfo); scanner.walkFunction(func); } }); @@ -198,7 +197,7 @@ using CallGraph = CallGraph buildCallGraph(const Module& module, const std::map& funcInfos, - const std::unordered_set addressedFuncs, + const std::unordered_set& addressedFuncs, bool closedWorld) { CallGraph callGraph; if (!closedWorld) { @@ -397,7 +396,8 @@ void copyEffectsToFunctions(const std::map& funcInfos) { struct GenerateGlobalEffects : public Pass { void run(Module* module) override { - auto funcInfos = analyzeFuncs(*module, getPassOptions()); + std::map funcInfos = + analyzeFuncs(*module, getPassOptions()); auto addressedFuncs = getAddressedFuncs(*module); From dd8092493f52f3524b00a92978a5e617e1ccfcb8 Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Thu, 23 Apr 2026 16:15:26 +0000 Subject: [PATCH 3/3] Fix comment --- src/passes/GlobalEffects.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/passes/GlobalEffects.cpp b/src/passes/GlobalEffects.cpp index c2c4a4ed722..51c7bec59a5 100644 --- a/src/passes/GlobalEffects.cpp +++ b/src/passes/GlobalEffects.cpp @@ -52,9 +52,9 @@ struct FuncInfo { 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 declare` statement (note that we already ignore `elem` + - 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`). + `ref.func`). - It's exported, because it may flow back to us as a reference. - It's imported, which implies it is `elem declare`d.