[wasm] [asm.js] Track direct function indices and fixup.

Recent changes to the wasm format prevent you from knowing
the function index of a direct call until you know how many
imports you have (the index spaces have been merged).

Asm.js validation can't know this until all callsites have been
visited if you want to avoid materializing the entire AST / parsing twice.
Instead, keep a list of fixups to perform for all direct callsites,
then update these with the imports count added when emitting the module.

BUG=v8:4203
R=aseemgarg@chromium.org,titzer@chromium.org

Review-Url: https://codereview.chromium.org/2384623003
Cr-Commit-Position: refs/heads/master@{#39937}

src/asmjs/asm-wasm-builder.cc

@@ -108,20 +108,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
     return ret;
   }
 
-  void BuildImports() {
-    for (const AsmTyper::FFIUseSignature& ffi : typer_->FFIUseSignatures()) {
-      size_t ret_count = ffi.return_type_ == AsmType::Void() ? 0 : 1;
-      FunctionSig::Builder b(zone_, ret_count, ffi.arg_types_.size());
-      for (AsmType* arg : ffi.arg_types_) b.AddParam(TypeFrom(arg));
-      if (ffi.return_type_ != AsmType::Void()) {
-        b.AddReturn(TypeFrom(ffi.return_type_));
-      }
-      imported_function_table_.AddFunction(ffi.var, b.Build());
-    }
-  }
-
   void Build() {
-    BuildImports();
     InitializeInitFunction();
     RECURSE(VisitFunctionLiteral(literal_));
     BuildForeignInitFunction();
@@ -733,11 +720,12 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
    private:
     class ImportedFunctionIndices : public ZoneObject {
      public:
-      bool has_name_;
+      const char* name_;
+      int name_length_;
       WasmModuleBuilder::SignatureMap signature_to_index_;
 
-      explicit ImportedFunctionIndices(Zone* zone)
-          : has_name_(false), signature_to_index_(zone) {}
+      ImportedFunctionIndices(const char* name, int name_length, Zone* zone)
+          : name_(name), name_length_(name_length), signature_to_index_(zone) {}
     };
     ZoneHashMap table_;
     AsmWasmBuilderImpl* builder_;
@@ -748,51 +736,31 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
                  ZoneAllocationPolicy(builder->zone())),
           builder_(builder) {}
 
