From 08634b40c017ceccfa447ffe7919ae03e10fc15d Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Tue, 21 Jul 2026 18:21:12 -0700 Subject: [PATCH 01/15] Parse text format for compact import sections Update the parser for imports to support both forms of compact imports. Because imports are no longer 1:1 with import statements, the locations of imported items the parser collects and re-parses in various phases must now be the locations of importdesc productions rather than import productions. Since e.g. a function importdesc cannot be differentiated from a function definition with an empty body without context, we must add a mechanism for the parser context to tell the parser whether or not it is parsing an import. --- src/parser/contexts.h | 38 ++++++++ src/parser/parsers.h | 144 +++++++++++++++++++++------- test/lit/basic/compact-imports.wast | 63 ++++++++++++ 3 files changed, 208 insertions(+), 37 deletions(-) create mode 100644 test/lit/basic/compact-imports.wast diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 31a3aa253e3..b8a84d90ede 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -1007,6 +1007,12 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { Lexer in; + bool isFunctionImported() { return false; } + bool isTableImported() { return false; } + bool isMemoryImported() { return false; } + bool isGlobalImported() { return false; } + bool isTagImported() { return false; } + // At this stage we only look at types to find implicit type definitions, // which are inserted directly into the context. We cannot materialize or // validate any types because we don't know what types exist yet. @@ -1470,6 +1476,22 @@ struct ParseModuleTypesCtx : TypeParserCtx, bool skipFunctionBody() { return true; } + bool isFunctionImported() { + return index < wasm.functions.size() && wasm.functions[index]->imported(); + } + bool isTableImported() { + return index < wasm.tables.size() && wasm.tables[index]->imported(); + } + bool isMemoryImported() { + return index < wasm.memories.size() && wasm.memories[index]->imported(); + } + bool isGlobalImported() { + return index < wasm.globals.size() && wasm.globals[index]->imported(); + } + bool isTagImported() { + return index < wasm.tags.size() && wasm.tags[index]->imported(); + } + Result getHeapTypeFromIdx(Index idx) { if (idx >= types.size()) { return in.err("type index out of bounds"); @@ -1644,6 +1666,22 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Lexer in; + bool isFunctionImported() { + return index < wasm.functions.size() && wasm.functions[index]->imported(); + } + bool isTableImported() { + return index < wasm.tables.size() && wasm.tables[index]->imported(); + } + bool isMemoryImported() { + return index < wasm.memories.size() && wasm.memories[index]->imported(); + } + bool isGlobalImported() { + return index < wasm.globals.size() && wasm.globals[index]->imported(); + } + bool isTagImported() { + return index < wasm.tags.size() && wasm.tags[index]->imported(); + } + Module& wasm; Builder builder; diff --git a/src/parser/parsers.h b/src/parser/parsers.h index d6f2297ff57..4e36f0526eb 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -393,6 +393,8 @@ template Result<> subtype(Ctx&); template MaybeResult<> typedef_(Ctx&); template MaybeResult<> rectype(Ctx&); template MaybeResult locals(Ctx&); +template +Result<> importdesc(Ctx&, Name, Name, std::optional); template MaybeResult<> import_(Ctx&); template MaybeResult<> func(Ctx&); template MaybeResult<> table(Ctx&); @@ -3424,60 +3426,51 @@ template MaybeResult locals(Ctx& ctx) { return {}; } -// import ::= '(' 'import' mod:name nm:name importdesc ')' // importdesc ::= '(' 'func' id? exacttypeuse ')' // | '(' 'table' id? tabletype ')' // | '(' 'memory' id? memtype ')' // | '(' 'global' id? globaltype ')' // | '(' 'tag' id? typeuse ')' -template MaybeResult<> import_(Ctx& ctx) { +template +Result<> importdesc(Ctx& ctx, Name mod, Name nm, std::optional id) { auto pos = ctx.in.getPos(); - - if (!ctx.in.takeSExprStart("import"sv)) { - return {}; - } - - auto mod = ctx.in.takeName(); - if (!mod) { - return ctx.in.err("expected import module name"); - } - - auto nm = ctx.in.takeName(); - if (!nm) { - return ctx.in.err("expected import name"); - } - ImportNames names{*mod, *nm}; - + // We only try to parse an ID if `id` is nullopt. + auto getID = [&]() { + if (id) { + return *id; + } + auto parsed = ctx.in.takeID(); + return parsed ? *parsed : Name{}; + }; + ImportNames names{mod, nm}; if (ctx.in.takeSExprStart("func"sv)) { - auto name = ctx.in.takeID(); + auto name = getID(); auto use = exacttypeuse(ctx); CHECK_ERR(use); auto [type, exact] = *use; // TODO: function import annotations - CHECK_ERR(ctx.addFunc( - name ? *name : Name{}, {}, &names, type, exact, std::nullopt, {}, pos)); + CHECK_ERR( + ctx.addFunc(name, {}, &names, type, exact, std::nullopt, {}, pos)); } else if (ctx.in.takeSExprStart("table"sv)) { - auto name = ctx.in.takeID(); + auto name = getID(); auto type = tabletype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTable( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addTable(name, {}, &names, *type, std::nullopt, pos)); } else if (ctx.in.takeSExprStart("memory"sv)) { - auto name = ctx.in.takeID(); + auto name = getID(); auto type = memtype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addMemory(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addMemory(name, {}, &names, *type, pos)); } else if (ctx.in.takeSExprStart("global"sv)) { - auto name = ctx.in.takeID(); + auto name = getID(); auto type = globaltype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addGlobal( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addGlobal(name, {}, &names, *type, std::nullopt, pos)); } else if (ctx.in.takeSExprStart("tag"sv)) { - auto name = ctx.in.takeID(); + auto name = getID(); auto type = typeuse(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTag(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addTag(name, {}, &names, *type, pos)); } else { return ctx.in.err("expected import description"); } @@ -3485,6 +3478,75 @@ template MaybeResult<> import_(Ctx& ctx) { if (!ctx.in.takeRParen()) { return ctx.in.err("expected end of import description"); } + return Ok{}; +} + +// import ::= '(' 'import' mod:name nm:name importdesc ')' +// | '(' 'import' mod:name (item id? nm:name importdesc)+ ')' +// | '(' 'import' mod:name (item id? nm:name)+ importdesc ')' +template MaybeResult<> import_(Ctx& ctx) { + if (!ctx.in.takeSExprStart("import"sv)) { + return {}; + } + + auto mod = ctx.in.takeName(); + if (!mod) { + return ctx.in.err("expected import module name"); + } + + if (auto nm = ctx.in.takeName()) { + CHECK_ERR(importdesc(ctx, *mod, *nm, std::nullopt)); + if (!ctx.in.takeRParen()) { + return ctx.in.err("expected end of import"); + } + return Ok{}; + } + + struct CompactItem { + Name id; + Name name; + }; + std::vector items; + std::optional hasSharedImportDesc; + while (ctx.in.takeSExprStart("item"sv)) { + auto id = ctx.in.takeID(); + if (!id) { + // Do not allow parsing an id in the importdesc. + id = Name{}; + } + + auto nm = ctx.in.takeName(); + if (!nm) { + return ctx.in.err("expected import name"); + } + + if (!hasSharedImportDesc.has_value()) { + hasSharedImportDesc = ctx.in.peekRParen(); + } + + if (*hasSharedImportDesc) { + items.emplace_back(*id, *nm); + } else { + CHECK_ERR(importdesc(ctx, *mod, *nm, id)) + } + + if (!ctx.in.takeRParen()) { + return ctx.in.err("expected end of import item"); + } + } + if (!hasSharedImportDesc.has_value()) { + // There were no items. + return ctx.in.err("expected import name or item"); + } + + if (*hasSharedImportDesc) { + auto pos = ctx.in.getPos(); + for (auto item : items) { + ctx.in.setPos(pos); + CHECK_ERR(importdesc(ctx, *mod, item.name, item.id)); + } + } + if (!ctx.in.takeRParen()) { return ctx.in.err("expected end of import"); } @@ -3515,12 +3577,14 @@ template MaybeResult<> func(Ctx& ctx) { auto import = inlineImport(ctx.in); CHECK_ERR(import); + bool isImport = import || ctx.isFunctionImported(); + typename Ctx::TypeUseT type; Exactness exact = Exact; std::optional localVars; bool skipped = false; - if (import) { + if (isImport) { auto use = exacttypeuse(ctx); CHECK_ERR(use); type = use->first; @@ -3540,7 +3604,7 @@ template MaybeResult<> func(Ctx& ctx) { } } - if ((import || !skipped) && !ctx.in.takeRParen()) { + if ((isImport || !skipped) && !ctx.in.takeRParen()) { return ctx.in.err("expected end of function"); } @@ -3580,6 +3644,8 @@ template MaybeResult<> table(Ctx& ctx) { auto import = inlineImport(ctx.in); CHECK_ERR(import); + bool isImport = import || ctx.isTableImported(); + auto addressType = Type::i32; if (ctx.in.takeKeyword("i64"sv)) { addressType = Type::i64; @@ -3599,7 +3665,7 @@ template MaybeResult<> table(Ctx& ctx) { if (!ctx.in.takeSExprStart("elem"sv)) { return ctx.in.err("expected table limits or inline elements"); } - if (import) { + if (isImport) { return ctx.in.err("imported tables cannot have inline elements"); } @@ -3628,7 +3694,7 @@ template MaybeResult<> table(Ctx& ctx) { auto tabtype = tabletypeContinued(ctx, addressType); CHECK_ERR(tabtype); ttype = *tabtype; - if (ctx.in.peekLParen() && !import) { + if (ctx.in.peekLParen() && !isImport) { // Imported tables cannot have initialization expression. auto e = expr(ctx); CHECK_ERR(e); @@ -3670,6 +3736,8 @@ template MaybeResult<> memory(Ctx& ctx) { auto import = inlineImport(ctx.in); CHECK_ERR(import); + bool isImport = import || ctx.isMemoryImported(); + auto addressType = Type::i32; if (ctx.in.takeKeyword("i64"sv)) { addressType = Type::i64; @@ -3682,7 +3750,7 @@ template MaybeResult<> memory(Ctx& ctx) { MaybeResult mempageSize = mempagesize(ctx); CHECK_ERR(mempageSize); if (ctx.in.takeSExprStart("data"sv)) { - if (import) { + if (isImport) { return ctx.in.err("imported memories cannot have inline data"); } auto datastr = datastring(ctx); @@ -3740,11 +3808,13 @@ template MaybeResult<> global(Ctx& ctx) { auto import = inlineImport(ctx.in); CHECK_ERR(import); + bool isImport = import || ctx.isGlobalImported(); + auto type = globaltype(ctx); CHECK_ERR(type); std::optional exp; - if (!import) { + if (!isImport) { auto e = expr(ctx); CHECK_ERR(e); exp = *e; diff --git a/test/lit/basic/compact-imports.wast b/test/lit/basic/compact-imports.wast new file mode 100644 index 00000000000..52823f9bf21 --- /dev/null +++ b/test/lit/basic/compact-imports.wast @@ -0,0 +1,63 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: wasm-opt %s -all -S -o - | filecheck %s + +(module + ;; CHECK: (type $sig1 (func (param i32) (result i32))) + (type $sig1 (func (param i32) (result i32))) + + ;; Compact Encoding 1: per-item import descriptions + (import "env" + (item $f1 "f1" (func (type $sig1))) + (item $f2 "f2" (func (param i32) (result i32))) + (item $g1 "g1" (global i32)) + (item $t1 "t1" (table 1 10 funcref)) + (item $m1 "m1" (memory 1 2)) + ) + + ;; Compact Encoding 2: shared import description + (import "math" + (item $sin "sin") + (item $cos "cos") + (item $tan "tan") + (func (param i32) (result i32)) + ) + + ;; Compact Encoding 2: shared global import description + (import "constants" + (item $pi "pi") + (item $e "e") + (global f64) + ) + + ;; CHECK: (type $1 (func (result i32))) + + ;; CHECK: (import "env" "m1" (memory $m1 1 2)) + + ;; CHECK: (import "env" "t1" (table $t1 1 10 funcref)) + + ;; CHECK: (import "env" "g1" (global $g1 i32)) + + ;; CHECK: (import "constants" "pi" (global $pi f64)) + + ;; CHECK: (import "constants" "e" (global $e f64)) + + ;; CHECK: (import "env" "f1" (func $f1 (type $sig1) (param i32) (result i32))) + + ;; CHECK: (import "env" "f2" (func $f2 (type $sig1) (param i32) (result i32))) + + ;; CHECK: (import "math" "sin" (func $sin (type $sig1) (param i32) (result i32))) + + ;; CHECK: (import "math" "cos" (func $cos (type $sig1) (param i32) (result i32))) + + ;; CHECK: (import "math" "tan" (func $tan (type $sig1) (param i32) (result i32))) + + ;; CHECK: (func $main (type $1) (result i32) + ;; CHECK-NEXT: (call $f1 + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $main (result i32) + (call $f1 (i32.const 1)) + ) +) From 8bec2069769afce54aefb099a1e45f57c1f72f40 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Tue, 21 Jul 2026 19:32:47 -0700 Subject: [PATCH 02/15] [WIP SLOP] Opportunisticly emit compact imports --- scripts/clusterfuzz/run.py | 2 +- scripts/test/shared.py | 1 + src/wasm/wasm-binary.cpp | 217 ++++++++++++++---- test/lit/basic/compact-imports-roundtrip.wast | 50 ++++ test/unit/test_compact_imports.py | 61 +++++ 5 files changed, 287 insertions(+), 44 deletions(-) create mode 100644 test/lit/basic/compact-imports-roundtrip.wast create mode 100644 test/unit/test_compact_imports.py diff --git a/scripts/clusterfuzz/run.py b/scripts/clusterfuzz/run.py index f7c48b1b047..aa0148c5834 100755 --- a/scripts/clusterfuzz/run.py +++ b/scripts/clusterfuzz/run.py @@ -33,7 +33,7 @@ # The V8 flags we put in the "fuzzer flags" files, which tell ClusterFuzz how to # run V8. By default we apply all staging flags. -FUZZER_FLAGS = '--wasm-staging --experimental-wasm-custom-descriptors --experimental-wasm-js-interop --experimental-wasm-wide-arithmetic' +FUZZER_FLAGS = '--wasm-staging --experimental-wasm-custom-descriptors --experimental-wasm-js-interop --experimental-wasm-wide-arithmetic --wasm-compact-imports' # Optional V8 flags to add to FUZZER_FLAGS, some of the time. OPTIONAL_FUZZER_FLAGS = [ diff --git a/scripts/test/shared.py b/scripts/test/shared.py index 08d35710b9c..41c0710fdee 100644 --- a/scripts/test/shared.py +++ b/scripts/test/shared.py @@ -267,6 +267,7 @@ def has_shell_timeout(): '--experimental-wasm-custom-descriptors', '--experimental-wasm-js-interop', '--experimental-wasm-wide-arithmetic', + '--wasm-compact-imports', ] # external tools diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 34ab5ec51ee..95b68027085 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -331,57 +331,188 @@ void WasmBinaryWriter::writeTypes() { finishSection(start); } +template struct Overloaded : Ts... { + using Ts::operator()...; +}; +template Overloaded(Ts...) -> Overloaded; + void WasmBinaryWriter::writeImports() { auto num = importInfo->getNumImports(); if (num == 0) { return; } auto start = startSection(BinaryConsts::Section::Import); - o << U32LEB(num); - auto writeImportHeader = [&](Importable* import) { - writeInlineString(import->module.view()); - writeInlineString(import->base.view()); + + using ImportItem = std::variant; + std::vector imports; + imports.reserve(num); + + ModuleUtils::iterImportedFunctions( + *wasm, [&](Function* func) { imports.push_back(func); }); + ModuleUtils::iterImportedGlobals( + *wasm, [&](Global* global) { imports.push_back(global); }); + ModuleUtils::iterImportedTags(*wasm, + [&](Tag* tag) { imports.push_back(tag); }); + ModuleUtils::iterImportedMemories( + *wasm, [&](Memory* memory) { imports.push_back(memory); }); + ModuleUtils::iterImportedTables( + *wasm, [&](Table* table) { imports.push_back(table); }); + + auto getModule = [](const ImportItem& item) -> Name { + return std::visit([](auto* i) { return i->module; }, item); + }; + auto getBase = [](const ImportItem& item) -> Name { + return std::visit([](auto* i) { return i->base; }, item); }; - ModuleUtils::iterImportedFunctions(*wasm, [&](Function* func) { - writeImportHeader(func); - uint32_t kind = ExternalKind::Function; - if (func->type.isExact()) { - kind |= BinaryConsts::ExactImport; + + auto shareImportType = [&](const ImportItem& a, const ImportItem& b) -> bool { + if (a.index() != b.index()) { + return false; } - o << U32LEB(kind) << U32LEB(getTypeIndex(func->type.getHeapType())); - }); - ModuleUtils::iterImportedGlobals(*wasm, [&](Global* global) { - writeImportHeader(global); - o << U32LEB(int32_t(ExternalKind::Global)); - writeType(global->type); - o << U32LEB(global->mutable_); - }); - ModuleUtils::iterImportedTags(*wasm, [&](Tag* tag) { - writeImportHeader(tag); - o << U32LEB(int32_t(ExternalKind::Tag)); - o << uint8_t(0); // Reserved 'attribute' field. Always 0. - o << U32LEB(getTypeIndex(tag->type)); - }); - ModuleUtils::iterImportedMemories(*wasm, [&](Memory* memory) { - writeImportHeader(memory); - o << U32LEB(int32_t(ExternalKind::Memory)); - writeResizableLimits(memory->initial, - memory->max, - memory->hasMax(), - memory->shared, - memory->is64(), - memory->pageSizeLog2); - }); - ModuleUtils::iterImportedTables(*wasm, [&](Table* table) { - writeImportHeader(table); - o << U32LEB(int32_t(ExternalKind::Table)); - writeType(table->type); - writeResizableLimits(table->initial, - table->max, - table->hasMax(), - /*shared=*/false, - table->is64()); - }); + if (const auto* fa = std::get_if(&a)) { + auto* fb = std::get(b); + return (*fa)->type.isExact() == fb->type.isExact() && + getTypeIndex((*fa)->type.getHeapType()) == + getTypeIndex(fb->type.getHeapType()); + } + if (const auto* ga = std::get_if(&a)) { + auto* gb = std::get(b); + return (*ga)->type == gb->type && (*ga)->mutable_ == gb->mutable_; + } + if (const auto* ta = std::get_if(&a)) { + auto* tb = std::get(b); + return getTypeIndex((*ta)->type) == getTypeIndex(tb->type); + } + if (const auto* ma = std::get_if(&a)) { + auto* mb = std::get(b); + return (*ma)->initial == mb->initial && (*ma)->max == mb->max && + (*ma)->hasMax() == mb->hasMax() && (*ma)->shared == mb->shared && + (*ma)->is64() == mb->is64() && + (*ma)->pageSizeLog2 == mb->pageSizeLog2; + } + if (const auto* ta = std::get_if(&a)) { + auto* tb = std::get(b); + return (*ta)->type == tb->type && (*ta)->initial == tb->initial && + (*ta)->max == tb->max && (*ta)->hasMax() == tb->hasMax() && + (*ta)->is64() == tb->is64(); + } + return false; + }; + + struct ImportGroup { + enum Kind { Single, SharedAll, SharedModule } kind; + size_t start; + size_t count; + }; + + std::vector groups; + if (wasm->features.hasCompactImports()) { + size_t i = 0; + size_t N = imports.size(); + while (i < N) { + if (i + 1 < N && getModule(imports[i]) == getModule(imports[i + 1]) && + shareImportType(imports[i], imports[i + 1])) { + size_t j = i + 1; + while (j + 1 < N && + getModule(imports[i]) == getModule(imports[j + 1]) && + shareImportType(imports[i], imports[j + 1])) { + j++; + } + groups.push_back({ImportGroup::SharedAll, i, j - i + 1}); + i = j + 1; + } else if (i + 1 < N && + getModule(imports[i]) == getModule(imports[i + 1])) { + size_t j = i + 1; + while (j + 1 < N && + getModule(imports[i]) == getModule(imports[j + 1])) { + j++; + } + groups.push_back({ImportGroup::SharedModule, i, j - i + 1}); + i = j + 1; + } else { + groups.push_back({ImportGroup::Single, i, 1}); + i++; + } + } + } else { + for (size_t i = 0; i < imports.size(); ++i) { + groups.push_back({ImportGroup::Single, i, 1}); + } + } + + o << U32LEB(groups.size()); + + auto writeImportDetails = [&](const ImportItem& item) { + std::visit(Overloaded{[&](Function* func) { + uint32_t kind = ExternalKind::Function; + if (func->type.isExact()) { + kind |= BinaryConsts::ExactImport; + } + o << U32LEB(kind) + << U32LEB(getTypeIndex(func->type.getHeapType())); + }, + [&](Global* global) { + o << U32LEB(int32_t(ExternalKind::Global)); + writeType(global->type); + o << U32LEB(global->mutable_); + }, + [&](Tag* tag) { + o << U32LEB(int32_t(ExternalKind::Tag)); + o << uint8_t( + 0); // Reserved 'attribute' field. Always 0. + o << U32LEB(getTypeIndex(tag->type)); + }, + [&](Memory* memory) { + o << U32LEB(int32_t(ExternalKind::Memory)); + writeResizableLimits(memory->initial, + memory->max, + memory->hasMax(), + memory->shared, + memory->is64(), + memory->pageSizeLog2); + }, + [&](Table* table) { + o << U32LEB(int32_t(ExternalKind::Table)); + writeType(table->type); + writeResizableLimits(table->initial, + table->max, + table->hasMax(), + /*shared=*/false, + table->is64()); + }}, + item); + }; + + for (const auto& group : groups) { + if (group.kind == ImportGroup::Single) { + const auto& item = imports[group.start]; + writeInlineString(getModule(item).view()); + writeInlineString(getBase(item).view()); + writeImportDetails(item); + } else if (group.kind == ImportGroup::SharedAll) { + const auto& item0 = imports[group.start]; + writeInlineString(getModule(item0).view()); + writeInlineString(""); + o << uint8_t(BinaryConsts::CompactImportsSharedAll); + writeImportDetails(item0); + o << U32LEB(group.count); + for (size_t k = 0; k < group.count; ++k) { + writeInlineString(getBase(imports[group.start + k]).view()); + } + } else if (group.kind == ImportGroup::SharedModule) { + const auto& item0 = imports[group.start]; + writeInlineString(getModule(item0).view()); + writeInlineString(""); + o << uint8_t(BinaryConsts::CompactImportsSharedModule); + o << U32LEB(group.count); + for (size_t k = 0; k < group.count; ++k) { + const auto& item = imports[group.start + k]; + writeInlineString(getBase(item).view()); + writeImportDetails(item); + } + } + } + finishSection(start); } diff --git a/test/lit/basic/compact-imports-roundtrip.wast b/test/lit/basic/compact-imports-roundtrip.wast new file mode 100644 index 00000000000..bb5db734ae0 --- /dev/null +++ b/test/lit/basic/compact-imports-roundtrip.wast @@ -0,0 +1,50 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. +;; RUN: wasm-opt %s -all -o %t.wasm && wasm-opt %t.wasm -all -S -o - | filecheck %s + +(module + (type $sig1 (func (param i32) (result i32))) + (type $sig2 (func (param f64) (result f64))) + + ;; SharedAll functions: same module "env", same signature type $sig1 + (import "env" "f1" (func $f1 (type $sig1))) + (import "env" "f2" (func $f2 (type $sig1))) + + ;; SharedModule functions: same module "math", different signature types + (import "math" "sin" (func $sin (type $sig1))) + (import "math" "sqrt" (func $sqrt (type $sig2))) + + ;; SharedAll globals: same module "consts", same type & mutability + (import "consts" "g1" (global $g1 i32)) + (import "consts" "g2" (global $g2 i32)) + + ;; SharedModule mixed: same module "mixed", different import kinds + (import "mixed" "g3" (global $g3 (mut i32))) + (import "mixed" "t1" (table $t1 1 10 funcref)) + + ;; Single import: unique module "single" + (import "single" "m1" (memory $m1 1 2)) + + (func $main (result i32) + (call $f1 (i32.const 1)) + ) +) + +;; CHECK: (type $0 (func (param i32) (result i32))) +;; CHECK-NEXT: (type $1 (func (param f64) (result f64))) +;; CHECK-NEXT: (type $2 (func (result i32))) + +;; CHECK: (import "single" "m1" (memory $mimport$0 1 2)) +;; CHECK-NEXT: (import "mixed" "t1" (table $timport$0 1 10 funcref)) +;; CHECK-NEXT: (import "consts" "g1" (global $gimport$0 i32)) +;; CHECK-NEXT: (import "consts" "g2" (global $gimport$1 i32)) +;; CHECK-NEXT: (import "mixed" "g3" (global $gimport$2 (mut i32))) +;; CHECK-NEXT: (import "env" "f1" (func $fimport$0 (type $0) (param i32) (result i32))) +;; CHECK-NEXT: (import "env" "f2" (func $fimport$1 (type $0) (param i32) (result i32))) +;; CHECK-NEXT: (import "math" "sin" (func $fimport$2 (type $0) (param i32) (result i32))) +;; CHECK-NEXT: (import "math" "sqrt" (func $fimport$3 (type $1) (param f64) (result f64))) + +;; CHECK: (func $0 (type $2) (result i32) +;; CHECK-NEXT: (call $fimport$0 +;; CHECK-NEXT: (i32.const 1) +;; CHECK-NEXT: ) +;; CHECK-NEXT: ) diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py new file mode 100644 index 00000000000..56ffcbf9b45 --- /dev/null +++ b/test/unit/test_compact_imports.py @@ -0,0 +1,61 @@ +import os +from scripts.test import shared +from . import utils + + +class CompactImportsTest(utils.BinaryenTestCase): + def get_binary(self, wat_str, flags=[]): + cmd = shared.WASM_OPT + ['--print-features', '-o', '-'] + flags + p = shared.run_process(cmd, input=wat_str, check=True, capture_output=True) + return p.stdout.encode('latin1') if isinstance(p.stdout, str) else p.stdout + + def test_shared_all_encoding(self): + wat = '''(module + (type $sig (func (param i32) (result i32))) + (import "env" "f1" (func (type $sig))) + (import "env" "f2" (func (type $sig))) + )''' + wasm_bytes = self.get_binary(wat, ['--enable-compact-imports']) + # 0x7E is CompactImportsSharedAll + self.assertIn(b'\x03env\x00\x7e', wasm_bytes) + + def test_shared_module_encoding(self): + wat = '''(module + (type $sig1 (func (param i32) (result i32))) + (type $sig2 (func (param f64) (result f64))) + (import "env" "f1" (func (type $sig1))) + (import "env" "f2" (func (type $sig2))) + )''' + wasm_bytes = self.get_binary(wat, ['--enable-compact-imports']) + # 0x7F is CompactImportsSharedModule + self.assertIn(b'\x03env\x00\x7f', wasm_bytes) + + def test_disabled_compact_imports(self): + wat = '''(module + (type $sig (func (param i32) (result i32))) + (import "env" "f1" (func (type $sig))) + (import "env" "f2" (func (type $sig))) + )''' + wasm_bytes = self.get_binary(wat, ['--disable-compact-imports']) + self.assertNotIn(b'\x03env\x00\x7e', wasm_bytes) + self.assertNotIn(b'\x03env\x00\x7f', wasm_bytes) + self.assertIn(b'\x03env\x02f1', wasm_bytes) + self.assertIn(b'\x03env\x02f2', wasm_bytes) + + def test_mixed_import_patterns(self): + wat = '''(module + (type $sig1 (func (param i32) (result i32))) + (type $sig2 (func (param f64) (result f64))) + ;; Run 1: SharedAll for "env" + (import "env" "f1" (func (type $sig1))) + (import "env" "f2" (func (type $sig1))) + ;; Run 2: SharedModule for "math" (different types) + (import "math" "sin" (func (type $sig1))) + (import "math" "sqrt" (func (type $sig2))) + ;; Run 3: Single import for "single" + (import "single" "m1" (memory 1 2)) + )''' + wasm_bytes = self.get_binary(wat, ['--enable-compact-imports']) + self.assertIn(b'\x03env\x00\x7e', wasm_bytes) + self.assertIn(b'\x04math\x00\x7f', wasm_bytes) + self.assertIn(b'\x06single\x02m1', wasm_bytes) From e401ce075d4f08fd550c74e696a29bfece0dc3ff Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jul 2026 17:09:37 -0700 Subject: [PATCH 03/15] [NFC] Track grammar of definitions in parser When e.g. a function is declared in the text format, it can either be with a normal `func` production (which may or may not have an inline import declaration) or it can be an `import` production. Previously the parser would just store the location of the declaration but not the kind of production it used. In later parser phases where the declaration has to be parsed again, we would just try both productions to see which one worked. This is rather hacky and brittle and will not play nicely with the text format for compact imports, so refactor to explicitly track how the declaration should be parsed. --- src/parser/context-decls.cpp | 31 ++++++++------ src/parser/context-defs.cpp | 6 ++- src/parser/contexts.h | 71 ++++++++++++++++++++++---------- src/parser/parse-5-defs.cpp | 8 ++-- src/parser/parsers.h | 50 ++++++++++++++++------ src/parser/wat-parser-internal.h | 8 ++-- 6 files changed, 119 insertions(+), 55 deletions(-) diff --git a/src/parser/context-decls.cpp b/src/parser/context-decls.cpp index 0298f1cee9a..2174b1bad83 100644 --- a/src/parser/context-decls.cpp +++ b/src/parser/context-decls.cpp @@ -71,13 +71,14 @@ Result<> ParseDeclsCtx::addFunc(Name name, Exactness exact, std::optional, std::vector&& annotations, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto f = addFuncDecl(pos, name, import); CHECK_ERR(f); CHECK_ERR(addExports(in, wasm, *f, exports, ExternalKind::Function)); funcDefs.push_back( - {name, pos, Index(funcDefs.size()), std::move(annotations)}); + {name, pos, Index(funcDefs.size()), std::move(annotations), kind}); return Ok{}; } @@ -110,13 +111,14 @@ Result<> ParseDeclsCtx::addTable(Name name, ImportNames* import, TableType type, std::optional, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto t = addTableDecl(pos, name, import, type); CHECK_ERR(t); CHECK_ERR(addExports(in, wasm, *t, exports, ExternalKind::Table)); // TODO: table annotations - tableDefs.push_back({name, pos, Index(tableDefs.size()), {}}); + tableDefs.push_back({name, pos, Index(tableDefs.size()), {}, kind}); return Ok{}; } @@ -167,13 +169,14 @@ Result<> ParseDeclsCtx::addMemory(Name name, const std::vector& exports, ImportNames* import, MemType type, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto m = addMemoryDecl(pos, name, import, type); CHECK_ERR(m); CHECK_ERR(addExports(in, wasm, *m, exports, ExternalKind::Memory)); // TODO: memory annotations - memoryDefs.push_back({name, pos, Index(memoryDefs.size()), {}}); + memoryDefs.push_back({name, pos, Index(memoryDefs.size()), {}, kind}); return Ok{}; } @@ -213,13 +216,14 @@ Result<> ParseDeclsCtx::addGlobal(Name name, ImportNames* import, GlobalTypeT, std::optional, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto g = addGlobalDecl(pos, name, import); CHECK_ERR(g); CHECK_ERR(addExports(in, wasm, *g, exports, ExternalKind::Global)); // TODO: global annotations - globalDefs.push_back({name, pos, Index(globalDefs.size()), {}}); + globalDefs.push_back({name, pos, Index(globalDefs.size()), {}, kind}); return Ok{}; } @@ -239,7 +243,8 @@ Result<> ParseDeclsCtx::addElem( e->name = name; } // TODO: element segment annotations - elemDefs.push_back({name, pos, Index(wasm.elementSegments.size()), {}}); + elemDefs.push_back( + {name, pos, Index(wasm.elementSegments.size()), {}, DefKind::Definition}); wasm.addElementSegment(std::move(e)); return Ok{}; } @@ -264,7 +269,8 @@ Result<> ParseDeclsCtx::addData(Name name, } d->data = std::move(data); // TODO: data segment annotations - dataDefs.push_back({name, pos, Index(wasm.dataSegments.size()), {}}); + dataDefs.push_back( + {name, pos, Index(wasm.dataSegments.size()), {}, DefKind::Definition}); wasm.addDataSegment(std::move(d)); return Ok{}; } @@ -292,13 +298,14 @@ Result<> ParseDeclsCtx::addTag(Name name, const std::vector& exports, ImportNames* import, TypeUseT type, - Index pos) { + Index pos, + DefKind kind) { CHECK_ERR(checkImport(pos, import)); auto t = addTagDecl(pos, name, import); CHECK_ERR(t); CHECK_ERR(addExports(in, wasm, *t, exports, ExternalKind::Tag)); // TODO: tag annotations - tagDefs.push_back({name, pos, Index(tagDefs.size()), {}}); + tagDefs.push_back({name, pos, Index(tagDefs.size()), {}, kind}); return Ok{}; } diff --git a/src/parser/context-defs.cpp b/src/parser/context-defs.cpp index d0d170ed2c5..f191ea638a9 100644 --- a/src/parser/context-defs.cpp +++ b/src/parser/context-defs.cpp @@ -55,7 +55,8 @@ Result<> ParseDefsCtx::addGlobal(Name, ImportNames*, GlobalTypeT, std::optional exp, - Index) { + Index, + DefKind) { if (exp) { wasm.globals[index]->init = *exp; } @@ -67,7 +68,8 @@ Result<> ParseDefsCtx::addTable(Name, ImportNames*, TableTypeT, std::optional init, - Index) { + Index, + DefKind) { if (init) { wasm.tables[index]->init = *init; } diff --git a/src/parser/contexts.h b/src/parser/contexts.h index 31a3aa253e3..0a403cf1073 100644 --- a/src/parser/contexts.h +++ b/src/parser/contexts.h @@ -63,6 +63,11 @@ struct TableType { Limits limits; }; +enum class DefKind { + ImportDesc, + Definition, +}; + // The location, possible name, and index in the respective module index space // of a module-level definition in the input. struct DefPos { @@ -70,6 +75,7 @@ struct DefPos { Index pos; Index index; std::vector annotations; + DefKind kind; }; struct GlobalType { @@ -1074,13 +1080,15 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { void setSupertype(HeapTypeT) {} void finishTypeDef(Name name, Index pos) { // TODO: type annotations - typeDefs.push_back({name, pos, Index(typeDefs.size()), {}}); + typeDefs.push_back( + {name, pos, Index(typeDefs.size()), {}, DefKind::Definition}); } size_t getRecGroupStartIndex() { return 0; } void addRecGroup(Index, size_t) {} void finishRectype(Index pos) { // TODO: type annotations - recTypeDefs.push_back({{}, pos, Index(recTypeDefs.size()), {}}); + recTypeDefs.push_back( + {{}, pos, Index(recTypeDefs.size()), {}, DefKind::Definition}); } bool skipFunctionBody(); @@ -1135,7 +1143,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { Exactness exact, std::optional, std::vector&&, - Index pos); + Index pos, + DefKind kind); Result addTableDecl(Index pos, Name name, @@ -1146,7 +1155,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { ImportNames*, TableType, std::optional, - Index); + Index, + DefKind kind); // TODO: Record index of implicit elem for use when parsing types and instrs. Result<> addImplicitElems(TypeT, ElemListT&& elems); @@ -1158,7 +1168,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { const std::vector& exports, ImportNames* import, MemType type, - Index pos); + Index pos, + DefKind kind); Result<> addImplicitData(DataStringT&& data); @@ -1169,14 +1180,15 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { ImportNames* import, GlobalTypeT, std::optional, - Index pos); + Index pos, + DefKind kind); Result<> addStart(FuncIdxT, Index pos) { if (!startDefs.empty()) { return Err{"unexpected extra 'start' function"}; } // TODO: start function annotations. - startDefs.push_back({{}, pos, 0, {}}); + startDefs.push_back({{}, pos, 0, {}, DefKind::Definition}); return Ok{}; } @@ -1196,7 +1208,8 @@ struct ParseDeclsCtx : NullTypeParserCtx, NullInstrParserCtx { const std::vector& exports, ImportNames* import, TypeUseT type, - Index pos); + Index pos, + DefKind kind); Result<> addExport(Index pos, Ok, Name, ExternalKind) { exportDefs.push_back(pos); @@ -1526,7 +1539,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, Exactness exact, std::optional locals, std::vector&& annotations, - Index pos) { + Index pos, + DefKind kind) { auto& f = wasm.functions[index]; if (!type.type.isSignature()) { return in.err(pos, "expected signature type"); @@ -1556,7 +1570,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, ImportNames*, Type ttype, std::optional init, - Index pos) { + Index pos, + DefKind kind) { auto& t = wasm.tables[index]; if (!ttype.isRef()) { return in.err(pos, "expected reference type"); @@ -1572,8 +1587,12 @@ struct ParseModuleTypesCtx : TypeParserCtx, return Ok{}; } - Result<> - addMemory(Name, const std::vector&, ImportNames*, MemTypeT, Index) { + Result<> addMemory(Name, + const std::vector&, + ImportNames*, + MemTypeT, + Index, + DefKind kind) { return Ok{}; } @@ -1584,7 +1603,8 @@ struct ParseModuleTypesCtx : TypeParserCtx, ImportNames*, GlobalType type, std::optional, - Index) { + Index, + DefKind kind) { auto& g = wasm.globals[index]; g->mutable_ = type.mutability; g->type = type.type; @@ -1600,8 +1620,12 @@ struct ParseModuleTypesCtx : TypeParserCtx, Result<> addDeclareElem(Name, ElemListT&&, Index) { return Ok{}; } - Result<> - addTag(Name, const std::vector&, ImportNames*, TypeUse use, Index pos) { + Result<> addTag(Name, + const std::vector&, + ImportNames*, + TypeUse use, + Index pos, + DefKind kind) { auto& t = wasm.tags[index]; if (!use.type.isSignature()) { return in.err(pos, "tag type must be a signature"); @@ -1916,7 +1940,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Exactness, std::optional, std::vector&&, - Index) { + Index, + DefKind) { return Ok{}; } @@ -1925,10 +1950,11 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { ImportNames*, TableTypeT, std::optional, - Index); + Index, + DefKind); - Result<> - addMemory(Name, const std::vector&, ImportNames*, TableTypeT, Index) { + Result<> addMemory( + Name, const std::vector&, ImportNames*, TableTypeT, Index, DefKind) { return Ok{}; } @@ -1937,7 +1963,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { ImportNames*, GlobalTypeT, std::optional exp, - Index); + Index, + DefKind); Result<> addStart(Name name, Index pos) { wasm.start = name; @@ -1961,8 +1988,8 @@ struct ParseDefsCtx : TypeParserCtx, AnnotationParserCtx { Result<> addData(Name, Name* mem, std::optional offset, DataStringT, Index pos); - Result<> - addTag(Name, const std::vector, ImportNames*, TypeUseT, Index) { + Result<> addTag( + Name, const std::vector, ImportNames*, TypeUseT, Index, DefKind) { return Ok{}; } diff --git a/src/parser/parse-5-defs.cpp b/src/parser/parse-5-defs.cpp index 82909ea1a16..14ad9ffcdd5 100644 --- a/src/parser/parse-5-defs.cpp +++ b/src/parser/parse-5-defs.cpp @@ -49,12 +49,14 @@ Result<> parseDefinitions( if (!f->imported()) { CHECK_ERR(ctx.visitFunctionStart(f)); } - if (auto parsed = func(ctx)) { - CHECK_ERR(parsed); - } else { + if (decls.funcDefs[i].kind == DefKind::ImportDesc) { auto im = import_(ctx); assert(im); CHECK_ERR(im); + } else { + auto parsed = func(ctx); + assert(parsed); + CHECK_ERR(parsed); } if (!f->imported()) { auto end = ctx.irBuilder.visitEnd(); diff --git a/src/parser/parsers.h b/src/parser/parsers.h index d6f2297ff57..8c43fb3e30c 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -3454,30 +3454,49 @@ template MaybeResult<> import_(Ctx& ctx) { CHECK_ERR(use); auto [type, exact] = *use; // TODO: function import annotations - CHECK_ERR(ctx.addFunc( - name ? *name : Name{}, {}, &names, type, exact, std::nullopt, {}, pos)); + CHECK_ERR(ctx.addFunc(name ? *name : Name{}, + {}, + &names, + type, + exact, + std::nullopt, + {}, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("table"sv)) { auto name = ctx.in.takeID(); auto type = tabletype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTable( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addTable(name ? *name : Name{}, + {}, + &names, + *type, + std::nullopt, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("memory"sv)) { auto name = ctx.in.takeID(); auto type = memtype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addMemory(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addMemory( + name ? *name : Name{}, {}, &names, *type, pos, DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("global"sv)) { auto name = ctx.in.takeID(); auto type = globaltype(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addGlobal( - name ? *name : Name{}, {}, &names, *type, std::nullopt, pos)); + CHECK_ERR(ctx.addGlobal(name ? *name : Name{}, + {}, + &names, + *type, + std::nullopt, + pos, + DefKind::ImportDesc)); } else if (ctx.in.takeSExprStart("tag"sv)) { auto name = ctx.in.takeID(); auto type = typeuse(ctx); CHECK_ERR(type); - CHECK_ERR(ctx.addTag(name ? *name : Name{}, {}, &names, *type, pos)); + CHECK_ERR(ctx.addTag( + name ? *name : Name{}, {}, &names, *type, pos, DefKind::ImportDesc)); } else { return ctx.in.err("expected import description"); } @@ -3551,7 +3570,8 @@ template MaybeResult<> func(Ctx& ctx) { exact, localVars, std::move(annotations), - pos)); + pos, + DefKind::Definition)); return Ok{}; } @@ -3640,7 +3660,8 @@ template MaybeResult<> table(Ctx& ctx) { return ctx.in.err("expected end of table declaration"); } - CHECK_ERR(ctx.addTable(name, *exports, import.getPtr(), *ttype, init, pos)); + CHECK_ERR(ctx.addTable( + name, *exports, import.getPtr(), *ttype, init, pos, DefKind::Definition)); if (elems) { CHECK_ERR(ctx.addImplicitElems(*type, std::move(*elems))); @@ -3711,7 +3732,8 @@ template MaybeResult<> memory(Ctx& ctx) { return ctx.in.err("expected end of memory declaration"); } - CHECK_ERR(ctx.addMemory(name, *exports, import.getPtr(), *mtype, pos)); + CHECK_ERR(ctx.addMemory( + name, *exports, import.getPtr(), *mtype, pos, DefKind::Definition)); if (data) { CHECK_ERR(ctx.addImplicitData(std::move(*data))); @@ -3754,7 +3776,8 @@ template MaybeResult<> global(Ctx& ctx) { return ctx.in.err("expected end of global"); } - CHECK_ERR(ctx.addGlobal(name, *exports, import.getPtr(), *type, exp, pos)); + CHECK_ERR(ctx.addGlobal( + name, *exports, import.getPtr(), *type, exp, pos, DefKind::Definition)); return Ok{}; } @@ -4027,7 +4050,8 @@ template MaybeResult<> tag(Ctx& ctx) { return ctx.in.err("expected end of tag"); } - CHECK_ERR(ctx.addTag(name, *exports, import.getPtr(), *type, pos)); + CHECK_ERR(ctx.addTag( + name, *exports, import.getPtr(), *type, pos, DefKind::Definition)); return Ok{}; } diff --git a/src/parser/wat-parser-internal.h b/src/parser/wat-parser-internal.h index 2f4ecc7c3e1..9219b62012c 100644 --- a/src/parser/wat-parser-internal.h +++ b/src/parser/wat-parser-internal.h @@ -81,12 +81,14 @@ Result<> parseDefs(Ctx& ctx, ctx.index = def.index; WithPosition with(ctx, def.pos); ctx.in.setAnnotations(def.annotations); - if (auto parsed = parser(ctx)) { - CHECK_ERR(parsed); - } else { + if (def.kind == DefKind::ImportDesc) { auto im = import_(ctx); assert(im); CHECK_ERR(im); + } else { + auto parsed = parser(ctx); + assert(parsed); + CHECK_ERR(parsed); } } return Ok{}; From 8b852342dad2efeb5460c4ff6d937fff8de76194 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jul 2026 18:00:06 -0700 Subject: [PATCH 04/15] disable compact imports in fuzz_shell test --- test/lit/d8/fuzz_shell_exceptions.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/d8/fuzz_shell_exceptions.wast b/test/lit/d8/fuzz_shell_exceptions.wast index f4d5d082d85..ae516c12ba0 100644 --- a/test/lit/d8/fuzz_shell_exceptions.wast +++ b/test/lit/d8/fuzz_shell_exceptions.wast @@ -37,7 +37,7 @@ ;; Build to a binary wasm. ;; -;; RUN: wasm-opt %s -o %t.wasm -q -all +;; RUN: wasm-opt %s -o %t.wasm -q -all --disable-compact-imports ;; Run in node. ;; From 3872ec11221882e4c73126520ae02430f5ff1a09 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 22 Jul 2026 18:49:23 -0700 Subject: [PATCH 05/15] update --- src/wasm/wasm-binary.cpp | 117 ++++++++++-------- test/lit/basic/compact-imports-roundtrip.wast | 50 -------- test/lit/basic/compact-imports.wast | 35 ++++++ 3 files changed, 98 insertions(+), 104 deletions(-) delete mode 100644 test/lit/basic/compact-imports-roundtrip.wast diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 95b68027085..381ed134a49 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -372,8 +372,7 @@ void WasmBinaryWriter::writeImports() { if (const auto* fa = std::get_if(&a)) { auto* fb = std::get(b); return (*fa)->type.isExact() == fb->type.isExact() && - getTypeIndex((*fa)->type.getHeapType()) == - getTypeIndex(fb->type.getHeapType()); + (*fa)->type.getHeapType() == fb->type.getHeapType(); } if (const auto* ga = std::get_if(&a)) { auto* gb = std::get(b); @@ -381,7 +380,7 @@ void WasmBinaryWriter::writeImports() { } if (const auto* ta = std::get_if(&a)) { auto* tb = std::get(b); - return getTypeIndex((*ta)->type) == getTypeIndex(tb->type); + return (*ta)->type == tb->type; } if (const auto* ma = std::get_if(&a)) { auto* mb = std::get(b); @@ -408,31 +407,34 @@ void WasmBinaryWriter::writeImports() { std::vector groups; if (wasm->features.hasCompactImports()) { size_t i = 0; - size_t N = imports.size(); - while (i < N) { - if (i + 1 < N && getModule(imports[i]) == getModule(imports[i + 1]) && - shareImportType(imports[i], imports[i + 1])) { - size_t j = i + 1; - while (j + 1 < N && - getModule(imports[i]) == getModule(imports[j + 1]) && - shareImportType(imports[i], imports[j + 1])) { - j++; - } - groups.push_back({ImportGroup::SharedAll, i, j - i + 1}); - i = j + 1; - } else if (i + 1 < N && - getModule(imports[i]) == getModule(imports[i + 1])) { - size_t j = i + 1; - while (j + 1 < N && - getModule(imports[i]) == getModule(imports[j + 1])) { - j++; - } - groups.push_back({ImportGroup::SharedModule, i, j - i + 1}); - i = j + 1; - } else { - groups.push_back({ImportGroup::Single, i, 1}); - i++; + size_t numImports = imports.size(); + while (i < numImports) { + // If the next import shares the module and type, then greedily collect + // the following imports as long as they share both the module and type. + size_t run = 1; + while (i + run < numImports && + getModule(imports[i]) == getModule(imports[i + run]) && + shareImportType(imports[i], imports[i + run])) { + ++run; + } + if (run > 1) { + groups.push_back({ImportGroup::SharedAll, i, run}); + i += run; + continue; } + // Otherwise, try greedily collecting imports that share just the module. + while (i + run < numImports && + getModule(imports[i]) == getModule(imports[i + run])) { + ++run; + } + if (run > 1) { + groups.push_back({ImportGroup::SharedModule, i, run}); + i += run; + continue; + } + // Otherwise, just use a normal import. + groups.push_back({ImportGroup::Single, i, 1}); + ++i; } } else { for (size_t i = 0; i < imports.size(); ++i) { @@ -442,7 +444,7 @@ void WasmBinaryWriter::writeImports() { o << U32LEB(groups.size()); - auto writeImportDetails = [&](const ImportItem& item) { + auto writeImportDesc = [&](const ImportItem& item) { std::visit(Overloaded{[&](Function* func) { uint32_t kind = ExternalKind::Function; if (func->type.isExact()) { @@ -458,8 +460,8 @@ void WasmBinaryWriter::writeImports() { }, [&](Tag* tag) { o << U32LEB(int32_t(ExternalKind::Tag)); - o << uint8_t( - 0); // Reserved 'attribute' field. Always 0. + // Reserved 'attribute' field. Always 0. + o << uint8_t(0); o << U32LEB(getTypeIndex(tag->type)); }, [&](Memory* memory) { @@ -484,31 +486,38 @@ void WasmBinaryWriter::writeImports() { }; for (const auto& group : groups) { - if (group.kind == ImportGroup::Single) { - const auto& item = imports[group.start]; - writeInlineString(getModule(item).view()); - writeInlineString(getBase(item).view()); - writeImportDetails(item); - } else if (group.kind == ImportGroup::SharedAll) { - const auto& item0 = imports[group.start]; - writeInlineString(getModule(item0).view()); - writeInlineString(""); - o << uint8_t(BinaryConsts::CompactImportsSharedAll); - writeImportDetails(item0); - o << U32LEB(group.count); - for (size_t k = 0; k < group.count; ++k) { - writeInlineString(getBase(imports[group.start + k]).view()); - } - } else if (group.kind == ImportGroup::SharedModule) { - const auto& item0 = imports[group.start]; - writeInlineString(getModule(item0).view()); - writeInlineString(""); - o << uint8_t(BinaryConsts::CompactImportsSharedModule); - o << U32LEB(group.count); - for (size_t k = 0; k < group.count; ++k) { - const auto& item = imports[group.start + k]; + switch (group.kind) { + case ImportGroup::Single: { + const auto& item = imports[group.start]; + writeInlineString(getModule(item).view()); writeInlineString(getBase(item).view()); - writeImportDetails(item); + writeImportDesc(item); + continue; + } + case ImportGroup::SharedAll: { + const auto& first = imports[group.start]; + writeInlineString(getModule(first).view()); + writeInlineString(""); + o << uint8_t(BinaryConsts::CompactImportsSharedAll); + writeImportDesc(first); + o << U32LEB(group.count); + for (size_t i = 0; i < group.count; ++i) { + writeInlineString(getBase(imports[group.start + i]).view()); + } + continue; + } + case ImportGroup::SharedModule: { + const auto& first = imports[group.start]; + writeInlineString(getModule(first).view()); + writeInlineString(""); + o << uint8_t(BinaryConsts::CompactImportsSharedModule); + o << U32LEB(group.count); + for (size_t i = 0; i < group.count; ++i) { + const auto& item = imports[group.start + i]; + writeInlineString(getBase(item).view()); + writeImportDesc(item); + } + continue; } } } diff --git a/test/lit/basic/compact-imports-roundtrip.wast b/test/lit/basic/compact-imports-roundtrip.wast deleted file mode 100644 index bb5db734ae0..00000000000 --- a/test/lit/basic/compact-imports-roundtrip.wast +++ /dev/null @@ -1,50 +0,0 @@ -;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: wasm-opt %s -all -o %t.wasm && wasm-opt %t.wasm -all -S -o - | filecheck %s - -(module - (type $sig1 (func (param i32) (result i32))) - (type $sig2 (func (param f64) (result f64))) - - ;; SharedAll functions: same module "env", same signature type $sig1 - (import "env" "f1" (func $f1 (type $sig1))) - (import "env" "f2" (func $f2 (type $sig1))) - - ;; SharedModule functions: same module "math", different signature types - (import "math" "sin" (func $sin (type $sig1))) - (import "math" "sqrt" (func $sqrt (type $sig2))) - - ;; SharedAll globals: same module "consts", same type & mutability - (import "consts" "g1" (global $g1 i32)) - (import "consts" "g2" (global $g2 i32)) - - ;; SharedModule mixed: same module "mixed", different import kinds - (import "mixed" "g3" (global $g3 (mut i32))) - (import "mixed" "t1" (table $t1 1 10 funcref)) - - ;; Single import: unique module "single" - (import "single" "m1" (memory $m1 1 2)) - - (func $main (result i32) - (call $f1 (i32.const 1)) - ) -) - -;; CHECK: (type $0 (func (param i32) (result i32))) -;; CHECK-NEXT: (type $1 (func (param f64) (result f64))) -;; CHECK-NEXT: (type $2 (func (result i32))) - -;; CHECK: (import "single" "m1" (memory $mimport$0 1 2)) -;; CHECK-NEXT: (import "mixed" "t1" (table $timport$0 1 10 funcref)) -;; CHECK-NEXT: (import "consts" "g1" (global $gimport$0 i32)) -;; CHECK-NEXT: (import "consts" "g2" (global $gimport$1 i32)) -;; CHECK-NEXT: (import "mixed" "g3" (global $gimport$2 (mut i32))) -;; CHECK-NEXT: (import "env" "f1" (func $fimport$0 (type $0) (param i32) (result i32))) -;; CHECK-NEXT: (import "env" "f2" (func $fimport$1 (type $0) (param i32) (result i32))) -;; CHECK-NEXT: (import "math" "sin" (func $fimport$2 (type $0) (param i32) (result i32))) -;; CHECK-NEXT: (import "math" "sqrt" (func $fimport$3 (type $1) (param f64) (result f64))) - -;; CHECK: (func $0 (type $2) (result i32) -;; CHECK-NEXT: (call $fimport$0 -;; CHECK-NEXT: (i32.const 1) -;; CHECK-NEXT: ) -;; CHECK-NEXT: ) diff --git a/test/lit/basic/compact-imports.wast b/test/lit/basic/compact-imports.wast index 89fd1003068..e92518d9c08 100644 --- a/test/lit/basic/compact-imports.wast +++ b/test/lit/basic/compact-imports.wast @@ -1,9 +1,11 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. ;; RUN: wasm-opt %s -all -S -o - | filecheck %s +;; RUN: wasm-opt %s -all --roundtrip -S -o - | filecheck %s --check-prefix=RTRIP (module ;; CHECK: (type $sig1 (func (param i32) (result i32))) + ;; RTRIP: (type $sig1 (func (param i32) (result i32))) (type $sig1 (func (param i32) (result i32))) ;; Compact Encoding 1: per-item import descriptions @@ -71,6 +73,39 @@ ;; CHECK-NEXT: (i32.const 1) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) + ;; RTRIP: (type $1 (func (result i32))) + + ;; RTRIP: (import "env" "m1" (memory $m1 1 2)) + + ;; RTRIP: (import "env" "t1" (table $t1 1 10 funcref)) + + ;; RTRIP: (import "env" "g1" (global $g1 i32)) + + ;; RTRIP: (import "constants" "pi" (global $pi f64)) + + ;; RTRIP: (import "constants" "e" (global $e f64)) + + ;; RTRIP: (import "env" "f1" (func $f1 (type $sig1) (param i32) (result i32))) + + ;; RTRIP: (import "env" "f2" (func $f2 (exact (type $sig1) (param i32) (result i32)))) + + ;; RTRIP: (import "math" "sin" (func $sin (type $sig1) (param i32) (result i32))) + + ;; RTRIP: (import "math" "cos" (func $cos (type $sig1) (param i32) (result i32))) + + ;; RTRIP: (import "math" "tan" (func $tan (type $sig1) (param i32) (result i32))) + + ;; RTRIP: (import "math" "sinh" (func $sinh (exact (type $sig1) (param i32) (result i32)))) + + ;; RTRIP: (import "math" "cosh" (func $cosh (exact (type $sig1) (param i32) (result i32)))) + + ;; RTRIP: (import "math" "tanh" (func $tanh (exact (type $sig1) (param i32) (result i32)))) + + ;; RTRIP: (func $main (type $1) (result i32) + ;; RTRIP-NEXT: (call $f1 + ;; RTRIP-NEXT: (i32.const 1) + ;; RTRIP-NEXT: ) + ;; RTRIP-NEXT: ) (func $main (result i32) (call $f1 (i32.const 1)) ) From cb7e598d22cfbaf32a3467f582fde26e397feb3c Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 23 Jul 2026 14:29:51 -0700 Subject: [PATCH 06/15] address feedback --- src/ir/memory-utils.h | 4 ++++ src/ir/table-utils.h | 5 ++++ src/wasm/wasm-binary.cpp | 33 +++++++------------------- test/lit/d8/fuzz_shell_exceptions.wast | 4 ++-- test/unit/test_compact_imports.py | 16 ++++++++++++- 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/ir/memory-utils.h b/src/ir/memory-utils.h index db9ff2bcba8..a586109e809 100644 --- a/src/ir/memory-utils.h +++ b/src/ir/memory-utils.h @@ -30,6 +30,10 @@ namespace wasm::MemoryUtils { bool isSubType(const Memory& a, const Memory& b); +inline bool sameType(const Memory& a, const Memory& b) { + return isSubType(a, b) && isSubType(b, a); +} + // Flattens memory into a single data segment, or no segment. If there is // a segment, it starts at 0. // Returns true if successful (e.g. relocatable segments cannot be flattened). diff --git a/src/ir/table-utils.h b/src/ir/table-utils.h index f5473a00012..bf1e7d663f0 100644 --- a/src/ir/table-utils.h +++ b/src/ir/table-utils.h @@ -25,6 +25,11 @@ namespace wasm::TableUtils { +inline bool sameType(const Table& a, const Table& b) { + return a.type == b.type && a.initial == b.initial && a.max == b.max && + a.addressType == b.addressType; +} + struct FlatTable { std::vector names; bool valid; diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 6b8bbd61974..4c97753c16f 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -18,6 +18,7 @@ #include #include +#include "ir/memory-utils.h" #include "ir/module-utils.h" #include "ir/names.h" #include "ir/table-utils.h" @@ -26,6 +27,7 @@ #include "support/bits.h" #include "support/stdckdint.h" #include "support/string.h" +#include "support/utilities.h" #include "wasm-annotations.h" #include "wasm-binary.h" #include "wasm-debug.h" @@ -331,11 +333,6 @@ void WasmBinaryWriter::writeTypes() { finishSection(start); } -template struct Overloaded : Ts... { - using Ts::operator()...; -}; -template Overloaded(Ts...) -> Overloaded; - void WasmBinaryWriter::writeImports() { auto num = importInfo->getNumImports(); if (num == 0) { @@ -347,16 +344,8 @@ void WasmBinaryWriter::writeImports() { std::vector imports; imports.reserve(num); - ModuleUtils::iterImportedFunctions( - *wasm, [&](Function* func) { imports.push_back(func); }); - ModuleUtils::iterImportedGlobals( - *wasm, [&](Global* global) { imports.push_back(global); }); - ModuleUtils::iterImportedTags(*wasm, - [&](Tag* tag) { imports.push_back(tag); }); - ModuleUtils::iterImportedMemories( - *wasm, [&](Memory* memory) { imports.push_back(memory); }); - ModuleUtils::iterImportedTables( - *wasm, [&](Table* table) { imports.push_back(table); }); + ModuleUtils::iterImports(*wasm, + [&](ImportItem item) { imports.push_back(item); }); auto getModule = [](const ImportItem& item) -> Name { return std::visit([](auto* i) { return i->module; }, item); @@ -371,8 +360,7 @@ void WasmBinaryWriter::writeImports() { } if (const auto* fa = std::get_if(&a)) { auto* fb = std::get(b); - return (*fa)->type.isExact() == fb->type.isExact() && - (*fa)->type.getHeapType() == fb->type.getHeapType(); + return (*fa)->type == fb->type; } if (const auto* ga = std::get_if(&a)) { auto* gb = std::get(b); @@ -384,16 +372,11 @@ void WasmBinaryWriter::writeImports() { } if (const auto* ma = std::get_if(&a)) { auto* mb = std::get(b); - return (*ma)->initial == mb->initial && (*ma)->max == mb->max && - (*ma)->hasMax() == mb->hasMax() && (*ma)->shared == mb->shared && - (*ma)->is64() == mb->is64() && - (*ma)->pageSizeLog2 == mb->pageSizeLog2; + return MemoryUtils::sameType(**ma, *mb); } if (const auto* ta = std::get_if(&a)) { auto* tb = std::get(b); - return (*ta)->type == tb->type && (*ta)->initial == tb->initial && - (*ta)->max == tb->max && (*ta)->hasMax() == tb->hasMax() && - (*ta)->is64() == tb->is64(); + return TableUtils::sameType(**ta, *tb); } return false; }; @@ -445,7 +428,7 @@ void WasmBinaryWriter::writeImports() { o << U32LEB(groups.size()); auto writeImportDesc = [&](const ImportItem& item) { - std::visit(Overloaded{[&](Function* func) { + std::visit(overloaded{[&](Function* func) { uint32_t kind = ExternalKind::Function; if (func->type.isExact()) { kind |= BinaryConsts::ExactImport; diff --git a/test/lit/d8/fuzz_shell_exceptions.wast b/test/lit/d8/fuzz_shell_exceptions.wast index ae516c12ba0..dcf576a914f 100644 --- a/test/lit/d8/fuzz_shell_exceptions.wast +++ b/test/lit/d8/fuzz_shell_exceptions.wast @@ -37,11 +37,11 @@ ;; Build to a binary wasm. ;; -;; RUN: wasm-opt %s -o %t.wasm -q -all --disable-compact-imports +;; RUN: wasm-opt %s -o %t.wasm -q -all ;; Run in node. ;; -;; RUN: v8 %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s +;; RUN: v8 --wasm-compact-imports %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s ;; ;; CHECK: [fuzz-exec] export throwing-js ;; CHECK: exception thrown: Error: js exception diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py index 56ffcbf9b45..552cefa3625 100644 --- a/test/unit/test_compact_imports.py +++ b/test/unit/test_compact_imports.py @@ -6,7 +6,9 @@ class CompactImportsTest(utils.BinaryenTestCase): def get_binary(self, wat_str, flags=[]): cmd = shared.WASM_OPT + ['--print-features', '-o', '-'] + flags - p = shared.run_process(cmd, input=wat_str, check=True, capture_output=True) + p = shared.run_process( + cmd, input=wat_str, check=True, capture_output=True, decode_output=False + ) return p.stdout.encode('latin1') if isinstance(p.stdout, str) else p.stdout def test_shared_all_encoding(self): @@ -59,3 +61,15 @@ def test_mixed_import_patterns(self): self.assertIn(b'\x03env\x00\x7e', wasm_bytes) self.assertIn(b'\x04math\x00\x7f', wasm_bytes) self.assertIn(b'\x06single\x02m1', wasm_bytes) + + def test_identical_imports_size_reduction(self): + imports = '\n'.join(['(import "env" "f" (func (type $sig)))'] * 1000) + wat = f'''(module + (type $sig (func (param i32) (result i32))) + {imports} + )''' + with_compact = self.get_binary(wat, ['--enable-compact-imports']) + without_compact = self.get_binary(wat, ['--disable-compact-imports']) + self.assertEqual(len(with_compact), 2098) + self.assertEqual(len(without_compact), 8064) + From 180991565be9000c4e32f00febac6a0c941907e0 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 23 Jul 2026 15:15:06 -0700 Subject: [PATCH 07/15] remove incorrect assertion --- src/parser/wat-parser-internal.h | 1 - .../input/reference_types_target_feature.wasm | Bin 178 -> 178 bytes 2 files changed, 1 deletion(-) diff --git a/src/parser/wat-parser-internal.h b/src/parser/wat-parser-internal.h index 4317890f9fb..33528701be5 100644 --- a/src/parser/wat-parser-internal.h +++ b/src/parser/wat-parser-internal.h @@ -83,7 +83,6 @@ Result<> parseDefs(Ctx& ctx, ctx.in.setAnnotations(def.annotations); if (def.kind == DefKind::ImportDesc) { auto im = importdesc(ctx, Name{}, Name{}, std::nullopt); - assert(!im.getErr()); CHECK_ERR(im); } else { auto parsed = parser(ctx); diff --git a/test/unit/input/reference_types_target_feature.wasm b/test/unit/input/reference_types_target_feature.wasm index b388b11bb579b4341852e0244ffc4da232ff4114..95338c95b3f32c18eee5635488764c8f143b5a7b 100644 GIT binary patch delta 25 gcmdnQxQTIs48IX`J_B=VUKwjiYH^7n!$j?N09(KZO8@`> delta 25 gcmdnQxQTIs48I`*19NI#8EZ*uafuOg{zUC|09l>~O8@`> From 0d52404614da664e7abb06c932f7a2b4a06a202d Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 23 Jul 2026 15:26:17 -0700 Subject: [PATCH 08/15] try to fix build --- src/parser/parsers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/parsers.h b/src/parser/parsers.h index 861d2c5cf46..4eadcd01ba4 100644 --- a/src/parser/parsers.h +++ b/src/parser/parsers.h @@ -3546,7 +3546,7 @@ template MaybeResult<> import_(Ctx& ctx) { } if (*hasSharedImportDesc) { - items.emplace_back(*id, *nm); + items.push_back(CompactItem{*id, *nm}); } else { CHECK_ERR(importdesc(ctx, *mod, *nm, id)) } From b787866121b6c4e741cfb8341d4b6c35113b5729 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 23 Jul 2026 15:52:51 -0700 Subject: [PATCH 09/15] fix ruff errors --- test/unit/test_compact_imports.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py index 552cefa3625..d47afdce628 100644 --- a/test/unit/test_compact_imports.py +++ b/test/unit/test_compact_imports.py @@ -1,5 +1,5 @@ -import os from scripts.test import shared + from . import utils @@ -7,7 +7,7 @@ class CompactImportsTest(utils.BinaryenTestCase): def get_binary(self, wat_str, flags=[]): cmd = shared.WASM_OPT + ['--print-features', '-o', '-'] + flags p = shared.run_process( - cmd, input=wat_str, check=True, capture_output=True, decode_output=False + cmd, input=wat_str, check=True, capture_output=True, decode_output=False, ) return p.stdout.encode('latin1') if isinstance(p.stdout, str) else p.stdout @@ -72,4 +72,3 @@ def test_identical_imports_size_reduction(self): without_compact = self.get_binary(wat, ['--disable-compact-imports']) self.assertEqual(len(with_compact), 2098) self.assertEqual(len(without_compact), 8064) - From 3e565b310bc58b897d381a3f7b0991b464b145e2 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 23 Jul 2026 18:11:01 -0700 Subject: [PATCH 10/15] fix tests --- test/unit/test_compact_imports.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py index d47afdce628..1f6f0ebf2ca 100644 --- a/test/unit/test_compact_imports.py +++ b/test/unit/test_compact_imports.py @@ -5,11 +5,11 @@ class CompactImportsTest(utils.BinaryenTestCase): def get_binary(self, wat_str, flags=[]): - cmd = shared.WASM_OPT + ['--print-features', '-o', '-'] + flags + cmd = shared.WASM_OPT + ['-o', '-'] + flags p = shared.run_process( cmd, input=wat_str, check=True, capture_output=True, decode_output=False, ) - return p.stdout.encode('latin1') if isinstance(p.stdout, str) else p.stdout + return p.stdout def test_shared_all_encoding(self): wat = '''(module @@ -70,5 +70,5 @@ def test_identical_imports_size_reduction(self): )''' with_compact = self.get_binary(wat, ['--enable-compact-imports']) without_compact = self.get_binary(wat, ['--disable-compact-imports']) - self.assertEqual(len(with_compact), 2098) - self.assertEqual(len(without_compact), 8064) + self.assertEqual(len(with_compact), 2030) + self.assertEqual(len(without_compact), 8021) From 73835f5118ca87c8b6035b531cd9d2fa32aa3203 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 24 Jul 2026 13:52:35 -0700 Subject: [PATCH 11/15] more std::visit --- src/wasm/wasm-binary.cpp | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index eceb77bd0a8..625ee72a75a 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -355,30 +355,18 @@ void WasmBinaryWriter::writeImports() { }; auto shareImportType = [&](const ImportItem& a, const ImportItem& b) -> bool { - if (a.index() != b.index()) { - return false; - } - if (const auto* fa = std::get_if(&a)) { - auto* fb = std::get(b); - return (*fa)->type == fb->type; - } - if (const auto* ga = std::get_if(&a)) { - auto* gb = std::get(b); - return (*ga)->type == gb->type && (*ga)->mutable_ == gb->mutable_; - } - if (const auto* ta = std::get_if(&a)) { - auto* tb = std::get(b); - return (*ta)->type == tb->type; - } - if (const auto* ma = std::get_if(&a)) { - auto* mb = std::get(b); - return MemoryUtils::sameType(**ma, *mb); - } - if (const auto* ta = std::get_if(&a)) { - auto* tb = std::get(b); - return TableUtils::sameType(**ta, *tb); - } - return false; + return std::visit( + overloaded{ + [](Function* a, Function* b) { return a->type == b->type; }, + [](Global* a, Global* b) { + return a->type == b->type && a->mutable_ == b->mutable_; + }, + [](Tag* a, Tag* b) { return a->type == b->type; }, + [](Memory* a, Memory* b) { return MemoryUtils::sameType(*a, *b); }, + [](Table* a, Table* b) { return TableUtils::sameType(*a, *b); }, + [](const auto& a, const auto& b) { return false; }}, + a, + b); }; struct ImportGroup { From 3e4db28b02af39ca13a8512da71fd872d985e2cc Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 24 Jul 2026 13:54:22 -0700 Subject: [PATCH 12/15] no optional python list param --- test/unit/test_compact_imports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py index 1f6f0ebf2ca..c5275ee13c5 100644 --- a/test/unit/test_compact_imports.py +++ b/test/unit/test_compact_imports.py @@ -4,7 +4,7 @@ class CompactImportsTest(utils.BinaryenTestCase): - def get_binary(self, wat_str, flags=[]): + def get_binary(self, wat_str, flags): cmd = shared.WASM_OPT + ['-o', '-'] + flags p = shared.run_process( cmd, input=wat_str, check=True, capture_output=True, decode_output=False, From 1091343013cdbf672932bb799e3f69278ab74d4e Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 24 Jul 2026 13:59:54 -0700 Subject: [PATCH 13/15] continue => break --- src/wasm/wasm-binary.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp index 625ee72a75a..e03274d8428 100644 --- a/src/wasm/wasm-binary.cpp +++ b/src/wasm/wasm-binary.cpp @@ -463,7 +463,7 @@ void WasmBinaryWriter::writeImports() { writeInlineString(getModule(item).view()); writeInlineString(getBase(item).view()); writeImportDesc(item); - continue; + break; } case ImportGroup::SharedAll: { const auto& first = imports[group.start]; @@ -475,7 +475,7 @@ void WasmBinaryWriter::writeImports() { for (size_t i = 0; i < group.count; ++i) { writeInlineString(getBase(imports[group.start + i]).view()); } - continue; + break; } case ImportGroup::SharedModule: { const auto& first = imports[group.start]; @@ -488,7 +488,7 @@ void WasmBinaryWriter::writeImports() { writeInlineString(getBase(item).view()); writeImportDesc(item); } - continue; + break; } } } From a2b7f795d8b6b6ac5fbbac9ca11118e14250c0c9 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 24 Jul 2026 14:04:00 -0700 Subject: [PATCH 14/15] explicit features --- test/lit/d8/fuzz_shell_exceptions.wast | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/lit/d8/fuzz_shell_exceptions.wast b/test/lit/d8/fuzz_shell_exceptions.wast index dcf576a914f..b8d075c8e86 100644 --- a/test/lit/d8/fuzz_shell_exceptions.wast +++ b/test/lit/d8/fuzz_shell_exceptions.wast @@ -37,11 +37,11 @@ ;; Build to a binary wasm. ;; -;; RUN: wasm-opt %s -o %t.wasm -q -all +;; RUN: wasm-opt %s -o %t.wasm -q --enable-reference-types --enable-exception-handling ;; Run in node. ;; -;; RUN: v8 --wasm-compact-imports %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s +;; RUN: v8 %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s ;; ;; CHECK: [fuzz-exec] export throwing-js ;; CHECK: exception thrown: Error: js exception From 6309acc068eba5dc0c493f7bd1b11b61021e1405 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Fri, 24 Jul 2026 14:12:12 -0700 Subject: [PATCH 15/15] assert ratio --- test/unit/test_compact_imports.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/unit/test_compact_imports.py b/test/unit/test_compact_imports.py index c5275ee13c5..c759c0fc114 100644 --- a/test/unit/test_compact_imports.py +++ b/test/unit/test_compact_imports.py @@ -70,5 +70,4 @@ def test_identical_imports_size_reduction(self): )''' with_compact = self.get_binary(wat, ['--enable-compact-imports']) without_compact = self.get_binary(wat, ['--disable-compact-imports']) - self.assertEqual(len(with_compact), 2030) - self.assertEqual(len(without_compact), 8021) + self.assertTrue(len(without_compact) / len(with_compact) > 3.9)