From b1437507fc09091638387444e96b0e26a2e2d3cc Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Aug 2024 16:41:16 -0700 Subject: [PATCH 1/9] start --- src/passes/param-utils.cpp | 41 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 0caccff1168..8b49d7d5c70 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -14,10 +14,10 @@ * limitations under the License. */ +#include "cfg/liveness-traversal.h" #include "passes/param-utils.h" #include "ir/eh-utils.h" #include "ir/function-utils.h" -#include "ir/local-graph.h" #include "ir/localize.h" #include "ir/possible-constant.h" #include "ir/type-updating.h" @@ -29,21 +29,36 @@ namespace wasm::ParamUtils { std::unordered_set getUsedParams(Function* func) { - LocalGraph localGraph(func); + // To find which params are used, compute liveness at the entry. + struct ParamLiveness : public LivenessWalker> { + auto Super = LivenessWalker>; + + // Ignore non-params. + static void doVisitLocalGet(SubType* self, Expression** currp) { + auto* get = (*currp)->cast(); + if (self->getFunction()->isParam(get->index)) { + Super::doVisitLocalGet(self, currp); + } + } + static void doVisitLocalSet(SubType* self, Expression** currp) { + auto* set = (*currp)->cast(); + if (self->getFunction()->isParam(set->index)) { + Super::doVisitLocalSet(self, currp); + } + } + } walker; + walker.walkFunction(func); - std::unordered_set usedParams; + if (!walker.entry) { + // Empty function. + return {}; + } - for (auto& [get, sets] : localGraph.getSetses) { - if (!func->isParam(get->index)) { - continue; - } + auto& livenessAtEntry = walker.entry->contents.start; - for (auto* set : sets) { - // A nullptr value indicates there is no LocalSet* that sets the value, - // so it must be the parameter value. - if (!set) { - usedParams.insert(get->index); - } + for (Index i = 0; i < func->getNumParams(); i++) { + if (livenessAtEntry.count(i)) { + usedParams.insert(get->index); } } From b2a20e68fa4fdeb4dfbc68d55dca8c00886c9396 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 21 Aug 2024 16:42:57 -0700 Subject: [PATCH 2/9] start --- src/passes/param-utils.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 8b49d7d5c70..7800629bf83 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -31,16 +31,16 @@ namespace wasm::ParamUtils { std::unordered_set getUsedParams(Function* func) { // To find which params are used, compute liveness at the entry. struct ParamLiveness : public LivenessWalker> { - auto Super = LivenessWalker>; + using Super = LivenessWalker>; // Ignore non-params. - static void doVisitLocalGet(SubType* self, Expression** currp) { + static void doVisitLocalGet(ParamLiveness* self, Expression** currp) { auto* get = (*currp)->cast(); if (self->getFunction()->isParam(get->index)) { Super::doVisitLocalGet(self, currp); } } - static void doVisitLocalSet(SubType* self, Expression** currp) { + static void doVisitLocalSet(ParamLiveness* self, Expression** currp) { auto* set = (*currp)->cast(); if (self->getFunction()->isParam(set->index)) { Super::doVisitLocalSet(self, currp); @@ -55,10 +55,11 @@ std::unordered_set getUsedParams(Function* func) { } auto& livenessAtEntry = walker.entry->contents.start; + std::unordered_set usedParams; for (Index i = 0; i < func->getNumParams(); i++) { - if (livenessAtEntry.count(i)) { - usedParams.insert(get->index); + if (livenessAtEntry.has(i)) { + usedParams.insert(i); } } From be2b20c3e9444784b54536d1201a74370a73e189 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 07:47:03 -0700 Subject: [PATCH 3/9] work --- src/passes/param-utils.cpp | 12 +++++------- test/lit/passes/dae-gc.wast | 13 +++---------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 7800629bf83..ece0c44505e 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -54,15 +54,13 @@ std::unordered_set getUsedParams(Function* func) { return {}; } - auto& livenessAtEntry = walker.entry->contents.start; + // We now have a sorted vector of the live params at the entry. Convert that + // to a set. + auto& sortedLiveness = walker.entry->contents.start; std::unordered_set usedParams; - - for (Index i = 0; i < func->getNumParams(); i++) { - if (livenessAtEntry.has(i)) { - usedParams.insert(i); - } + for (auto live : sortedLiveness) { + usedParams.insert(live); } - return usedParams; } diff --git a/test/lit/passes/dae-gc.wast b/test/lit/passes/dae-gc.wast index 9878230207f..ab36fd99d23 100644 --- a/test/lit/passes/dae-gc.wast +++ b/test/lit/passes/dae-gc.wast @@ -24,25 +24,18 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (local.tee $0 - ;; CHECK-NEXT: (unreachable) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: ) (func $bar (param $0 i31ref) (drop - ;; after the parameter is removed, we create a nullable local to replace it, - ;; and must update the tee's type accordingly to avoid a validation error, - ;; and also add a ref.as_non_null so that the outside still receives the - ;; same type as before + ;; After the parameter is removed, we create a nullable local to replace it. (local.tee $0 (ref.i31 (i32.const 2) ) ) ) - ;; test for an unreachable tee, whose type must be unreachable even after - ;; the change (the tee would need to be dropped if it were not unreachable, - ;; so the correctness in this case is visible in the output) + ;; Test we do not error on an unreachable tee (we can simply remove it). (local.tee $0 (unreachable) ) From c928db0ffe384a5e8ea8bd337aea957b0c58cbb1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 08:37:37 -0700 Subject: [PATCH 4/9] fix --- src/passes/param-utils.cpp | 6 +++--- test/lit/passes/dae-gc.wast | 4 +++- test/lit/passes/dae_all-features.wast | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index ece0c44505e..2adf8fe0a9f 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -33,16 +33,16 @@ std::unordered_set getUsedParams(Function* func) { struct ParamLiveness : public LivenessWalker> { using Super = LivenessWalker>; - // Ignore non-params. + // Ignore unreachable code and non-params. static void doVisitLocalGet(ParamLiveness* self, Expression** currp) { auto* get = (*currp)->cast(); - if (self->getFunction()->isParam(get->index)) { + if (self->currBasicBlock && self->getFunction()->isParam(get->index)) { Super::doVisitLocalGet(self, currp); } } static void doVisitLocalSet(ParamLiveness* self, Expression** currp) { auto* set = (*currp)->cast(); - if (self->getFunction()->isParam(set->index)) { + if (self->currBasicBlock && self->getFunction()->isParam(set->index)) { Super::doVisitLocalSet(self, currp); } } diff --git a/test/lit/passes/dae-gc.wast b/test/lit/passes/dae-gc.wast index ab36fd99d23..a95bb0377c5 100644 --- a/test/lit/passes/dae-gc.wast +++ b/test/lit/passes/dae-gc.wast @@ -24,7 +24,9 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (local.tee $0 + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $bar (param $0 i31ref) (drop diff --git a/test/lit/passes/dae_all-features.wast b/test/lit/passes/dae_all-features.wast index 17ea77942f5..98e245ca351 100644 --- a/test/lit/passes/dae_all-features.wast +++ b/test/lit/passes/dae_all-features.wast @@ -940,3 +940,21 @@ (unreachable) ) ) + +(module + ;; CHECK: (type $0 (func (param f64 f32))) + + ;; CHECK: (func $0 (type $0) (param $0 f64) (param $1 f32) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (local.get $0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $0 (param $0 f64) (param $1 f32) + ;; Both parameters here are unused: there is a get, but it is unreachable. + (unreachable) + (drop + (local.get $0) + ) + ) +) From 5dfe2285239bc63504bd3de71939cc69fbc7a17f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 08:38:51 -0700 Subject: [PATCH 5/9] fix --- test/lit/passes/dae-gc.wast | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/dae-gc.wast b/test/lit/passes/dae-gc.wast index a95bb0377c5..9878230207f 100644 --- a/test/lit/passes/dae-gc.wast +++ b/test/lit/passes/dae-gc.wast @@ -30,14 +30,19 @@ ;; CHECK-NEXT: ) (func $bar (param $0 i31ref) (drop - ;; After the parameter is removed, we create a nullable local to replace it. + ;; after the parameter is removed, we create a nullable local to replace it, + ;; and must update the tee's type accordingly to avoid a validation error, + ;; and also add a ref.as_non_null so that the outside still receives the + ;; same type as before (local.tee $0 (ref.i31 (i32.const 2) ) ) ) - ;; Test we do not error on an unreachable tee (we can simply remove it). + ;; test for an unreachable tee, whose type must be unreachable even after + ;; the change (the tee would need to be dropped if it were not unreachable, + ;; so the correctness in this case is visible in the output) (local.tee $0 (unreachable) ) From 17c6f55d2b5675b8d0907eea40f7494cc0fe9c44 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 08:42:46 -0700 Subject: [PATCH 6/9] test --- src/passes/param-utils.cpp | 2 +- test/lit/passes/dae_all-features.wast | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 2adf8fe0a9f..20d8e915e21 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -50,7 +50,7 @@ std::unordered_set getUsedParams(Function* func) { walker.walkFunction(func); if (!walker.entry) { - // Empty function. + // Empty function: nothing is used. return {}; } diff --git a/test/lit/passes/dae_all-features.wast b/test/lit/passes/dae_all-features.wast index 98e245ca351..da9558dd8e0 100644 --- a/test/lit/passes/dae_all-features.wast +++ b/test/lit/passes/dae_all-features.wast @@ -942,19 +942,32 @@ ) (module - ;; CHECK: (type $0 (func (param f64 f32))) + ;; CHECK: (type $0 (func)) + + ;; CHECK: (type $1 (func (param i32))) - ;; CHECK: (func $0 (type $0) (param $0 f64) (param $1 f32) + ;; CHECK: (func $target (type $0) + ;; CHECK-NEXT: (local $0 i32) ;; CHECK-NEXT: (unreachable) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (local.get $0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $0 (param $0 f64) (param $1 f32) - ;; Both parameters here are unused: there is a get, but it is unreachable. + (func $target (param $0 i32) + ;; The parameter here is unused: there is a get, but it is unreachable. We can + ;; remove the parameter here, and in the caller below. (unreachable) (drop (local.get $0) ) ) + + ;; CHECK: (func $caller (type $1) (param $x i32) + ;; CHECK-NEXT: (call $target) + ;; CHECK-NEXT: ) + (func $caller (param $x i32) + (call $target + (local.get $x) + ) + ) ) From 85912582dd6653d9605918ed4a7a39760cbef1a5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 08:44:40 -0700 Subject: [PATCH 7/9] format --- src/passes/param-utils.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 20d8e915e21..02d9ead818b 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -14,8 +14,8 @@ * limitations under the License. */ -#include "cfg/liveness-traversal.h" #include "passes/param-utils.h" +#include "cfg/liveness-traversal.h" #include "ir/eh-utils.h" #include "ir/function-utils.h" #include "ir/localize.h" @@ -30,7 +30,8 @@ namespace wasm::ParamUtils { std::unordered_set getUsedParams(Function* func) { // To find which params are used, compute liveness at the entry. - struct ParamLiveness : public LivenessWalker> { + struct ParamLiveness + : public LivenessWalker> { using Super = LivenessWalker>; // Ignore unreachable code and non-params. From c9842a09964a225ab5cb5e65369a84fea495b83c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 08:48:30 -0700 Subject: [PATCH 8/9] todo --- src/passes/param-utils.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index 02d9ead818b..e483c622721 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -30,6 +30,11 @@ namespace wasm::ParamUtils { std::unordered_set getUsedParams(Function* func) { // To find which params are used, compute liveness at the entry. + // TODO: We could write bespoke code here rather than reuse LivenessWalker, as + // we only need liveness at the entry. The code below computes it for + // the param indexes in the entire function. However, there are usually + // very few params (compared to locals, which we ignore here), so this + // may be fast enough, and is very simple. struct ParamLiveness : public LivenessWalker> { using Super = LivenessWalker>; From 480d0fb0293d8b03b476a8bd5b67a0bc4c13f05b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 22 Aug 2024 15:56:01 -0700 Subject: [PATCH 9/9] module and ignoreBranchesOutsideOfFunc --- src/passes/DeadArgumentElimination.cpp | 2 +- src/passes/SignaturePruning.cpp | 2 +- src/passes/param-utils.cpp | 7 ++++++- src/passes/param-utils.h | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/passes/DeadArgumentElimination.cpp b/src/passes/DeadArgumentElimination.cpp index 83cd7e86d07..99a70965475 100644 --- a/src/passes/DeadArgumentElimination.cpp +++ b/src/passes/DeadArgumentElimination.cpp @@ -158,7 +158,7 @@ struct DAEScanner // part of, say if we are exported, or if another parallel function finds a // RefFunc to us and updates it before we check it). if (numParams > 0 && !info->hasUnseenCalls) { - auto usedParams = ParamUtils::getUsedParams(func); + auto usedParams = ParamUtils::getUsedParams(func, getModule()); for (Index i = 0; i < numParams; i++) { if (usedParams.count(i) == 0) { info->unusedParams.insert(i); diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 4480c18321e..a9fb4d23aa4 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -96,7 +96,7 @@ struct SignaturePruning : public Pass { info.calls = std::move(FindAll(func->body).list); info.callRefs = std::move(FindAll(func->body).list); - info.usedParams = ParamUtils::getUsedParams(func); + info.usedParams = ParamUtils::getUsedParams(func, module); }); // A map of types to all the information combined over all the functions diff --git a/src/passes/param-utils.cpp b/src/passes/param-utils.cpp index e483c622721..f54f91bd9b6 100644 --- a/src/passes/param-utils.cpp +++ b/src/passes/param-utils.cpp @@ -28,7 +28,7 @@ namespace wasm::ParamUtils { -std::unordered_set getUsedParams(Function* func) { +std::unordered_set getUsedParams(Function* func, Module* module) { // To find which params are used, compute liveness at the entry. // TODO: We could write bespoke code here rather than reuse LivenessWalker, as // we only need liveness at the entry. The code below computes it for @@ -39,6 +39,10 @@ std::unordered_set getUsedParams(Function* func) { : public LivenessWalker> { using Super = LivenessWalker>; + // Branches outside of the function can be ignored, as we only look at + // locals, which vanish when we leave. + bool ignoreBranchesOutsideOfFunc = true; + // Ignore unreachable code and non-params. static void doVisitLocalGet(ParamLiveness* self, Expression** currp) { auto* get = (*currp)->cast(); @@ -53,6 +57,7 @@ std::unordered_set getUsedParams(Function* func) { } } } walker; + walker.setModule(module); walker.walkFunction(func); if (!walker.entry) { diff --git a/src/passes/param-utils.h b/src/passes/param-utils.h index 4c458390afa..35e5d9f808a 100644 --- a/src/passes/param-utils.h +++ b/src/passes/param-utils.h @@ -42,7 +42,7 @@ namespace wasm::ParamUtils { // function foo(x) { // bar(x); // read of a param value // } -std::unordered_set getUsedParams(Function* func); +std::unordered_set getUsedParams(Function* func, Module* module); // The outcome of an attempt to remove a parameter(s). enum RemovalOutcome {