MIPS[64]: Reland of `Fix unaligned arguments storage in Wasm-to-interpreter entry`

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 bb984253017de9addbbc353615b0fb36e91b669a..183f8d5182ba56337b7459039172fe9a5d78f2e4 100644
--- a/src/compiler/wasm-compiler.cc
+++ b/src/compiler/wasm-compiler.cc
@@ -2811,10 +2811,11 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
       sig->return_count() == 0 ? 0 : 1 << ElementSizeLog2Of(sig->GetReturn(0));
 
   // Get a stack slot for the arguments.
-  Node* arg_buffer = args_size_bytes == 0 && return_size_bytes == 0
-                         ? jsgraph()->IntPtrConstant(0)
-                         : graph()->NewNode(jsgraph()->machine()->StackSlot(
-                               std::max(args_size_bytes, return_size_bytes)));
+  Node* arg_buffer =
+      args_size_bytes == 0 && return_size_bytes == 0
+          ? jsgraph()->IntPtrConstant(0)
+          : graph()->NewNode(jsgraph()->machine()->StackSlot(
+                std::max(args_size_bytes, return_size_bytes), 8));
 
   // Now store all our arguments to the buffer.
   int param_index = 0;
@@ -2824,27 +2825,61 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
     Node* param = Param(param_index++);
     if (Int64Lowering::IsI64AsTwoParameters(jsgraph()->machine(),
                                             sig->GetParam(i))) {
-      StoreRepresentation store_rep(wasm::kWasmI32,
-                                    WriteBarrierKind::kNoWriteBarrier);
-      *effect_ =
-          graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
-                           Int32Constant(offset + kInt64LowerHalfMemoryOffset),
-                           param, *effect_, *control_);
-
-      param = Param(param_index++);
-      *effect_ =
-          graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
-                           Int32Constant(offset + kInt64UpperHalfMemoryOffset),
-                           param, *effect_, *control_);
+      int alignment = offset % (1 << ElementSizeLog2Of(wasm::kWasmI32));
+      bool offset_aligned = (alignment == 0);
+
+      if (offset_aligned ||

[Clemens Hammacher, 2017/05/23 12:35:12]

You introduce lots of code duplication here.
Please extract a method like this:

const Operator* GetSafeStoreOperator(MachineOperatorBuilder* machine,
                                     int offset, wasm::ValueType type) {
  int alignment = offset % (1 << ElementSizeLog2Of(type));
  if (alignment == 0 || machine->UnalignedStoreSupported(
                            MachineType::TypeForRepresentation(type), 0)) {
    StoreRepresentation rep(type, WriteBarrierKind::kNoWriteBarrier);
    return machine->Store(rep);
  }
  UnalignedStoreRepresentation rep(type);
  return machine->UnalignedStore(rep);
}

And then just use this to generate the nodes:
*effect_ = graph()->NewNode(
          GetSafeStoreOperator(jsgraph()->machine(), offset, param_rep),
          arg_buffer, ...

[ivica.bogosavljevic, 2017/05/23 13:54:40]

I like this approach better, consider it done

+          jsgraph()->machine()->UnalignedStoreSupported(
+              MachineType::TypeForRepresentation(wasm::kWasmI32), 0)) {
+        StoreRepresentation store_rep(wasm::kWasmI32,
+                                      WriteBarrierKind::kNoWriteBarrier);
+
+        *effect_ = graph()->NewNode(
+            jsgraph()->machine()->Store(store_rep), arg_buffer,
+            Int32Constant(offset + kInt64LowerHalfMemoryOffset), param,
+            *effect_, *control_);
+
+        param = Param(param_index++);
+        *effect_ = graph()->NewNode(
+            jsgraph()->machine()->Store(store_rep), arg_buffer,
+            Int32Constant(offset + kInt64UpperHalfMemoryOffset), param,
+            *effect_, *control_);
+      } else {
+        UnalignedStoreRepresentation store_rep(wasm::kWasmI32);
+
+        *effect_ = graph()->NewNode(
+            jsgraph()->machine()->UnalignedStore(store_rep), arg_buffer,
+            Int32Constant(offset + kInt64LowerHalfMemoryOffset), param,
+            *effect_, *control_);
+
+        param = Param(param_index++);
+        *effect_ = graph()->NewNode(
+            jsgraph()->machine()->UnalignedStore(store_rep), arg_buffer,
+            Int32Constant(offset + kInt64UpperHalfMemoryOffset), param,
+            *effect_, *control_);
+      }
       offset += 8;
 
     } else {
       MachineRepresentation param_rep = sig->GetParam(i);
-      StoreRepresentation store_rep(param_rep,
-                                    WriteBarrierKind::kNoWriteBarrier);
-      *effect_ =
-          graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
-                           Int32Constant(offset), param, *effect_, *control_);
+      int alignment = offset % (1 << ElementSizeLog2Of(param_rep));
+      bool offset_aligned = (alignment == 0);
+
+      if (offset_aligned ||
+          jsgraph()->machine()->UnalignedStoreSupported(
+              MachineType::TypeForRepresentation(param_rep), 0)) {
+        StoreRepresentation store_rep(param_rep,
+                                      WriteBarrierKind::kNoWriteBarrier);
+        *effect_ =
+            graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
+                             Int32Constant(offset), param, *effect_, *control_);
+
+      } else {
+        UnalignedStoreRepresentation store_rep(param_rep);
+        *effect_ = graph()->NewNode(
+            jsgraph()->machine()->UnalignedStore(store_rep), arg_buffer,
+            Int32Constant(offset), param, *effect_, *control_);
+      }
       offset += 1 << ElementSizeLog2Of(param_rep);
     }
   }
@@ -3841,7 +3876,10 @@ Handle<Code> CompileWasmInterpreterEntry(Isolate* isolate, uint32_t func_index,
   Zone zone(isolate->allocator(), ZONE_NAME);
   Graph graph(&zone);
   CommonOperatorBuilder common(&zone);
-  MachineOperatorBuilder machine(&zone);
+  MachineOperatorBuilder machine(
+      &zone, MachineType::PointerRepresentation(),
+      InstructionSelector::SupportedMachineOperatorFlags(),
+      InstructionSelector::AlignmentRequirements());
   JSGraph jsgraph(isolate, &graph, &common, nullptr, nullptr, &machine);
 
   Node* control = nullptr;