-
Notifications
You must be signed in to change notification settings - Fork 853
[wasm-split] Support --placeholdermap for --multi-split #7792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
cf7ca5c
188e163
a278ec2
11d1a57
ae2142d
b7ad1e1
be1c37f
cc6978a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,13 +20,11 @@ | |||||||||||||||||||||
| #include <fstream> | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #include "ir/module-splitting.h" | ||||||||||||||||||||||
| #include "ir/names.h" | ||||||||||||||||||||||
| #include "support/file.h" | ||||||||||||||||||||||
| #include "support/name.h" | ||||||||||||||||||||||
| #include "support/path.h" | ||||||||||||||||||||||
| #include "support/utilities.h" | ||||||||||||||||||||||
| #include "wasm-binary.h" | ||||||||||||||||||||||
| #include "wasm-builder.h" | ||||||||||||||||||||||
| #include "wasm-io.h" | ||||||||||||||||||||||
| #include "wasm-validator.h" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -203,12 +201,24 @@ void writeSymbolMap(Module& wasm, std::string filename) { | |||||||||||||||||||||
| runner.run(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void writePlaceholderMap(const std::map<size_t, Name> placeholderMap, | ||||||||||||||||||||||
| std::string filename) { | ||||||||||||||||||||||
| void writePlaceholderMap( | ||||||||||||||||||||||
| Module& wasm, | ||||||||||||||||||||||
| const std::unordered_map<Name, std::map<size_t, Name>> placeholderMap, | ||||||||||||||||||||||
| std::string filename) { | ||||||||||||||||||||||
| Output output(filename, Flags::Text); | ||||||||||||||||||||||
| auto& o = output.getStream(); | ||||||||||||||||||||||
| for (auto& [index, func] : placeholderMap) { | ||||||||||||||||||||||
| o << index << ':' << func << '\n'; | ||||||||||||||||||||||
| for (Index i = 0; i < wasm.tables.size(); i++) { | ||||||||||||||||||||||
| const auto& table = wasm.tables[i]; | ||||||||||||||||||||||
| auto it = placeholderMap.find(table->name); | ||||||||||||||||||||||
| if (it != placeholderMap.end()) { | ||||||||||||||||||||||
| o << "table " << i << " (" << table->name << ")" << "\n"; | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the maps would be more readable with the
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about printing only explicit names?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So when reference types is not enabled (or some emscripten condition is met) we use an existing table, so in this case there will be a single table to print placeholders on. binaryen/src/ir/module-splitting.cpp Lines 164 to 167 in 9fe0d16
And when reference types is enabled we create table per split module, but they don't have explicit names (and even if we set binaryen/src/ir/module-splitting.cpp Lines 230 to 233 in 9fe0d16
Should we just print indices only?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good point about the explicit names not meaning much. Yes, I guess we should print only the indices. |
||||||||||||||||||||||
| for (auto& [index, func] : it->second) { | ||||||||||||||||||||||
| o << index << ':' << func << '\n'; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (i < wasm.tables.size() - 1) { | ||||||||||||||||||||||
| o << "\n"; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -344,7 +354,8 @@ void splitModule(const WasmSplitOptions& options) { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (options.placeholderMap) { | ||||||||||||||||||||||
| writePlaceholderMap(splitResults.placeholderMap, | ||||||||||||||||||||||
| writePlaceholderMap(wasm, | ||||||||||||||||||||||
| splitResults.placeholderMap, | ||||||||||||||||||||||
| options.primaryOutput + ".placeholders"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -422,6 +433,7 @@ void multiSplitModule(const WasmSplitOptions& options) { | |||||||||||||||||||||
| wasm.name = Path::getBaseName(options.output); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| std::unordered_map<Name, std::map<size_t, Name>> placeholderMap; | ||||||||||||||||||||||
| for (auto& [mod, funcs] : moduleFuncs) { | ||||||||||||||||||||||
| if (options.verbose) { | ||||||||||||||||||||||
| std::cerr << "Splitting module " << mod << '\n'; | ||||||||||||||||||||||
|
|
@@ -431,12 +443,14 @@ void multiSplitModule(const WasmSplitOptions& options) { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| config.secondaryFuncs = std::set<Name>(funcs.begin(), funcs.end()); | ||||||||||||||||||||||
| auto splitResults = ModuleSplitting::splitFunctions(wasm, config); | ||||||||||||||||||||||
| // TODO: placeholderMap | ||||||||||||||||||||||
| auto moduleName = | ||||||||||||||||||||||
| options.outPrefix + mod + (options.emitBinary ? ".wasm" : ".wast"); | ||||||||||||||||||||||
| if (options.symbolMap) { | ||||||||||||||||||||||
| writeSymbolMap(*splitResults.secondary, moduleName + ".symbols"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (options.placeholderMap) { | ||||||||||||||||||||||
| placeholderMap.merge(splitResults.placeholderMap); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (options.emitModuleNames) { | ||||||||||||||||||||||
| splitResults.secondary->name = Path::getBaseName(moduleName); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -445,7 +459,9 @@ void multiSplitModule(const WasmSplitOptions& options) { | |||||||||||||||||||||
| if (options.symbolMap) { | ||||||||||||||||||||||
| writeSymbolMap(wasm, options.output + ".symbols"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (options.placeholderMap) { | ||||||||||||||||||||||
| writePlaceholderMap(wasm, placeholderMap, options.output + ".placeholders"); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| writeModule(wasm, options.output, options); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| ;; RUN: wasm-split -all --multi-split %s --manifest %S/multi-split.wast.manifest --out-prefix=%t --placeholdermap -o %t.wasm | ||
| ;; RUN: filecheck %s --check-prefix MAP < %t.wasm.placeholders | ||
|
|
||
| ;; MAP: table 0 (0) | ||
| ;; MAP-NEXT: 0:A | ||
| ;; MAP-NEXT: | ||
| ;; MAP-NEXT: table 1 (0_1) | ||
| ;; MAP-NEXT: 0:B | ||
| ;; MAP-NEXT: | ||
| ;; MAP-NEXT: table 2 (0_2) | ||
| ;; MAP-NEXT: 0:C | ||
|
|
||
| (module | ||
| (type $ret-i32 (func (result i32))) | ||
| (type $ret-i64 (func (result i64))) | ||
| (type $ret-f32 (func (result f32))) | ||
|
|
||
| (func $A (type $ret-i32) (result i32) | ||
| (drop | ||
| (call_ref $ret-i32 | ||
| (ref.func $A) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-i64 | ||
| (ref.func $B) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-f32 | ||
| (ref.func $C) | ||
| ) | ||
| ) | ||
| (i32.const 0) | ||
| ) | ||
|
|
||
| (func $B (type $ret-i64) (result i64) | ||
| (drop | ||
| (call_ref $ret-i32 | ||
| (ref.func $A) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-i64 | ||
| (ref.func $B) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-f32 | ||
| (ref.func $C) | ||
| ) | ||
| ) | ||
| (i64.const 0) | ||
| ) | ||
|
|
||
| (func $C (type $ret-f32) (result f32) | ||
| (drop | ||
| (call_ref $ret-i32 | ||
| (ref.func $A) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-i64 | ||
| (ref.func $B) | ||
| ) | ||
| ) | ||
| (drop | ||
| (call_ref $ret-f32 | ||
| (ref.func $C) | ||
| ) | ||
| ) | ||
| (f32.const 0) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the call sites, it looks like this should be a reference (even though it wasn't before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done