diff --git a/src/ir/module-splitting.cpp b/src/ir/module-splitting.cpp index 77ae38fe709..3544610b654 100644 --- a/src/ir/module-splitting.cpp +++ b/src/ir/module-splitting.cpp @@ -583,6 +583,25 @@ Expression* ModuleSplitter::maybeLoadSecondary(Builder& builder, return builder.makeSequence(loadSecondary, callIndirect); } +// Helper to walk expressions in segments but NOT in globals. +template +static void walkSegments(Walker& walker, Module* module) { + walker.setModule(module); + for (auto& curr : module->elementSegments) { + if (curr->offset) { + walker.walk(curr->offset); + } + for (auto* item : curr->data) { + walker.walk(item); + } + } + for (auto& curr : module->dataSegments) { + if (curr->offset) { + walker.walk(curr->offset); + } + } +} + void ModuleSplitter::indirectReferencesToSecondaryFunctions() { // Turn references to secondary functions into references to thunks that // perform a direct call to the original referent. The direct calls in the @@ -977,7 +996,19 @@ void ModuleSplitter::shareImportableItems() { } NameCollector collector(used); - collector.walkModuleCode(&module); + // We shouldn't use collector.walkModuleCode here, because we don't want to + // walk on global initializers. At this point, all globals are still in the + // primary module, so if we walk on global initializers here, globals appear + // in their initialalizers will be all marked as used in the primary module, + // which is not true. + // + // For example, we have (global $a i32 (global.get $b)). Because $a is at + // this point still in the primary module, $b will be marked as "used" in + // the primary module. But $a can be moved to a secondary module later if it + // is used exclusively by that module. Then $b can be also moved, in case it + // doesn't have other uses. But if it is marked as "used" in the primary + // module, it can't. + walkSegments(collector, &module); for (auto& segment : module.dataSegments) { if (segment->memory.is()) { used.memories.insert(segment->memory); @@ -1009,25 +1040,33 @@ void ModuleSplitter::shareImportableItems() { secondaryUsed.push_back(getUsedNames(*secondaryPtr)); } - // Compute 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 computeDependentItems = [&](UsedNames& used) { + // Compute 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) { std::vector worklist(used.globals.begin(), used.globals.end()); - for (auto name : worklist) { + std::unordered_set visited(used.globals.begin(), used.globals.end()); + while (!worklist.empty()) { + Name currName = worklist.back(); + worklist.pop_back(); // At this point all globals are still in the primary module, so this // exists - auto* global = primary.getGlobal(name); + auto* global = primary.getGlobal(currName); if (!global->imported() && global->init) { for (auto* get : FindAll(global->init).list) { - used.globals.insert(get->name); + if (visited.insert(get->name).second) { + worklist.push_back(get->name); + used.globals.insert(get->name); + } } } } }; + computeTransitiveGlobals(primaryUsed); for (auto& used : secondaryUsed) { - computeDependentItems(used); + computeTransitiveGlobals(used); } // Given a name and module item kind, returns the list of secondary modules diff --git a/test/lit/wasm-split/transitive-globals.wast b/test/lit/wasm-split/transitive-globals.wast index 90740adc3a3..603e0cc4c8a 100644 --- a/test/lit/wasm-split/transitive-globals.wast +++ b/test/lit/wasm-split/transitive-globals.wast @@ -3,26 +3,20 @@ ;; RUN: wasm-dis %t.2.wasm | filecheck %s --check-prefix SECONDARY ;; Check that transitive dependencies in global initializers are correctly -;; analyzed and exported from the primary module to the secondary module. -;; TODO Move $b and $c to the secondary module +;; analyzed and moved to the secondary module. (module - ;; PRIMARY: (global $c i32 (i32.const 42)) + ;; SECONDARY: (global $c i32 (i32.const 42)) (global $c i32 (i32.const 42)) ;; $b depends on $c. - ;; PRIMARY: (global $b i32 (global.get $c)) + ;; SECONDARY: (global $b i32 (global.get $c)) (global $b i32 (global.get $c)) - ;; Globals $b is exported to the secondary module - ;; PRIMARY: (export "global" (global $b)) - - ;; Globals $b is imported from the primary module - ;; SECONDARY: (import "primary" "global" (global $b i32)) - - ;; $a depends on $b. Since $a is exclusively used by the secondary module, - ;; it will be moved there. Its dependency $b should be exported from the - ;; primary module and imported into the secondary module. + ;; $a depends on $b. since $a is exclusively used by the secondary module, + ;; it will be moved there. The transitive dependency must ensure that $b (and + ;; $c) are moved to the secondary module too, because they are not used in the + ;; primary module. ;; SECONDARY: (global $a i32 (global.get $b)) (global $a i32 (global.get $b))