From 25aefe4998bd6ec5ffac9b4bb44bca1898654b90 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Tue, 19 May 2026 03:35:17 +0000 Subject: [PATCH 1/3] [wasm-split] Share active table with caller modules The way we currently share the active table with secondary modules https://github.com/WebAssembly/binaryen/blob/2c2509b5bfe399df400b2185caa370f56e563d32/src/ir/module-splitting.cpp#L1122-L1156 has missed an important case in multi-split. This is one of the unexpected consequences of #8688. Here the `secondary` module in this loop is a secondary module whose functions have been put in the active table, mostly by `getTrampoline` function. When we have a call from `$A` to `$B` and `$B` is split, `getTrampoline` puts `$B` on the active table, which will be converted to a placeholder in `setupTablePatcing`. So the code above share the active table (and its base global, if exists) with `$B`'s module. The problem is we didn't share the active global with `$A`'s module. In a two-way split this is not a problem because `$A` is in the primary module. But in multi-split, `$A`'s module itself is another secondary module, and this module needs to access the active table because it should call the placeholder for `$B`. This does it in `indirectCallsToSecondaryFunctions`. --- I wish we can do the active table sharing for callee modules in `indirectCallsToSecondaryFunctions` too and get done with it, but we still need to share it in `shareImportableItems` for secondary modules that have functions to be replaced in the active table, because there is a case of existing secondary function names in the active table. See `KEEP-NONE` RUN line in the test below: https://github.com/WebAssembly/binaryen/blob/main/test/lit/wasm-split/basic.wast In this test, we reuse the existing table as the active table, which already has `$foo` in its element section. And that `foo` will be split. Because both `bar` and `foo` are split there is no cross-module call between them to process in `indirectCallsToSecondaryFunctions`. This won't be converted to trampolines because https://github.com/WebAssembly/binaryen/blob/5fcc1af12ae19032339ee5e56889e08b6912e8cb/src/ir/module-splitting.cpp#L947-L958 and because there is no trampoline, this doesn't get to have a call instruction from primary->secondary, but we still need to share the active table with the secondary module, and we can't handle it in `indirectCallsToSecondaryFunctions` --- This fixes the error discussed in https://github.com/WebAssembly/binaryen/pull/8711#discussion_r3251422502. --- src/ir/module-splitting.cpp | 81 ++++++++++++++++----------- test/lit/wasm-split/multi-split2.wast | 67 ++++++++++++++++++++++ 2 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 test/lit/wasm-split/multi-split2.wast diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp index 6b233c5e624..33f7b202b18 100644 --- a/src/ir/module-splitting.cpp +++ b/src/ir/module-splitting.cpp @@ -342,6 +342,8 @@ struct ModuleSplitter { // Map from original secondary function name to its trampoline std::unordered_map trampolineMap; + void shareActiveTable(Module* secondary); + // Initialization helpers static std::unique_ptr initSecondary(const Module& primary); static std::unordered_map @@ -590,6 +592,32 @@ Name ModuleSplitter::getTrampoline(Name funcName) { return trampoline; } +void ModuleSplitter::shareActiveTable(Module* secondary) { + assert(tableManager.activeTable); + auto secondaryTable = + secondary->getTableOrNull(tableManager.activeTable->name); + if (secondaryTable) { + // In case it's already in the secondary module, sync the initial/max + secondaryTable->initial = tableManager.activeTable->initial; + secondaryTable->max = tableManager.activeTable->max; + } else { + secondaryTable = + ModuleUtils::copyTable(tableManager.activeTable, *secondary); + makeImportExport( + *tableManager.activeTable, *secondaryTable, "table", ExternalKind::Table); + } + if (tableManager.activeBase.global) { + auto* primaryGlobal = primary.getGlobal(tableManager.activeBase.global); + auto* secondaryGlobal = + secondary->getGlobalOrNull(tableManager.activeBase.global); + if (!secondaryGlobal) { + secondaryGlobal = ModuleUtils::copyGlobal(primaryGlobal, *secondary); + makeImportExport( + *primaryGlobal, *secondaryGlobal, "global", ExternalKind::Global); + } + } +} + void ModuleSplitter::thunkExportedSecondaryFunctions() { // Update exports of secondary functions in the primary module to export // wrapper functions that indirectly call the secondary functions. We are @@ -986,9 +1014,14 @@ void ModuleSplitter::indirectReferencesToSecondaryFunctions() { void ModuleSplitter::indirectCallsToSecondaryFunctions() { // Update direct calls of secondary functions to be indirect calls of their // corresponding table indices instead. + std::set activeTableUsingSecondaries; struct CallIndirector : public PostWalker { ModuleSplitter& parent; - CallIndirector(ModuleSplitter& parent) : parent(parent) {} + std::set& activeTableUsingSecondaries; + CallIndirector(ModuleSplitter& parent, + std::set& activeTableUsingSecondaries) + : parent(parent), + activeTableUsingSecondaries(activeTableUsingSecondaries) {} void visitCall(Call* curr) { // Return if the call's target is not in one of the secondary module. if (!parent.allSecondaryFuncs.contains(curr->target)) { @@ -1014,13 +1047,23 @@ void ModuleSplitter::indirectCallsToSecondaryFunctions() { curr->operands, func->type.getHeapType(), curr->isReturn)); + + // Share the active table with the current module (caller). We share the + // active table with with calleeModule later in setupTablePathing. + if (currModule != &parent.primary) { + activeTableUsingSecondaries.insert(currModule); + } } }; - CallIndirector callIndirector(*this); + CallIndirector callIndirector(*this, activeTableUsingSecondaries); callIndirector.walkModule(&primary); for (auto& secondaryPtr : secondaries) { callIndirector.walkModule(secondaryPtr.get()); } + + for (auto* secondary : activeTableUsingSecondaries) { + shareActiveTable(secondary); + } } void ModuleSplitter::exportImportCalledPrimaryFunctions() { @@ -1120,40 +1163,10 @@ void ModuleSplitter::setupTablePatching() { for (auto& [secondaryPtr, replacedElems] : moduleToReplacedElems) { Module& secondary = *secondaryPtr; - // Import and export the active table if necessary. Unless we use an - // existing table as an active table (e.g. because reference-types is - // disabled) and that table was already being used by an existing indirect - // call, shareImportableItems wasn't able to mark it as used in secondaries, - // so we should export and import the active table here. - auto secondaryTable = - secondary.getTableOrNull(tableManager.activeTable->name); - if (secondaryTable) { - // In case it's already in the secondary module, sync the initial/max - secondaryTable->initial = tableManager.activeTable->initial; - secondaryTable->max = tableManager.activeTable->max; - } else { - secondaryTable = - ModuleUtils::copyTable(tableManager.activeTable, secondary); - makeImportExport(*tableManager.activeTable, - *secondaryTable, - "table", - ExternalKind::Table); - } + shareActiveTable(&secondary); + auto* secondaryTable = secondary.getTable(tableManager.activeTable->name); if (tableManager.activeBase.global) { - // Import and export the active table's base global if necessary. Unless - // the base global was already being used elsewhere in secondaries, - // shareImportableItems wasn't able to mark it as used in secondaries, so - // we should export and import it here. - auto* primaryGlobal = primary.getGlobal(tableManager.activeBase.global); - auto* secondaryGlobal = - secondary.getGlobalOrNull(tableManager.activeBase.global); - if (!secondaryGlobal) { - secondaryGlobal = ModuleUtils::copyGlobal(primaryGlobal, secondary); - makeImportExport( - *primaryGlobal, *secondaryGlobal, "global", ExternalKind::Global); - } - assert(tableManager.activeTableSegments.size() == 1 && "Unexpected number of segments with non-const base"); assert(secondary.tables.size() == 1 && secondary.elementSegments.empty()); diff --git a/test/lit/wasm-split/multi-split2.wast b/test/lit/wasm-split/multi-split2.wast new file mode 100644 index 00000000000..73578fd814e --- /dev/null +++ b/test/lit/wasm-split/multi-split2.wast @@ -0,0 +1,67 @@ +;; 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/multi-split.wast.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=MOD1 +;; RUN: wasm-dis %t2.wasm | filecheck %s --check-prefix=MOD2 +;; RUN: wasm-dis %t3.wasm | filecheck %s --check-prefix=MOD3 + +;; A regresion test for the case the active table was not correctly shared with +;; MOD1. Because func $A is not called by any function, MOD1 is a secondary +;; module who only acts as a caller. + +(module + ;; MOD1: (type $0 (func)) + + ;; MOD1: (import "primary" "table" (table $timport$0 2 funcref)) + + ;; MOD1: (func $A + ;; MOD1-NEXT: (call_indirect (type $0) + ;; MOD1-NEXT: (i32.const 0) + ;; MOD1-NEXT: ) + ;; MOD1-NEXT: (call_indirect (type $0) + ;; MOD1-NEXT: (i32.const 1) + ;; MOD1-NEXT: ) + ;; MOD1-NEXT: ) + (func $A + (call $B) + (call $C) + ) + + ;; MOD2: (type $0 (func)) + + ;; MOD2: (import "primary" "table" (table $timport$0 2 funcref)) + + ;; MOD2: (elem $0 (i32.const 0) $B) + + ;; MOD2: (func $B + ;; MOD2-NEXT: (call_indirect (type $0) + ;; MOD2-NEXT: (i32.const 1) + ;; MOD2-NEXT: ) + ;; MOD2-NEXT: ) + (func $B + (call $C) + ) + + ;; MOD3: (type $0 (func)) + + ;; MOD3: (import "primary" "table" (table $timport$0 2 funcref)) + + ;; MOD3: (elem $0 (i32.const 1) $C) + + ;; MOD3: (func $C + ;; MOD3-NEXT: ) + (func $C + ) +) +;; PRIMARY: (type $0 (func)) + +;; PRIMARY: (import "placeholder.2" "0" (func $placeholder_0)) + +;; PRIMARY: (import "placeholder.3" "1" (func $placeholder_1)) + +;; PRIMARY: (table $0 2 funcref) + +;; PRIMARY: (elem $0 (i32.const 0) $placeholder_0 $placeholder_1) + +;; PRIMARY: (export "table" (table $0)) From d1a6d3d1035508bfafe36ca6a3365e557489d127 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 20 May 2026 00:04:09 +0000 Subject: [PATCH 2/3] Use unordered_set for activeUsingSecondaries --- src/ir/module-splitting.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp index 33f7b202b18..1bf557f5603 100644 --- a/src/ir/module-splitting.cpp +++ b/src/ir/module-splitting.cpp @@ -1014,7 +1014,7 @@ void ModuleSplitter::indirectReferencesToSecondaryFunctions() { void ModuleSplitter::indirectCallsToSecondaryFunctions() { // Update direct calls of secondary functions to be indirect calls of their // corresponding table indices instead. - std::set activeTableUsingSecondaries; + std::unordered_set activeTableUsingSecondaries; struct CallIndirector : public PostWalker { ModuleSplitter& parent; std::set& activeTableUsingSecondaries; From 7cf5907018789ff8397902408309648a0861b853 Mon Sep 17 00:00:00 2001 From: Heejin Ahn Date: Wed, 20 May 2026 00:06:55 +0000 Subject: [PATCH 3/3] Make activeTableUsingSecondaries a class member --- src/ir/module-splitting.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp index 1bf557f5603..af31ff20b37 100644 --- a/src/ir/module-splitting.cpp +++ b/src/ir/module-splitting.cpp @@ -1014,14 +1014,10 @@ void ModuleSplitter::indirectReferencesToSecondaryFunctions() { void ModuleSplitter::indirectCallsToSecondaryFunctions() { // Update direct calls of secondary functions to be indirect calls of their // corresponding table indices instead. - std::unordered_set activeTableUsingSecondaries; struct CallIndirector : public PostWalker { ModuleSplitter& parent; - std::set& activeTableUsingSecondaries; - CallIndirector(ModuleSplitter& parent, - std::set& activeTableUsingSecondaries) - : parent(parent), - activeTableUsingSecondaries(activeTableUsingSecondaries) {} + std::unordered_set activeTableUsingSecondaries; + CallIndirector(ModuleSplitter& parent) : parent(parent) {} void visitCall(Call* curr) { // Return if the call's target is not in one of the secondary module. if (!parent.allSecondaryFuncs.contains(curr->target)) { @@ -1055,13 +1051,13 @@ void ModuleSplitter::indirectCallsToSecondaryFunctions() { } } }; - CallIndirector callIndirector(*this, activeTableUsingSecondaries); + CallIndirector callIndirector(*this); callIndirector.walkModule(&primary); for (auto& secondaryPtr : secondaries) { callIndirector.walkModule(secondaryPtr.get()); } - for (auto* secondary : activeTableUsingSecondaries) { + for (auto* secondary : callIndirector.activeTableUsingSecondaries) { shareActiveTable(secondary); } }