fix

Author: kripken

src/binaryen-c.cpp

@@ -5754,6 +5754,14 @@ void BinaryenSetFlexibleInlineMaxSize(BinaryenIndex size) {
   globalPassOptions.inlining.flexibleInlineMaxSize = size;
 }
 
+BinaryenIndex BinaryenGetMaxCombinedBinarySize(void) {
+  return globalPassOptions.inlining.maxCombinedBinarySize;
+}
+
+void BinaryenSetMaxCombinedBinarySize(BinaryenIndex size) {
+  globalPassOptions.inlining.maxCombinedBinarySize = size;
+}
+
 BinaryenIndex BinaryenGetOneCallerInlineMaxSize(void) {
   return globalPassOptions.inlining.oneCallerInlineMaxSize;
 }

src/binaryen-c.h

@@ -3148,6 +3148,14 @@ BINARYEN_API BinaryenIndex BinaryenGetFlexibleInlineMaxSize(void);
 // Applies to all modules, globally.
 BINARYEN_API void BinaryenSetFlexibleInlineMaxSize(BinaryenIndex size);
 
+// Gets the limit for the combined size of the code after inlining.
+// Applies to all modules, globally.
+BINARYEN_API BinaryenIndex BinaryenGetMaxCombinedBinarySize(void);
+
+// Sets the limit for the combined size of the code after inlining.
+// Applies to all modules, globally.
+BINARYEN_API void BinaryenSetMaxCombinedBinarySize(BinaryenIndex size);
+
 // Gets the function size which we inline when there is only one caller.
 // Applies to all modules, globally.
 BINARYEN_API BinaryenIndex BinaryenGetOneCallerInlineMaxSize(void);

src/pass.h

@@ -89,6 +89,17 @@ struct InliningOptions {
   // This is checked after alwaysInlineMaxSize and oneCallerInlineMaxSize, but
   // the order normally won't matter.
   Index flexibleInlineMaxSize = 20;
+  // The limit for the combined size of the code after inlining.
+  // We have an absolute limit in order to avoid extremely-large sizes after
+  // inlining, as they may hit limits in VMs and/or slow down startup
+  // (measurements there indicate something like ~1 second to optimize a 100K
+  // function). See e.g.
+  // https://github.com/WebAssembly/binaryen/pull/3730#issuecomment-867939138
+  // https://github.com/emscripten-core/emscripten/issues/13899#issuecomment-825073344
+  // The limit is arbitrary, but based on the links above. It is a very high
+  // value that should appear very rarely in practice (for example, it does
+  // not occur on the Emscripten benchmark suite of real-world codebases).
+  Index maxCombinedBinarySize = 400 * 1024;
   // Loops usually mean the function does heavy work, so the call overhead
   // is not significant and we do not inline such functions by default.
   bool allowFunctionsWithLoops = false;

src/passes/Inlining.cpp

@@ -1501,21 +1501,13 @@ struct Inlining : public Pass {
   }
 
   // Checks if the combined size of the code after inlining is under the
-  // absolute size limit. We have an absolute limit in order to avoid
-  // extremely-large sizes after inlining, as they may hit limits in VMs and/or
-  // slow down startup (measurements there indicate something like ~1 second to
-  // optimize a 100K function). See e.g.
-  // https://github.com/WebAssembly/binaryen/pull/3730#issuecomment-867939138
-  // https://github.com/emscripten-core/emscripten/issues/13899#issuecomment-825073344
+  // absolute size limit.
   bool isUnderSizeLimit(Name target, Name source) {
     // Estimate the combined binary size from the number of instructions.
     auto combinedSize = infos[target].size + infos[source].size;
     auto estimatedBinarySize = Measurer::BytesPerExpr * combinedSize;
-    // The limit is arbitrary, but based on the links above. It is a very high
-    // value that should appear very rarely in practice (for example, it does
-    // not occur on the Emscripten benchmark suite of real-world codebases).
-    const Index MaxCombinedBinarySize = 400 * 1024;
-    return estimatedBinarySize < MaxCombinedBinarySize;
+    auto& options = getPassRunner()->options;
+    return estimatedBinarySize < options.inlining.maxCombinedBinarySize;
   }
 };
 

src/passes/RemoveUnusedModuleElements.cpp

@@ -190,6 +190,16 @@ struct Noter : public PostWalker<Noter, UnifiedExpressionVisitor<Noter>> {
     auto type = curr->ref->type.getHeapType();
     noteStructField(StructField{type, curr->index});
   }
+
+  void visitContNew(ContNew* curr) {
+    // The function reference that is passed in here will be called, just as if
+    // we were a call_ref, except at a potentially later time.
+    if (!curr->func->type.isRef()) {
+      return;
+    }
+
+    noteCallRef(curr->func->type.getHeapType());
+  }
 };
 
 // Analyze a module to find what things are referenced and what things are used.

src/passes/TypeMerging.cpp

@@ -248,6 +248,30 @@ struct ShapeHash {
   size_t operator()(const HeapType& type) const { return shapeHash(type); }
 };
 
