[wasm-split] Support --placeholdermap for --multi-split #7792

aheejin · 2025-08-05T06:24:04Z

This adds support for --placeholdermap for --multi-split. Because there can be multiple tables where placeholders are placed, this modifies the placeholder file format slightly by adding table indices for each table.

This adds support for `--placeholdermap` for `--multi-split`. In the normal two-way split mode, `--placeholdermap` emits a single placeholdermap file for the primary module, but semantically a placeholdermap should exist per split module. I guess we should switch to make the placeholder file name follow the secondary module for the two-way split for consistency?

tlively · 2025-08-05T18:00:06Z

Why do we need a placeholder per split module? All the functions should still have unique names across all the modules and each placeholder still has a unique index, so shouldn't a single map be able to describe all the placeholders?

aheejin · 2025-08-06T18:01:58Z

Right, I was confused by the fact that when we give -all, we create a separate table for each split module. But in that case how can we structure the placeholdermap file? For example, if we have three tables table_a, table_b, and table_c and each of them is shared with module module_a, module_b, and module_c, then placeholders will start from 0 for each table, then we have three placeholders at index 0, for example.

tlively · 2025-08-06T18:13:46Z

Oh right, I forgot there would be multiple tables. To make this useful for people, we'll need to say what table each placeholder is in, whether or not we produce multiple map files.

My preference would be to continue emitting just one map file, but to organize it into sections for each table. It would be good to check whether browser errors include the table name or index and make sure that same information is present in the placeholder map.

aheejin · 2025-08-06T21:00:45Z

How can I check for the browser errors?

Who is using this format at the moment? I can trace this feature to the discussions in emscripten-core/emscripten#14330. Is @waterlike86 the only person using this? Is this file format expected to be parsed by the browser?

tlively · 2025-08-06T21:17:11Z

This is supposed to be a human readable format to help interpret JS errors about calling functions that have not been loaded yet. The idea is that the error gives you a table index and you look in the file to see what function was supposed to be there. With multiple tables, the file should let you map a table and an index to the missing function.

aheejin · 2025-08-09T05:16:07Z

I tried to intentionally generate errors with placeholder functions in a browser by modifying js files, but I didn't see table indices or table names in the error messages. Maybe I didn't generate the right kind of error. But anyway if this file is supposed to be read by human, I think printing both indices and names wouldn't hurt.

aheejin · 2025-08-09T05:18:44Z

I wonder what clang-format CI is doing...?

