From e5ba9c2a0ded524c8857f0a89c0073bd05cb2d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vouillon?= Date: Wed, 3 Apr 2024 11:10:43 +0200 Subject: [PATCH] wasm-merge: preserve explicit names - only write explicit function names - when merging modules, the name of types, globals and tags in all modules but the first were lost --- src/ir/module-splitting.cpp | 7 +++--- src/ir/module-utils.cpp | 10 +++++++- src/wasm/wasm-binary.cpp | 32 +++++++++++++++---------- src/wasm/wasm-s-parser.cpp | 1 + test/lit/merge/names.wat | 20 ++++++++-------- test/passes/func-metrics.txt | 4 ++-- test/unit/input/gc_target_feature.wasm | Bin 94 -> 91 bytes 7 files changed, 46 insertions(+), 28 deletions(-) diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp index 3d876bf7a6d..8ef0e7b8c5c 100644 --- a/src/ir/module-splitting.cpp +++ b/src/ir/module-splitting.cpp @@ -395,8 +395,9 @@ void ModuleSplitter::exportImportFunction(Name funcName) { // Import the function if it is not already imported into the secondary // module. if (secondary.getFunctionOrNull(funcName) == nullptr) { - auto func = - Builder::makeFunction(funcName, primary.getFunction(funcName)->type, {}); + auto primaryFunc = primary.getFunction(funcName); + auto func = Builder::makeFunction(funcName, primaryFunc->type, {}); + func->hasExplicitName = primaryFunc->hasExplicitName; func->module = config.importNamespace; func->base = exportName; secondary.addFunction(std::move(func)); @@ -542,7 +543,7 @@ void ModuleSplitter::setupTablePatching() { placeholder->base = std::to_string(index); placeholder->name = Names::getValidFunctionName( primary, std::string("placeholder_") + placeholder->base.toString()); - placeholder->hasExplicitName = false; + placeholder->hasExplicitName = true; placeholder->type = secondaryFunc->type; elem = placeholder->name; primary.addFunction(std::move(placeholder)); diff --git a/src/ir/module-utils.cpp b/src/ir/module-utils.cpp index fdee4f86995..9858c98acb3 100644 --- a/src/ir/module-utils.cpp +++ b/src/ir/module-utils.cpp @@ -48,6 +48,7 @@ Function* copyFunction(Function* func, std::optional> fileIndexMap) { auto ret = std::make_unique(); ret->name = newName.is() ? newName : func->name; + ret->hasExplicitName = func->hasExplicitName && !newName.is(); ret->type = func->type; ret->vars = func->vars; ret->localNames = func->localNames; @@ -77,6 +78,7 @@ Function* copyFunction(Function* func, Global* copyGlobal(Global* global, Module& out) { auto* ret = new Global(); ret->name = global->name; + ret->hasExplicitName = global->hasExplicitName; ret->type = global->type; ret->mutable_ = global->mutable_; ret->module = global->module; @@ -93,6 +95,7 @@ Global* copyGlobal(Global* global, Module& out) { Tag* copyTag(Tag* tag, Module& out) { auto* ret = new Tag(); ret->name = tag->name; + ret->hasExplicitName = tag->hasExplicitName; ret->sig = tag->sig; ret->module = tag->module; ret->base = tag->base; @@ -209,6 +212,12 @@ void copyModuleItems(const Module& in, Module& out) { for (auto& curr : in.dataSegments) { copyDataSegment(curr.get(), out); } + + for (auto& [type, names] : in.typeNames) { + if (!out.typeNames.count(type)) { + out.typeNames[type] = names; + } + } } void copyModule(const Module& in, Module& out) { @@ -222,7 +231,6 @@ void copyModule(const Module& in, Module& out) { out.customSections = in.customSections; out.debugInfoFileNames = in.debugInfoFileNames; out.features = in.features; - out.typeNames = in.typeNames; } void clearModule(Module& wasm) { diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 28a630c4a2b..cb05688e6cb 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -864,19 +864,27 @@ void WasmBinaryWriter::writeNames() { // function names { - auto substart = - startSubsection(BinaryConsts::CustomSections::Subsection::NameFunction); - o << U32LEB(indexes.functionIndexes.size()); - Index emitted = 0; - auto add = [&](Function* curr) { - o << U32LEB(emitted); - writeEscapedName(curr->name.str); - emitted++; + std::vector> functionsWithNames; + Index checked = 0; + auto check = [&](Function* curr) { + if (curr->hasExplicitName) { + functionsWithNames.push_back({checked, curr}); + } + checked++; }; - ModuleUtils::iterImportedFunctions(*wasm, add); - ModuleUtils::iterDefinedFunctions(*wasm, add); - assert(emitted == indexes.functionIndexes.size()); - finishSubsection(substart); + ModuleUtils::iterImportedFunctions(*wasm, check); + ModuleUtils::iterDefinedFunctions(*wasm, check); + assert(checked == indexes.functionIndexes.size()); + if (functionsWithNames.size() > 0) { + auto substart = + startSubsection(BinaryConsts::CustomSections::Subsection::NameFunction); + o << U32LEB(functionsWithNames.size()); + for (auto& [index, global] : functionsWithNames) { + o << U32LEB(index); + writeEscapedName(global->name.str); + } + finishSubsection(substart); + } } // local names diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp index c7cedfdbd67..73ffd755da4 100644 --- a/src/wasm/wasm-s-parser.cpp +++ b/src/wasm/wasm-s-parser.cpp @@ -1181,6 +1181,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) { // make a new function currFunction = std::unique_ptr( Builder(wasm).makeFunction(name, std::move(params), type, std::move(vars))); + currFunction->hasExplicitName = hasExplicitName; currFunction->profile = profile; // parse body diff --git a/test/lit/merge/names.wat b/test/lit/merge/names.wat index d9f4cf3d8b2..24b1dab0d99 100644 --- a/test/lit/merge/names.wat +++ b/test/lit/merge/names.wat @@ -8,13 +8,13 @@ ;; CHECK: (type $2 (func (param (ref $t)))) - ;; CHECK: (type $3 (struct (field i64) (field i32))) + ;; CHECK: (type $u (struct (field $c i64) (field $d i32))) - ;; CHECK: (type $4 (func (param (ref $3)))) + ;; CHECK: (type $4 (func (param (ref $u)))) ;; CHECK: (global $global$0 i32 (i32.const 0)) - ;; CHECK: (global $global$1 i32 (i32.const 0)) + ;; CHECK: (global $glob2 i32 (i32.const 0)) ;; CHECK: (global $global$2 i32 (i32.const 0)) @@ -40,7 +40,7 @@ ;; CHECK: (tag $tag$1) - ;; CHECK: (tag $tag$2) + ;; CHECK: (tag $tag2) ;; CHECK: (tag $tag$3) @@ -72,21 +72,21 @@ ;; CHECK: (export "f2" (func $func2)) - ;; CHECK: (export "f3" (func $1_3)) + ;; CHECK: (export "f3" (func $4)) ;; CHECK: (export "t2" (table $table2)) ;; CHECK: (export "t3" (table $3)) - ;; CHECK: (export "g2" (global $global$1)) + ;; CHECK: (export "g2" (global $glob2)) ;; CHECK: (export "g3" (global $global$0)) - ;; CHECK: (export "tag2" (tag $tag$2)) + ;; CHECK: (export "tag2" (tag $tag2)) ;; CHECK: (export "tag3" (tag $tag$3)) - ;; CHECK: (export "func2" (func $2_3)) + ;; CHECK: (export "func2" (func $5)) ;; CHECK: (func $func0 (type $0) ;; CHECK-NEXT: (nop) @@ -128,10 +128,10 @@ ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) -;; CHECK: (func $1_3 (type $0) +;; CHECK: (func $4 (type $0) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) -;; CHECK: (func $2_3 (type $4) (param $0 (ref $3)) +;; CHECK: (func $5 (type $4) (param $0 (ref $u)) ;; CHECK-NEXT: (nop) ;; CHECK-NEXT: ) diff --git a/test/passes/func-metrics.txt b/test/passes/func-metrics.txt index 69b89557c32..8921b6977b4 100644 --- a/test/passes/func-metrics.txt +++ b/test/passes/func-metrics.txt @@ -244,7 +244,7 @@ func: func_a Block : 1 Call : 5 start: func_a - [removable-bytes-without-it]: 57 + [removable-bytes-without-it]: 60 [total] : 0 (module (type $0 (func)) @@ -274,7 +274,7 @@ func: 0 [vars] : 0 GlobalGet : 1 export: stackSave (0) - [removable-bytes-without-it]: 62 + [removable-bytes-without-it]: 79 [total] : 0 (module (type $0 (func (result i32))) diff --git a/test/unit/input/gc_target_feature.wasm b/test/unit/input/gc_target_feature.wasm index bf2ecf9348b83c2f08c01d43e1c5637dde47389a..8fab7a31ab12489722fe2eea6d7e82ebe66d3bcd 100644 GIT binary patch delta 14 Vcma!xo*>31!;+Vnn>tb69snK)1Qq}Q delta 17 Ycma!!n;^z1&ytszo65+@Fj2-H04K@>8vp