+// The supertypes of each type in a descriptor chain. Hash and equality-compare
+// them using real identity to identify siblings chains that share the same
+// supertypes.
+using ChainSupers = std::vector<std::optional<HeapType>>;
+
+struct ChainSupersEq {
+  bool operator()(const ChainSupers& a, const ChainSupers& b) const {
+    return a == b;
+  }
+};
+
+struct ChainSupersHash {
+  size_t operator()(const ChainSupers& supers) const {
+    auto digest = wasm::hash(supers.size());
+    for (auto& super : supers) {
+      wasm::rehash(digest, !!super);
+      if (super) {
+        wasm::rehash(digest, *super);
+      }
+    }
+    return digest;
+  }
+};
+
 void TypeMerging::run(Module* module_) {
   module = module_;
 
@@ -339,8 +363,10 @@ bool TypeMerging::merge(MergeKind kind) {
   // that siblings that refine the supertype in the same way can be assigned to
   // the same partition and potentially merged.
   std::unordered_map<
-    std::optional<HeapType>,
-    std::unordered_map<HeapType, Partitions::iterator, ShapeHash, ShapeEq>>
+    std::vector<std::optional<HeapType>>,
+    std::unordered_map<HeapType, Partitions::iterator, ShapeHash, ShapeEq>,
+    ChainSupersHash,
+    ChainSupersEq>
     shapePartitions;
 
   // Ensure the type has a partition and return a reference to it. Since we
@@ -358,12 +384,16 @@ bool TypeMerging::merge(MergeKind kind) {
   // Similar to the above, but look up or create a partition associated with the
   // type's supertype and top-level shape rather than its identity.
   auto ensureShapePartition = [&](HeapType type) -> Partitions::iterator {
-    auto super = type.getDeclaredSuperType();
-    if (super) {
-      super = getMerged(*super);
+    ChainSupers supers;
+    for (auto t : type.getDescriptorChain()) {
+      auto super = t.getDeclaredSuperType();
+      if (super) {
+        super = getMerged(*super);
+      }
+      supers.push_back(super);
     }
     auto [it, inserted] =
-      shapePartitions[super].insert({type, partitions.end()});
+      shapePartitions[supers].insert({type, partitions.end()});
     if (inserted) {
       it->second = partitions.insert(partitions.end(), Partition{});
     }

src/tools/optimization-options.h

@@ -246,6 +246,17 @@ struct OptimizationOptions : public ToolOptions {
              passOptions.inlining.oneCallerInlineMaxSize =
                static_cast<Index>(atoi(argument.c_str()));
            })
+      .add("--inline-max-combined-binary-size",
+           "-imcbs",
+           "Max size of combined functions after inlining. "
+           "Default: " +
+             std::to_string(InliningOptions().maxCombinedBinarySize),
+           OptimizationOptionsCategory,
+           Options::Arguments::One,
+           [this](Options* o, const std::string& argument) {
+             passOptions.inlining.maxCombinedBinarySize =
+               static_cast<Index>(atoi(argument.c_str()));
+           })
       .add("--inline-functions-with-loops",
            "-ifwl",
            "Allow inlining functions with loops",

src/wasm-interpreter.h

@@ -4824,7 +4824,11 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
       return funcFlow;
     }
     // Create a new continuation for the target function.
-    Name funcName = funcFlow.getSingleValue().getFunc();
+    auto funcValue = funcFlow.getSingleValue();
+    if (funcValue.isNull()) {
+      trap("null ref");
+    }
+    auto funcName = funcValue.getFunc();
     auto* func = self()->getModule()->getFunction(funcName);
     return Literal(std::make_shared<ContData>(
       self()->makeFuncData(func->name, func->type), curr->type.getHeapType()));

test/lit/help/wasm-metadce.test

@@ -634,6 +634,9 @@
 ;; CHECK-NEXT:                                                 caller (default -1, which means
 ;; CHECK-NEXT:                                                 all such functions are inlined)
 ;; CHECK-NEXT:
+;; CHECK-NEXT:   --inline-max-combined-binary-size,-imcbs      Max size of combined functions
+;; CHECK-NEXT:                                                 after inlining. Default: 409600
+;; CHECK-NEXT:
 ;; CHECK-NEXT:   --inline-functions-with-loops,-ifwl           Allow inlining functions with
 ;; CHECK-NEXT:                                                 loops
 ;; CHECK-NEXT:

test/lit/help/wasm-opt.test

@@ -658,6 +658,9 @@
 ;; CHECK-NEXT:                                                 caller (default -1, which means
 ;; CHECK-NEXT:                                                 all such functions are inlined)
 ;; CHECK-NEXT:
+;; CHECK-NEXT:   --inline-max-combined-binary-size,-imcbs      Max size of combined functions
+;; CHECK-NEXT:                                                 after inlining. Default: 409600
+;; CHECK-NEXT:
 ;; CHECK-NEXT:   --inline-functions-with-loops,-ifwl           Allow inlining functions with
 ;; CHECK-NEXT:                                                 loops
 ;; CHECK-NEXT: