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
WebAssembly · Apr 3, 2024 · 2738190 · 2738190
1 parent 766c029
commit 2738190
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 28 deletions.
diff --git 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
@@ -48,6 +48,7 @@ Function* copyFunction(Function* func,
                        std::optional<std::vector<Index>> fileIndexMap) {
   auto ret = std::make_unique<Function>();
   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
@@ -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<std::pair<Index, Function*>> 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
@@ -1181,6 +1181,7 @@ void SExpressionWasmBuilder::parseFunction(Element& s, bool preParseImport) {
   // make a new function
   currFunction = std::unique_ptr<Function>(
     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
@@ -7,13 +7,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))
 
@@ -39,7 +39,7 @@
 
    ;; CHECK:      (tag $tag$1)
 
-   ;; CHECK:      (tag $tag$2)
+   ;; CHECK:      (tag $tag2)
 
    ;; CHECK:      (tag $tag$3)
 
@@ -71,21 +71,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
    ;; CHECK-NEXT:  (nop)
@@ -127,10 +127,10 @@
 ;; CHECK-NEXT:  (nop)
 ;; CHECK-NEXT: )
 
-;; CHECK:      (func $1_3
+;; CHECK:      (func $4
 ;; CHECK-NEXT:  (nop)
 ;; CHECK-NEXT: )
 
-;; CHECK:      (func $2_3 (param $0 (ref $3))
+;; CHECK:      (func $5 (param $0 (ref $u))
 ;; CHECK-NEXT:  (nop)
 ;; CHECK-NEXT: )
diff --git 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