-    // Set the imported name of a variable. Must happen after all signatures
-    // (and thus import indices) are added for a given variable.
-    void SetImportName(Variable* v, const char* name, int name_length) {
-      auto indices = GetEntry(v);
-      if (indices) {
-        for (auto entry : indices->signature_to_index_) {
-          uint32_t index = entry.second;
-          builder_->builder_->SetImportName(index, name, name_length);
-        }
-        indices->has_name_ = true;
-      }
-    }
-
-    // Get a function's index. Does not insert new entries.
-    uint32_t GetFunctionIndex(Variable* v, FunctionSig* sig) {
-      auto indices = GetEntry(v);
-      DCHECK_NOT_NULL(indices);
-      auto pos = indices->signature_to_index_.find(sig);
-      DCHECK(pos != indices->signature_to_index_.end());
-      return pos->second;
-    }
-
-    // Add a function and register it as an import with the builder.
-    void AddFunction(Variable* v, FunctionSig* sig) {
-      auto entry = table_.LookupOrInsert(
+    void AddImport(Variable* v, const char* name, int name_length) {
+      ImportedFunctionIndices* indices = new (builder_->zone())
+          ImportedFunctionIndices(name, name_length, builder_->zone());
+      auto* entry = table_.LookupOrInsert(
           v, ComputePointerHash(v), ZoneAllocationPolicy(builder_->zone()));
-      if (entry->value == nullptr) {
-        entry->value =
-            new (builder_->zone()) ImportedFunctionIndices(builder_->zone());
-      }
-      auto indices = reinterpret_cast<ImportedFunctionIndices*>(entry->value);
-      DCHECK(!indices->has_name_);
-      auto pos = indices->signature_to_index_.find(sig);
-      if (pos == indices->signature_to_index_.end()) {
-        // A new import. Name is not known up front.
-        uint32_t index = builder_->builder_->AddImport(nullptr, 0, sig);
+      entry->value = indices;
+    }
+
+    // Get a function's index (or allocate if new).
+    uint32_t LookupOrInsertImport(Variable* v, FunctionSig* sig) {
+      ZoneHashMap::Entry* entry = table_.Lookup(v, ComputePointerHash(v));
+      DCHECK_NOT_NULL(entry);
+      ImportedFunctionIndices* indices =
+          reinterpret_cast<ImportedFunctionIndices*>(entry->value);
+      WasmModuleBuilder::SignatureMap::iterator pos =
+          indices->signature_to_index_.find(sig);
+      if (pos != indices->signature_to_index_.end()) {
+        return pos->second;
+      } else {
+        uint32_t index = builder_->builder_->AddImport(
+            indices->name_, indices->name_length_, sig);
         indices->signature_to_index_[sig] = index;
+        return index;
       }
     }
-
-    ImportedFunctionIndices* GetEntry(Variable* v) {
-      auto entry = table_.Lookup(v, ComputePointerHash(v));
-      if (entry == nullptr) return nullptr;
-      return reinterpret_cast<ImportedFunctionIndices*>(entry->value);
-    }
   };
 
   void EmitAssignmentLhs(Expression* target, MachineType* mtype) {
@@ -942,7 +910,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
           if (typer_->TypeOf(target)->AsFFIType() != nullptr) {
             const AstRawString* name =
                 prop->key()->AsLiteral()->AsRawPropertyName();
-            imported_function_table_.SetImportName(
+            imported_function_table_.AddImport(
                 target->var(), reinterpret_cast<const char*>(name->raw_data()),
                 name->length());
           }
@@ -1393,8 +1361,8 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
           for (int i = 0; i < args->length(); ++i) {
             sig.AddParam(TypeOf(args->at(i)));
           }
-          uint32_t index =
-              imported_function_table_.GetFunctionIndex(vp->var(), sig.Build());
+          uint32_t index = imported_function_table_.LookupOrInsertImport(
+              vp->var(), sig.Build());
           VisitCallArgs(expr);
           current_function_builder_->Emit(kExprCallFunction);
           current_function_builder_->EmitVarInt(index);
@@ -1402,7 +1370,8 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
           WasmFunctionBuilder* function = LookupOrInsertFunction(vp->var());
           VisitCallArgs(expr);
           current_function_builder_->Emit(kExprCallFunction);
-          current_function_builder_->EmitVarInt(function->func_index());
+          current_function_builder_->EmitDirectCallIndex(
+              function->func_index());
           returns_value = function->signature()->return_count() > 0;
         }
         break;

src/wasm/wasm-module-builder.cc

@@ -51,14 +51,14 @@ WasmFunctionBuilder::WasmFunctionBuilder(WasmModuleBuilder* builder)
       locals_(builder->zone()),
       signature_index_(0),
       exported_(0),
-      func_index_(static_cast<uint32_t>(builder->imports_.size() +
-                                        builder->functions_.size())),
+      func_index_(static_cast<uint32_t>(builder->functions_.size())),
       body_(builder->zone()),
       name_(builder->zone()),
       i32_temps_(builder->zone()),
       i64_temps_(builder->zone()),
       f32_temps_(builder->zone()),
-      f64_temps_(builder->zone()) {}
+      f64_temps_(builder->zone()),
+      direct_calls_(builder->zone()) {}
 
 void WasmFunctionBuilder::EmitVarInt(uint32_t val) {
   byte buffer[8];
@@ -130,6 +130,15 @@ void WasmFunctionBuilder::EmitI32Const(int32_t value) {
   }
 }
 
+void WasmFunctionBuilder::EmitDirectCallIndex(uint32_t index) {
+  DirectCallIndex call;
+  call.offset = body_.size();
+  call.direct_index = index;
+  direct_calls_.push_back(call);
+  byte code[] = {U32V_5(0)};
+  EmitCode(code, sizeof(code));
+}
+
 void WasmFunctionBuilder::SetExported() { exported_ = true; }
 
 void WasmFunctionBuilder::SetName(const char* name, int name_length) {
@@ -152,7 +161,8 @@ void WasmFunctionBuilder::WriteExport(ZoneBuffer& buffer) const {
       buffer.write(reinterpret_cast<const byte*>(&name_[0]), name_.size());
     }
     buffer.write_u8(kExternalFunction);
-    buffer.write_u32v(func_index_);
+    buffer.write_u32v(func_index_ +
+                      static_cast<uint32_t>(builder_->imports_.size()));
   }
 }
 
@@ -164,7 +174,13 @@ void WasmFunctionBuilder::WriteBody(ZoneBuffer& buffer) const {
   locals_.Emit(*ptr);
   (*ptr) += locals_size;  // UGLY: manual bump of position pointer
   if (body_.size() > 0) {
+    size_t base = buffer.offset();
     buffer.write(&body_[0], body_.size());
+    for (DirectCallIndex call : direct_calls_) {
+      buffer.patch_u32v(
+          base + call.offset,
+          call.direct_index + static_cast<uint32_t>(builder_->imports_.size()));
+    }
   }
 }
 
@@ -230,7 +246,6 @@ void WasmModuleBuilder::AddIndirectFunction(uint32_t index) {
 
 uint32_t WasmModuleBuilder::AddImport(const char* name, int name_length,
                                       FunctionSig* sig) {
-  DCHECK_EQ(0, functions_.size());  // imports must be added before functions!
   imports_.push_back({AddSignature(sig), name, name_length});
   return static_cast<uint32_t>(imports_.size() - 1);
 }
@@ -369,7 +384,8 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer& buffer) const {
   // == emit start function index ==============================================
   if (start_function_index_ >= 0) {
     size_t start = EmitSection(kStartSectionCode, buffer);
-    buffer.write_u32v(start_function_index_);
+    buffer.write_u32v(start_function_index_ +
+                      static_cast<uint32_t>(imports_.size()));
     FixupSection(buffer, start);
   }
 
@@ -384,7 +400,7 @@ void WasmModuleBuilder::WriteTo(ZoneBuffer& buffer) const {
     buffer.write_size(indirect_functions_.size());  // element count
 
     for (auto index : indirect_functions_) {
-      buffer.write_u32v(index);
+      buffer.write_u32v(index + static_cast<uint32_t>(imports_.size()));
     }
 
     FixupSection(buffer, start);

src/wasm/wasm-module-builder.h

@@ -126,6 +126,7 @@ class V8_EXPORT_PRIVATE WasmFunctionBuilder : public ZoneObject {
   void EmitWithU8(WasmOpcode opcode, const byte immediate);
   void EmitWithU8U8(WasmOpcode opcode, const byte imm1, const byte imm2);
   void EmitWithVarInt(WasmOpcode opcode, uint32_t immediate);
+  void EmitDirectCallIndex(uint32_t index);
   void SetExported();
   void SetName(const char* name, int name_length);
 
@@ -141,6 +142,12 @@ class V8_EXPORT_PRIVATE WasmFunctionBuilder : public ZoneObject {
   explicit WasmFunctionBuilder(WasmModuleBuilder* builder);
   friend class WasmModuleBuilder;
   friend class WasmTemporary;
+
+  struct DirectCallIndex {
+    size_t offset;
+    uint32_t direct_index;
+  };
+
   WasmModuleBuilder* builder_;
   LocalDeclEncoder locals_;
   uint32_t signature_index_;
@@ -152,6 +159,7 @@ class V8_EXPORT_PRIVATE WasmFunctionBuilder : public ZoneObject {
   ZoneVector<uint32_t> i64_temps_;
   ZoneVector<uint32_t> f32_temps_;
   ZoneVector<uint32_t> f64_temps_;
+  ZoneVector<DirectCallIndex> direct_calls_;
 };
 
 class WasmTemporary {