From 54d8e739e7f384e3ec65c2cb6b81c7bc56f6c5aa Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 14:43:11 -0700 Subject: [PATCH 01/11] fix --- src/ir/type-updating.cpp | 2 +- src/ir/type-updating.h | 19 +++++++++++-------- src/passes/SignaturePruning.cpp | 11 +++++++++-- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 12a8c7c3621..c9139105e0f 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -28,7 +28,7 @@ namespace wasm { GlobalTypeRewriter::GlobalTypeRewriter(Module& wasm) : wasm(wasm) {} -void GlobalTypeRewriter::update() { mapTypes(rebuildTypes()); } +void GlobalTypeRewriter::update(const std::vector& additionalPrivateTypes) { mapTypes(rebuildTypes(additionalPrivateTypes)); } GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( const std::vector& additionalPrivateTypes) { diff --git a/src/ir/type-updating.h b/src/ir/type-updating.h index 1626b40bdf6..c44832d36df 100644 --- a/src/ir/type-updating.h +++ b/src/ir/type-updating.h @@ -352,7 +352,12 @@ class GlobalTypeRewriter { // Main entry point. This performs the entire process of creating new heap // types and calling the hooks below, then applies the new types throughout // the module. - void update(); + // + // This only operates on private types (so as not to modify the module's + // external ABI). It takes as a parameter a list of public types to consider + // private, which allows more flexibility (e.g. in closed world if a pass + // knows a type is safe to modify despite being public, it can add it). + void update(const std::vector& additionalPrivateTypes = {}); using TypeMap = std::unordered_map; @@ -398,7 +403,7 @@ class GlobalTypeRewriter { // Helper for the repeating pattern of just updating Signature types using a // map of old heap type => new Signature. - static void updateSignatures(const SignatureUpdates& updates, Module& wasm) { + static void updateSignatures(const SignatureUpdates& updates, Module& wasm, const std::vector& additionalPrivateTypes = {}) { if (updates.empty()) { return; } @@ -407,9 +412,9 @@ class GlobalTypeRewriter { const SignatureUpdates& updates; public: - SignatureRewriter(Module& wasm, const SignatureUpdates& updates) + SignatureRewriter(Module& wasm, const SignatureUpdates& updates, const std::vector& additionalPrivateTypes) : GlobalTypeRewriter(wasm), updates(updates) { - update(); + update(additionalPrivateTypes); } void modifySignature(HeapType oldSignatureType, Signature& sig) override { @@ -419,7 +424,7 @@ class GlobalTypeRewriter { sig.results = getTempType(iter->second.results); } } - } rewriter(wasm, updates); + } rewriter(wasm, updates, additionalPrivateTypes); } protected: @@ -427,9 +432,7 @@ class GlobalTypeRewriter { // returns a map from the old types to the modified types. Used internally in // update(). // - // This only operates on private types (so as not to modify the module's - // external ABI). It takes as a parameter a list of public types to consider - // private, which allows more flexibility. + // See above regarding private types. TypeMap rebuildTypes(const std::vector& additionalPrivateTypes = {}); diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 2e4be89e895..7d3eb0f599b 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -291,8 +291,15 @@ struct SignaturePruning : public Pass { } } - // Rewrite the types. - GlobalTypeRewriter::updateSignatures(newSignatures, *module); + // Rewrite the types. We pass in all the types we intend to modify as being + // "additional private types" because we have proven above that they are + // safe to modify, even if they are technically public (e.g. they may be in + // a singleton big rec group that is public because one member is public). + std::vector additionalPrivateTypes; + for (auto& [type, sig] : newSignatures) { + additionalPrivateTypes.push_back(type); + } + GlobalTypeRewriter::updateSignatures(newSignatures, *module, additionalPrivateTypes); if (callTargetsToLocalize.empty()) { return false; From 64878972f26c069ffef3ea508cab8c87bf4d4142 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 14:57:10 -0700 Subject: [PATCH 02/11] hmm --- src/ir/type-updating.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index c9139105e0f..3fdcf0d3841 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -39,9 +39,22 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( // come before their subtypes. Index i = 0; auto privateTypes = ModuleUtils::getPrivateHeapTypes(wasm); + std::unordered_set privateTypesSet(privateTypes.begin(), privateTypes.end()); + +for (auto t : privateTypes) { +std::cout << "P: " << t << " and " << privateTypesSet.count(t) << '\n'; +} +for (auto t : additionalPrivateTypes) { +std::cout << "ADDp: " << t << " and " << privateTypesSet.count(t) << '\n'; +} for (auto t : additionalPrivateTypes) { - privateTypes.push_back(t); + // Only add additional private types that are not already in the list. + if (!privateTypesSet.count(t)) { +std::cout << "actually add " << t << " and " << privateTypesSet.count(t) << '\n'; + privateTypes.push_back(t); + privateTypesSet.insert(t); + } } // Topological sort to have supertypes first, but we have to account for the @@ -145,6 +158,7 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( for (auto& [old, new_] : oldToNewTypes) { if (auto it = wasm.typeNames.find(old); it != wasm.typeNames.end()) { wasm.typeNames[new_] = it->second; + wasm.typeNames[old].name = "old"; // XXX not quite right yet } } From 9da9900fb3b4798d765224ea24591890a0b759e8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 15:43:54 -0700 Subject: [PATCH 03/11] fix --- src/ir/type-updating.cpp | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 3fdcf0d3841..9608fe7d959 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -41,17 +41,9 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( auto privateTypes = ModuleUtils::getPrivateHeapTypes(wasm); std::unordered_set privateTypesSet(privateTypes.begin(), privateTypes.end()); -for (auto t : privateTypes) { -std::cout << "P: " << t << " and " << privateTypesSet.count(t) << '\n'; -} -for (auto t : additionalPrivateTypes) { -std::cout << "ADDp: " << t << " and " << privateTypesSet.count(t) << '\n'; -} - for (auto t : additionalPrivateTypes) { // Only add additional private types that are not already in the list. if (!privateTypesSet.count(t)) { -std::cout << "actually add " << t << " and " << privateTypesSet.count(t) << '\n'; privateTypes.push_back(t); privateTypesSet.insert(t); } @@ -158,7 +150,10 @@ std::cout << "actually add " << t << " and " << privateTypesSet.count(t) << '\n' for (auto& [old, new_] : oldToNewTypes) { if (auto it = wasm.typeNames.find(old); it != wasm.typeNames.end()) { wasm.typeNames[new_] = it->second; - wasm.typeNames[old].name = "old"; // XXX not quite right yet + // Erase the old name. This avoids the same name appearing twice if for + // some reason the old type is still in use somehow, which can be + // confusing during debugging. + wasm.typeNames.erase(old); } } From cba4a85a1c46c747037e1bb94bdd14a49d4a64e4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 15:58:35 -0700 Subject: [PATCH 04/11] close --- src/ir/type-updating.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 9608fe7d959..4d57a0aa80e 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -43,10 +43,10 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( for (auto t : additionalPrivateTypes) { // Only add additional private types that are not already in the list. - if (!privateTypesSet.count(t)) { +// if (!privateTypesSet.count(t)) { privateTypes.push_back(t); - privateTypesSet.insert(t); - } +// privateTypesSet.insert(t); +// } } // Topological sort to have supertypes first, but we have to account for the @@ -150,10 +150,6 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( for (auto& [old, new_] : oldToNewTypes) { if (auto it = wasm.typeNames.find(old); it != wasm.typeNames.end()) { wasm.typeNames[new_] = it->second; - // Erase the old name. This avoids the same name appearing twice if for - // some reason the old type is still in use somehow, which can be - // confusing during debugging. - wasm.typeNames.erase(old); } } From 1c72e1e84d6357bce0f38ef2fed783022cb93db7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 16:17:03 -0700 Subject: [PATCH 05/11] work --- src/ir/type-updating.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 4d57a0aa80e..d0f585d6b5f 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -43,10 +43,10 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( for (auto t : additionalPrivateTypes) { // Only add additional private types that are not already in the list. -// if (!privateTypesSet.count(t)) { + if (!privateTypesSet.count(t)) { privateTypes.push_back(t); -// privateTypesSet.insert(t); -// } + privateTypesSet.insert(t); + } } // Topological sort to have supertypes first, but we have to account for the From fceb73a538b2e1bf484ead08565abfdf9191aa65 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 16:47:06 -0700 Subject: [PATCH 06/11] fix --- src/passes/SignaturePruning.cpp | 10 +++--- test/lit/passes/signature-pruning.wast | 47 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 7d3eb0f599b..609a9a17bc7 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -45,11 +45,6 @@ namespace wasm { namespace { struct SignaturePruning : public Pass { - // Maps each heap type to the possible pruned heap type. We will fill this - // during analysis and then use it while doing an update of the types. If a - // type has no improvement that we can find, it will not appear in this map. - std::unordered_map newSignatures; - void run(Module* module) override { if (!module->features.hasGC()) { return; @@ -182,6 +177,11 @@ struct SignaturePruning : public Pass { // types with subtyping relations at once. SubTypes subTypes(*module); + // Maps each heap type to the possible pruned heap type. We will fill this + // during analysis and then use it while doing an update of the types. If a + // type has no improvement that we can find, it will not appear in this map. + std::unordered_map newSignatures; + // Find parameters to prune. // // TODO: The order matters here, and more than one cycle can find more work diff --git a/test/lit/passes/signature-pruning.wast b/test/lit/passes/signature-pruning.wast index cad9af82b7a..89f67a45c7d 100644 --- a/test/lit/passes/signature-pruning.wast +++ b/test/lit/passes/signature-pruning.wast @@ -1151,3 +1151,50 @@ ) ) ) + +;; $exported is exported. The entire rec group becomes exported as well, which +;; causes $unused-param's type to be public, which means we cannot normally +;; modify it. However, in closed world we allow such changes, and we can remove +;; the unused param there. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $much (func)) + + ;; CHECK: (type $1 (func)) + + ;; CHECK: (rec + ;; CHECK-NEXT: (type $none (func)) + (type $none (func)) + ;; CHECK: (type $much (func (param i32))) + (type $much (func (param i32))) + ) + + ;; CHECK: (export "exported" (func $exported)) + + ;; CHECK: (func $exported (type $none) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + (func $exported (export "exported") (type $none) + ) + + ;; CHECK: (func $unused-param (type $much) + ;; CHECK-NEXT: (local $0 i32) + ;; CHECK-NEXT: (local.set $0 + ;; CHECK-NEXT: (i32.const 0) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (nop) + ;; CHECK-NEXT: ) + (func $unused-param (type $much) (param $param i32) + ) + + ;; CHECK: (func $caller (type $1) + ;; CHECK-NEXT: (call $unused-param) + ;; CHECK-NEXT: ) + (func $caller + (call $unused-param + (i32.const 0) + ) + ) +) + From 5f8258663a21e2515b52156b9f634cd0db3d90d7 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 16:47:16 -0700 Subject: [PATCH 07/11] format --- .clang-format | 3 --- src/ir/type-updating.cpp | 8 ++++++-- src/ir/type-updating.h | 9 +++++++-- src/passes/SignaturePruning.cpp | 3 ++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.clang-format b/.clang-format index 316f76d89fc..cedadcbd0e6 100644 --- a/.clang-format +++ b/.clang-format @@ -12,6 +12,3 @@ BinPackParameters: false Language: JavaScript DisableFormat: true --- -Language: Json -BasedOnStyle: LLVM ---- diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index d0f585d6b5f..55951eac6c9 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -28,7 +28,10 @@ namespace wasm { GlobalTypeRewriter::GlobalTypeRewriter(Module& wasm) : wasm(wasm) {} -void GlobalTypeRewriter::update(const std::vector& additionalPrivateTypes) { mapTypes(rebuildTypes(additionalPrivateTypes)); } +void GlobalTypeRewriter::update( + const std::vector& additionalPrivateTypes) { + mapTypes(rebuildTypes(additionalPrivateTypes)); +} GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( const std::vector& additionalPrivateTypes) { @@ -39,7 +42,8 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( // come before their subtypes. Index i = 0; auto privateTypes = ModuleUtils::getPrivateHeapTypes(wasm); - std::unordered_set privateTypesSet(privateTypes.begin(), privateTypes.end()); + std::unordered_set privateTypesSet(privateTypes.begin(), + privateTypes.end()); for (auto t : additionalPrivateTypes) { // Only add additional private types that are not already in the list. diff --git a/src/ir/type-updating.h b/src/ir/type-updating.h index c44832d36df..60b92e58531 100644 --- a/src/ir/type-updating.h +++ b/src/ir/type-updating.h @@ -403,7 +403,10 @@ class GlobalTypeRewriter { // Helper for the repeating pattern of just updating Signature types using a // map of old heap type => new Signature. - static void updateSignatures(const SignatureUpdates& updates, Module& wasm, const std::vector& additionalPrivateTypes = {}) { + static void + updateSignatures(const SignatureUpdates& updates, + Module& wasm, + const std::vector& additionalPrivateTypes = {}) { if (updates.empty()) { return; } @@ -412,7 +415,9 @@ class GlobalTypeRewriter { const SignatureUpdates& updates; public: - SignatureRewriter(Module& wasm, const SignatureUpdates& updates, const std::vector& additionalPrivateTypes) + SignatureRewriter(Module& wasm, + const SignatureUpdates& updates, + const std::vector& additionalPrivateTypes) : GlobalTypeRewriter(wasm), updates(updates) { update(additionalPrivateTypes); } diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 609a9a17bc7..92b99468cf8 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -299,7 +299,8 @@ struct SignaturePruning : public Pass { for (auto& [type, sig] : newSignatures) { additionalPrivateTypes.push_back(type); } - GlobalTypeRewriter::updateSignatures(newSignatures, *module, additionalPrivateTypes); + GlobalTypeRewriter::updateSignatures( + newSignatures, *module, additionalPrivateTypes); if (callTargetsToLocalize.empty()) { return false; From a4904ed47fc40bcc896a1dbd70a9ddafbda181a5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 16:49:22 -0700 Subject: [PATCH 08/11] comment --- test/lit/passes/signature-pruning.wast | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/signature-pruning.wast b/test/lit/passes/signature-pruning.wast index 89f67a45c7d..c273d1588ba 100644 --- a/test/lit/passes/signature-pruning.wast +++ b/test/lit/passes/signature-pruning.wast @@ -1155,7 +1155,9 @@ ;; $exported is exported. The entire rec group becomes exported as well, which ;; causes $unused-param's type to be public, which means we cannot normally ;; modify it. However, in closed world we allow such changes, and we can remove -;; the unused param there. +;; the unused param there. What happens is that we keep the original public rec +;; group as-is, and add a new rec group for private types, put the pruned type +;; there, and use that pruned type on $unused-param. (module (rec ;; CHECK: (rec From e3fb49c145da38c179a53c45698d4dae7b7645db Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 28 May 2024 16:50:17 -0700 Subject: [PATCH 09/11] wat --- .clang-format | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.clang-format b/.clang-format index cedadcbd0e6..316f76d89fc 100644 --- a/.clang-format +++ b/.clang-format @@ -12,3 +12,6 @@ BinPackParameters: false Language: JavaScript DisableFormat: true --- +Language: Json +BasedOnStyle: LLVM +--- From 56900e6febee93315e68e63a7b34f75ad2b21c40 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 29 May 2024 10:39:31 -0700 Subject: [PATCH 10/11] fastr --- src/ir/type-updating.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/ir/type-updating.cpp b/src/ir/type-updating.cpp index 55951eac6c9..760d71b9c20 100644 --- a/src/ir/type-updating.cpp +++ b/src/ir/type-updating.cpp @@ -42,14 +42,17 @@ GlobalTypeRewriter::TypeMap GlobalTypeRewriter::rebuildTypes( // come before their subtypes. Index i = 0; auto privateTypes = ModuleUtils::getPrivateHeapTypes(wasm); - std::unordered_set privateTypesSet(privateTypes.begin(), - privateTypes.end()); - for (auto t : additionalPrivateTypes) { + if (!additionalPrivateTypes.empty()) { // Only add additional private types that are not already in the list. - if (!privateTypesSet.count(t)) { - privateTypes.push_back(t); - privateTypesSet.insert(t); + std::unordered_set privateTypesSet(privateTypes.begin(), + privateTypes.end()); + + for (auto t : additionalPrivateTypes) { + if (!privateTypesSet.count(t)) { + privateTypes.push_back(t); + privateTypesSet.insert(t); + } } } From 2213028eb2f9a01a9eef47ac8bf582097d23029e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 29 May 2024 13:45:23 -0700 Subject: [PATCH 11/11] Update src/passes/SignaturePruning.cpp Co-authored-by: Thomas Lively --- src/passes/SignaturePruning.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/SignaturePruning.cpp b/src/passes/SignaturePruning.cpp index 92b99468cf8..4480c18321e 100644 --- a/src/passes/SignaturePruning.cpp +++ b/src/passes/SignaturePruning.cpp @@ -177,7 +177,7 @@ struct SignaturePruning : public Pass { // types with subtyping relations at once. SubTypes subTypes(*module); - // Maps each heap type to the possible pruned heap type. We will fill this + // Maps each heap type to the possible pruned signature. We will fill this // during analysis and then use it while doing an update of the types. If a // type has no improvement that we can find, it will not appear in this map. std::unordered_map newSignatures;