Opportunisticly emit compact imports

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-acquire-release --experimental-wasm-wide-arithmetic'
+FUZZER_FLAGS = '--wasm-staging --experimental-wasm-custom-descriptors --experimental-wasm-js-interop --experimental-wasm-acquire-release --experimental-wasm-wide-arithmetic --wasm-compact-imports'
 
 # Optional V8 flags to add to FUZZER_FLAGS, some of the time.
 OPTIONAL_FUZZER_FLAGS = [

scripts/test/shared.py

@@ -268,6 +268,7 @@ def has_shell_timeout():
     '--experimental-wasm-js-interop',
     '--experimental-wasm-acquire-release',
     '--experimental-wasm-wide-arithmetic',
+    '--wasm-compact-imports',
 ]
 
 # external tools

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).

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<Name> names;
   bool valid;

src/parser/parsers.h

@@ -3546,7 +3546,7 @@ template<typename Ctx> 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))
     }

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);

src/wasm/wasm-binary.cpp

@@ -18,6 +18,7 @@
 #include <fstream>
 #include <optional>
 
+#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"
@@ -337,51 +339,172 @@ void WasmBinaryWriter::writeImports() {
     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<Function*, Global*, Tag*, Memory*, Table*>;
+  std::vector<ImportItem> imports;
+  imports.reserve(num);
+
+  ModuleUtils::iterImports(*wasm,
+                           [&](ImportItem item) { imports.push_back(item); });
+
+  auto getModule = [](const ImportItem& item) -> Name {
+    return std::visit([](auto* i) { return i->module; }, item);
   };
-  ModuleUtils::iterImportedFunctions(*wasm, [&](Function* func) {
-    writeImportHeader(func);
-    uint32_t kind = ExternalKind::Function;
-    if (func->type.isExact()) {
-      kind |= BinaryConsts::ExactImport;
+  auto getBase = [](const ImportItem& item) -> Name {
+    return std::visit([](auto* i) { return i->base; }, item);
+  };
+
+  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<Function*>(&a)) {
+      auto* fb = std::get<Function*>(b);
+      return (*fa)->type == fb->type;
+    }
+    if (const auto* ga = std::get_if<Global*>(&a)) {
+      auto* gb = std::get<Global*>(b);
+      return (*ga)->type == gb->type && (*ga)->mutable_ == gb->mutable_;
+    }
+    if (const auto* ta = std::get_if<Tag*>(&a)) {
+      auto* tb = std::get<Tag*>(b);
+      return (*ta)->type == tb->type;
+    }
+    if (const auto* ma = std::get_if<Memory*>(&a)) {
+      auto* mb = std::get<Memory*>(b);
+      return MemoryUtils::sameType(**ma, *mb);
+    }
+    if (const auto* ta = std::get_if<Table*>(&a)) {
+      auto* tb = std::get<Table*>(b);
+      return TableUtils::sameType(**ta, *tb);
+    }
+    return false;
+  };
+
+  struct ImportGroup {
+    enum Kind { Single, SharedAll, SharedModule } kind;
+    size_t start;
+    size_t count;
+  };
+
+  std::vector<ImportGroup> groups;
+  if (wasm->features.hasCompactImports()) {
+    size_t i = 0;
+    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) {
+      groups.push_back({ImportGroup::Single, i, 1});
+    }
+  }
+
+  o << U32LEB(groups.size());
+
+  auto writeImportDesc = [&](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));
+                            // Reserved 'attribute' field. Always 0.
+                            o << uint8_t(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) {
+    switch (group.kind) {
+      case ImportGroup::Single: {
+        const auto& item = imports[group.start];
+        writeInlineString(getModule(item).view());
+        writeInlineString(getBase(item).view());
+        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;
+      }
+    }
+  }
+
   finishSection(start);
 }
 

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))
   )

test/lit/d8/fuzz_shell_exceptions.wast

@@ -41,7 +41,7 @@
 
 ;; 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