[wasm-split] Remove unnecessary global exports

src/ir/module-splitting.cpp

@@ -77,8 +77,6 @@
 #include "ir/find_all.h"
 #include "ir/module-utils.h"
 #include "ir/names.h"
-#include "support/insert_ordered.h"
-#include "support/unique_deferring_queue.h"
 #include "wasm-builder.h"
 #include "wasm.h"
 
@@ -755,13 +753,6 @@ ModuleSplitter::PrimarySecondaryUsedNames ModuleSplitter::computeUsedNames() {
       }
     }
 
-    for (auto name : used.tables) {
-      if (auto* table = primary.getTableOrNull(name)) {
-        if (table->init) {
-          collector.walk(table->init);
-        }
-      }
-    }
     return used;
   };
 
@@ -821,32 +812,60 @@ ModuleSplitter::PrimarySecondaryUsedNames ModuleSplitter::computeUsedNames() {
     }
   }
 
-  // Compute the transitive closure of globals referenced in other globals'
-  // initializers. Since globals can reference other globals, we must ensure
-  // that if a global is used in a module, all its dependencies are also marked
-  // as used.
-  auto computeTransitiveGlobals = [&](UsedNames& used) {
-    UniqueNonrepeatingDeferredQueue<Name> worklist;
-    for (auto global : used.globals) {
-      worklist.push(global);
+  // Given a name and a module item kind (field pointer), find which module
+  // "owns" it. If it is used by exactly one secondary module, that secondary
+  // module is the owner. If it is used by the primary module or multiple
+  // secondary modules, the primary module is the owner. If it is not used,
+  // returns nullptr.
+  auto getOwner = [&](Name name, auto UsedNames::* field) -> UsedNames* {
+    UsedNames* owner = nullptr;
+    if ((primaryUsed.*field).contains(name)) {
+      owner = &primaryUsed;
     }
-    while (!worklist.empty()) {
-      Name name = worklist.pop();
-      // At this point all globals are still in the primary module, so this
-      // exists
-      auto* global = primary.getGlobal(name);
-      if (!global->imported() && global->init) {
-        for (auto* get : FindAll<GlobalGet>(global->init).list) {
-          worklist.push(get->name);
-          used.globals.insert(get->name);
+    for (auto& sec : secondaryUsed) {
+      if ((sec.*field).contains(name)) {
+        if (owner) {
+          owner = &primaryUsed;
+          break;
         }
+        owner = &sec;
       }
     }
+    return owner;
   };
 
-  computeTransitiveGlobals(primaryUsed);
-  for (auto& used : secondaryUsed) {
-    computeTransitiveGlobals(used);
+  // Scan table initializers into their owning modules. If a table is used by a
+  // single secondary module, its initializer dependencies are marked as "used"
+  // in that secondary module. Otherwise, they are marked as used in the primary
+  // module.
+  if (primary.features.hasGC()) {
+    for (auto& table : primary.tables) {
+      if (!table->init) {
+        continue;
+      }
+      if (UsedNames* owner = getOwner(table->name, &UsedNames::tables)) {
+        NameCollector(*owner).walk(table->init);
+      }
+    }
+  }
+
+  // Compute the transitive closure of globals referenced in other globals'
+  // initializers. WebAssembly validation requires that global initializers only
+  // refer to previously defined globals. Therefore, `primary.globals` is
+  // guaranteed to be topologically sorted with respect to its internal
+  // dependencies. By iterating in reverse, we are guaranteed to process
+  // dependent globals before the globals they depend on, allowing us to
+  // propagate ownership in a single pass.
+  for (auto it = primary.globals.rbegin(); it != primary.globals.rend(); ++it) {
+    auto& global = *it;
+    if (!global->init) {
+      continue;
+    }
+    if (UsedNames* owner = getOwner(global->name, &UsedNames::globals)) {
+      for (auto* get : FindAll<GlobalGet>(global->init).list) {
+        owner->globals.insert(get->name);
+      }
+    }
   }
 
   return std::make_pair(primaryUsed, secondaryUsed);

test/lit/wasm-split/table-init-deps.wast

@@ -0,0 +1,53 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: wasm-split %s -all -g -o1 %t.1.wasm -o2 %t.2.wasm --keep-funcs=keep
+;; RUN: wasm-dis %t.1.wasm | filecheck %s --check-prefix PRIMARY
+;; RUN: wasm-dis %t.2.wasm | filecheck %s --check-prefix SECONDARY
+
+(module
+  ;; PRIMARY:      (type $0 (func))
+
+  ;; PRIMARY:      (import "env" "g" (global $g funcref))
+  (import "env" "g" (global $g funcref))
+
+  ;; A table used by both $keep and $split.
+  ;; The table stays in PRIMARY. $g should stay in PRIMARY and NOT be
+  ;; exported/imported.
+  ;; PRIMARY:      (table $t 1 1 funcref (global.get $g))
+  (table $t 1 1 funcref (global.get $g))
+
+  ;; PRIMARY:      (export "table" (table $t))
+
+  ;; PRIMARY:      (func $keep
+  ;; PRIMARY-NEXT:  (drop
+  ;; PRIMARY-NEXT:   (table.get $t
+  ;; PRIMARY-NEXT:    (i32.const 0)
+  ;; PRIMARY-NEXT:   )
+  ;; PRIMARY-NEXT:  )
+  ;; PRIMARY-NEXT: )
+  (func $keep
+    (drop
+      (table.get $t
+        (i32.const 0)
+      )
+    )
+  )
+
+  ;; SECONDARY:      (type $0 (func))
+
+  ;; SECONDARY:      (import "primary" "table" (table $t 1 1 funcref))
+
+  ;; SECONDARY:      (func $split
+  ;; SECONDARY-NEXT:  (drop
+  ;; SECONDARY-NEXT:   (table.get $t
+  ;; SECONDARY-NEXT:    (i32.const 0)
+  ;; SECONDARY-NEXT:   )
+  ;; SECONDARY-NEXT:  )
+  ;; SECONDARY-NEXT: )
+  (func $split
+    (drop
+      (table.get $t
+        (i32.const 0)
+      )
+    )
+  )
+)

test/lit/wasm-split/transitive-globals-multi.wast

@@ -0,0 +1,52 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: wasm-split -all -g --multi-split %s --manifest %s.manifest --out-prefix=%t -o %t.wasm
+;; RUN: wasm-dis %t.wasm | filecheck %s --check-prefix PRIMARY
+;; RUN: wasm-dis %t1.wasm | filecheck %s --check-prefix SECONDARY1
+;; RUN: wasm-dis %t2.wasm | filecheck %s --check-prefix SECONDARY2
+
+;; Because global $e is used in both module1 ($split1) and module2 ($split2), $e
+;; will be exported / imported, but we don't need to export $f.
+
+(module
+  ;; PRIMARY:      (type $0 (func))
+
+  ;; PRIMARY:      (global $f i32 (i32.const 42))
+  (global $f i32 (i32.const 42))
+  ;; PRIMARY:      (global $e i32 (global.get $f))
+  (global $e i32 (global.get $f))
+
+  ;; PRIMARY:      (export "global" (global $e))
+
+  ;; PRIMARY:      (func $keep
+  ;; PRIMARY-NEXT:  (nop)
+  ;; PRIMARY-NEXT: )
+  (func $keep
+    (nop)
+  )
+
+  ;; SECONDARY1:      (type $0 (func))
+
+  ;; SECONDARY1:      (import "primary" "global" (global $e i32))
+
+  ;; SECONDARY1:      (func $split1
+  ;; SECONDARY1-NEXT:  (drop
+  ;; SECONDARY1-NEXT:   (global.get $e)
+  ;; SECONDARY1-NEXT:  )
+  ;; SECONDARY1-NEXT: )
+  (func $split1
+    (drop (global.get $e))
+  )
+
+  ;; SECONDARY2:      (type $0 (func))
+
+  ;; SECONDARY2:      (import "primary" "global" (global $e i32))
+
+  ;; SECONDARY2:      (func $split2
+  ;; SECONDARY2-NEXT:  (drop
+  ;; SECONDARY2-NEXT:   (global.get $e)
+  ;; SECONDARY2-NEXT:  )
+  ;; SECONDARY2-NEXT: )
+  (func $split2
+    (drop (global.get $e))
+  )
+)

test/lit/wasm-split/transitive-globals-multi.wast.manifest

@@ -0,0 +1,5 @@
+1:
+split1
+
+2:
+split2

test/lit/wasm-split/transitive-globals.wast

@@ -22,11 +22,9 @@
   ;; PRIMARY:      (global $f i32 (i32.const 42))
   ;; PRIMARY:      (global $e i32 (global.get $f))
 
-  ;; PRIMARY:      (export "global" (global $f))
-  ;; PRIMARY:      (export "global_1" (global $e))
+  ;; PRIMARY:      (export "global" (global $e))
 
-  ;; SECONDARY:      (import "primary" "global" (global $f i32))
-  ;; SECONDARY:      (import "primary" "global_1" (global $e i32))
+  ;; SECONDARY:      (import "primary" "global" (global $e i32))
 
   ;; SECONDARY:      (global $c i32 (i32.const 42))
   ;; SECONDARY:      (global $b i32 (global.get $c))

test/lit/wasm-split/transitive-globals2.wast

@@ -0,0 +1,46 @@
+;; RUN: wasm-split %s -all -g -o1 %t.1.wasm -o2 %t.2.wasm --split-funcs=split
+;; RUN: wasm-dis -all %t.1.wasm | filecheck %s --check-prefix PRIMARY
+;; RUN: wasm-dis -all %t.2.wasm | filecheck %s --check-prefix SECONDARY
+
+;; The dependence chain is $g4->$g3->$g2->$g1, and because $g4 is used in the
+;; primary module, all four globals should end up in the primary module. Only
+;; $g2 needs to be exported to the secondary module, not $g1.
+
+(module
+  (global $g1 i32 (i32.const 42))
+  (global $g2 i32 (global.get $g1))
+  (global $g3 i32 (global.get $g2))
+  (global $g4 i32 (global.get $g3))
+
+  (func $keep
+    (drop (global.get $g4))
+  )
+
+  (func $split
+    (drop (global.get $g2))
+  )
+)
+
+;; PRIMARY:      (module
+;; PRIMARY-NEXT:  (type $0 (func))
+;; PRIMARY-NEXT:  (global $g1 i32 (i32.const 42))
+;; PRIMARY-NEXT:  (global $g2 i32 (global.get $g1))
+;; PRIMARY-NEXT:  (global $g3 i32 (global.get $g2))
+;; PRIMARY-NEXT:  (global $g4 i32 (global.get $g3))
+;; PRIMARY-NEXT:  (export "global" (global $g2))
+;; PRIMARY-NEXT:  (func $keep (type $0)
+;; PRIMARY-NEXT:   (drop
+;; PRIMARY-NEXT:    (global.get $g4)
+;; PRIMARY-NEXT:   )
+;; PRIMARY-NEXT:  )
+;; PRIMARY-NEXT: )
+
+;; SECONDARY:      (module
+;; SECONDARY-NEXT:  (type $0 (func))
+;; SECONDARY-NEXT:  (import "primary" "global" (global $g2 i32))
+;; SECONDARY-NEXT:  (func $split (type $0)
+;; SECONDARY-NEXT:   (drop
+;; SECONDARY-NEXT:    (global.get $g2)
+;; SECONDARY-NEXT:   )
+;; SECONDARY-NEXT:  )
+;; SECONDARY-NEXT: )