diff --git a/src/tools/wasm-split/wasm-split.cpp b/src/tools/wasm-split/wasm-split.cpp
index 451380110..a230f3e0d 100644
--- a/src/tools/wasm-split/wasm-split.cpp
+++ b/src/tools/wasm-split/wasm-split.cpp
@@ -211,7 +211,8 @@ void writePlaceholderMap(
     const auto& table = wasm.tables[i];
     auto it = placeholderMap.find(table->name);
     if (it != placeholderMap.end()) {
-      o << "table " << i << " (" << table->name << ")" << "\n";
+      o << "table " << i << " (" << table->name << ")"
+        << "\n";
       for (auto& [index, func] : it->second) {
         o << index << ':' << func << '\n';
       }

That line is only 63 cols so we shouldn't be wrapping it....

tlively · 2025-08-10T03:52:51Z

Hmm, I would hope that the error you get when the table slot is null (i.e. --no-placeholders) would include the index and table information. But maybe you'd have to provide placeholders with your own error messages.

tlively · 2025-08-10T03:54:01Z

Also if you want to merge despite the clang-format error, that's ok with me.

tlively · 2025-08-10T03:55:38Z

src/tools/wasm-split/wasm-split.cpp

-                         std::string filename) {
+void writePlaceholderMap(
+  Module& wasm,
+  const std::unordered_map<Name, std::map<size_t, Name>> placeholderMap,


Based on the call sites, it looks like this should be a reference (even though it wasn't before).

tlively · 2025-08-10T03:59:36Z

src/tools/wasm-split/wasm-split.cpp

+    const auto& table = wasm.tables[i];
+    auto it = placeholderMap.find(table->name);
+    if (it != placeholderMap.end()) {
+      o << "table " << i << " (" << table->name << ")" << "\n";


I think the maps would be more readable with the $ on the name. Also, should we only print the name if it is explicit?

Suggested change

o << "table " << i << " (" << table->name << ")" << "\n";

o << "table " << i << " ($" << table->name << ")" << "\n";

WDYT about printing only explicit names?

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

if (module.features.hasReferenceTypes() && !emscriptenTableExport &&

!emscriptenTableImport) {

return;

}

And when reference types is enabled we create table per split module, but they don't have explicit names (and even if we set hasExplicitName to true, it wouldn't mean much, given that they are autogenerated:

binaryen/src/ir/module-splitting.cpp

Lines 230 to 233 in 9fe0d16

Table* TableSlotManager::makeTable() {

return module.addTable(

Builder::makeTable(Names::getValidTableName(module, Name::fromInt(0))));

}

Should we just print indices only?

Oh, good point about the explicit names not meaning much. Yes, I guess we should print only the indices.

aheejin · 2025-08-11T22:54:45Z

Hmm, I would hope that the error you get when the table slot is null (i.e. --no-placeholders) would include the index and table information. But maybe you'd have to provide placeholders with your own error messages.

I tried this but in this case we get the RuntimeError: null function or function signature mismatch without any table index or name.

Co-authored-by: Thomas Lively <tlively123@gmail.com>

aheejin requested a review from tlively August 5, 2025 06:24

aheejin added 4 commits August 9, 2025 01:43

Merge branch 'main' into multi_split_placeholdermap

188e163

Print multiple tables for primary module

a278ec2

Remove include

11d1a57

fix

ae2142d

tlively reviewed Aug 10, 2025

View reviewed changes

aheejin and others added 3 commits August 11, 2025 15:55

Update src/tools/wasm-split/wasm-split.cpp

b7ad1e1

Co-authored-by: Thomas Lively <tlively123@gmail.com>

Merge branch 'main' into multi_split_placeholdermap

be1c37f

Only print indices

cc6978a

tlively approved these changes Aug 12, 2025

View reviewed changes

aheejin merged commit 819a39f into WebAssembly:main Aug 12, 2025
16 checks passed

aheejin deleted the multi_split_placeholdermap branch August 12, 2025 01:18

	o << "table " << i << " (" << table->name << ")" << "\n";
	o << "table " << i << " ($" << table->name << ")" << "\n";

	if (module.features.hasReferenceTypes() && !emscriptenTableExport &&
	!emscriptenTableImport) {
	return;
	}

	Table* TableSlotManager::makeTable() {
	return module.addTable(
	Builder::makeTable(Names::getValidTableName(module, Name::fromInt(0))));
	}

[wasm-split] Support --placeholdermap for --multi-split #7792

[wasm-split] Support --placeholdermap for --multi-split #7792

Uh oh!

Conversation

aheejin commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlively commented Aug 5, 2025

Uh oh!

aheejin commented Aug 6, 2025

Uh oh!

tlively commented Aug 6, 2025

Uh oh!

aheejin commented Aug 6, 2025

Uh oh!

tlively commented Aug 6, 2025

Uh oh!

aheejin commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aheejin commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlively commented Aug 10, 2025

Uh oh!

tlively commented Aug 10, 2025

Uh oh!

tlively Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

aheejin Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

tlively Aug 10, 2025

Choose a reason for hiding this comment

Uh oh!

tlively Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

aheejin Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

tlively Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

aheejin commented Aug 11, 2025

Uh oh!

Uh oh!

Uh oh!

aheejin commented Aug 5, 2025 •

edited

Loading

aheejin commented Aug 9, 2025 •

edited

Loading

aheejin commented Aug 9, 2025 •

edited

Loading