[wasm] Add support for multiple indirect function tables

src/compiler/wasm-compiler.cc

Index: src/compiler/wasm-compiler.cc
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
index 300547a31add6284d64c51dacbd9f7761351b962..5d037d904dc7c1fcc0d5410c334f97222a09ac8c 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -276,7 +276,7 @@ WasmGraphBuilder::WasmGraphBuilder(
       module_(nullptr),
       mem_buffer_(nullptr),
       mem_size_(nullptr),
-      function_table_(nullptr),
+      function_table_(zone),
       control_(nullptr),
       effect_(nullptr),
       cur_buffer_(def_buffer_),
@@ -2058,10 +2058,12 @@ Node* WasmGraphBuilder::CallIndirect(uint32_t index, Node** args,
   Node* key = args[0];
 
   // Bounds check the index.
-  int table_size = static_cast<int>(module_->FunctionTableSize());
-  if (table_size > 0) {
+  int table_size = 0;
+  // Assume only one table for now.

[ahaas, 2016/07/25 19:23:44]

Could you add a DCHECK for this assumption?

[ddchen, 2016/07/25 22:17:36]

Done.

+  if (module_->IsValidTable(0) &&
+      ((table_size = module_->GetTable(0)->max_size) > 0)) {

[ahaas, 2016/07/25 19:23:44]

I personally don't like assignments in conditions.

[ddchen, 2016/07/25 22:17:36]

Done.

     // Bounds check against the table size.
-    Node* size = Int32Constant(static_cast<int>(table_size));
+    Node* size = Int32Constant(table_size);
     Node* in_bounds = graph()->NewNode(machine->Uint32LessThan(), key, size);
     trap_->AddTrapIfFalse(wasm::kTrapFuncInvalid, in_bounds, position);
   } else {
@@ -2069,7 +2071,7 @@ Node* WasmGraphBuilder::CallIndirect(uint32_t index, Node** args,
     trap_->AddTrapIfFalse(wasm::kTrapFuncInvalid, Int32Constant(0), position);
     return trap_->GetTrapValue(module_->GetSignature(index));
   }
-  Node* table = FunctionTable();
+  Node* table = FunctionTable(0);
 
   // Load signature from the table and check.
   // The table is a FixedArray; signatures are encoded as SMIs.
@@ -2129,10 +2131,12 @@ Node* WasmGraphBuilder::JITSingleFunction(Node* const base, Node* const length,
 
   // Bounds check the index.
   {
-    int table_size = static_cast<int>(module_->FunctionTableSize());
-    if (table_size > 0) {
+    int table_size = 0;
+    // Assume only one table for now.

[ahaas, 2016/07/25 19:23:44]

same here.

[ddchen, 2016/07/25 22:17:36]

Done.

+    if (module_->IsValidTable(0) &&
+        ((table_size = module_->GetTable(0)->max_size) > 0)) {
       // Bounds check against the table size.
-      Node* size = Int32Constant(static_cast<int>(table_size));
+      Node* size = Int32Constant(table_size);
       Node* in_bounds =
           graph()->NewNode(machine->Uint32LessThan(), index, size);
       trap_->AddTrapIfFalse(wasm::kTrapInvalidIndex, in_bounds, position);
@@ -2162,7 +2166,7 @@ Node* WasmGraphBuilder::JITSingleFunction(Node* const base, Node* const length,
   inputs[1] = BuildChangeUint32ToSmi(base);
   inputs[2] = BuildChangeUint32ToSmi(length);
   inputs[3] = BuildChangeUint32ToSmi(index);
-  inputs[4] = FunctionTable();
+  inputs[4] = FunctionTable(0);
   inputs[5] = Uint32Constant(sig_index);
   inputs[6] = BuildChangeUint32ToSmi(Uint32Constant(return_count));
 
@@ -2804,13 +2808,20 @@ Node* WasmGraphBuilder::DefaultS128Value() {
                           zero, zero);
 }
 
-Node* WasmGraphBuilder::FunctionTable() {
+Node* WasmGraphBuilder::FunctionTable(uint32_t index) {
   DCHECK(module_ && module_->instance &&
-         !module_->instance->function_table.is_null());
-  if (!function_table_) {
-    function_table_ = HeapConstant(module_->instance->function_table);
+         index < module_->instance->function_table.size());
+  if (index >= function_table_.size()) {
+    for (uint32_t i = 0; i <= index; i++) {
+      if (i < function_table_.size() && function_table_[i]) {
+        continue;
+      }
+      DCHECK(!module_->instance->function_table[index].is_null());

[ahaas, 2016/07/25 19:23:44]

Shouldn't this be {i} instead of {index}? If you really meant to check
function_table[index], then I think the DCHECK should be outside the for loop.

+      function_table_.push_back(

[ahaas, 2016/07/25 19:23:44]

Does {push_back} actually add the element at the right location if {i <
function_table_.size()} is true and {function_table_[i]} is false?

[ddchen, 2016/07/25 22:17:36]

This code was incorrect, it should be fixed now.

+          HeapConstant(module_->instance->function_table[index]));
+    }
   }
-  return function_table_;
+  return function_table_[index];
 }
 
 Node* WasmGraphBuilder::ChangeToRuntimeCall(Node* node,