[wasm] implement simd lowering for replaceLane, load, store and test for phi

src/compiler/simd-scalar-lowering.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/simd-scalar-lowering.h"
 #include "src/compiler/diamond.h"
 #include "src/compiler/linkage.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
 
@@ skipping 113 matching lines @@
     Signature<MachineRepresentation>* signature) {
   int result = static_cast<int>(signature->return_count());
   for (int i = 0; i < static_cast<int>(signature->return_count()); i++) {
     if (signature->GetReturn(i) == MachineRepresentation::kSimd128) {
       result += 3;
     }
   }
   return result;
 }
 
+void SimdScalarLowering::GetIndexNodes(Node* index, Node** new_indices) {
+  new_indices[0] = index;
+  for (size_t i = 1; i < kMaxLanes; i++) {

[Mircea Trofin, 2016/11/15 16:21:27]

++i

[aseemgarg, 2016/11/19 01:56:54]

Done.

+    new_indices[i] = graph()->NewNode(
+        machine()->Int32Add(), index,
+        graph()->NewNode(common()->Int32Constant(static_cast<int>(i) * 4)));

[titzer, 2016/11/15 09:21:17]

Should this be 16 / kMaxLanes for clarity?

[Mircea Trofin, 2016/11/15 16:21:27]

Consider using the relevant constant for "4" (or "16", after Ben's feedback)
I'd add a DCHECK that 16%kMaxLanes == 0

[aseemgarg, 2016/11/19 01:56:54]

Done. But I don't get how it is clearer? 4 is just the number of bytes in
int32_t. 

[titzer, 2016/11/22 12:01:52]

I meant, please introduce a constant that defines the width of a lane. the size
of a vector divided by the number of lanes is the width of a lane.

[aseemgarg, 2016/11/23 23:07:56]

Done.

+  }
+}
+
 void SimdScalarLowering::LowerNode(Node* node) {
   SimdType rep_type = ReplacementType(node);
   switch (node->opcode()) {
     case IrOpcode::kStart: {
       int parameter_count = GetParameterCountAfterLowering();
       // Only exchange the node if the parameter count actually changed.
       if (parameter_count != static_cast<int>(signature()->parameter_count())) {
         int delta =
             parameter_count - static_cast<int>(signature()->parameter_count());
         int new_output_count = node->op()->ValueOutputCount() + delta;
@@ skipping 24 matching lines @@
             for (int i = 1; i < kMaxLanes; i++) {
               new_node[i] = graph()->NewNode(common()->Parameter(new_index + i),
                                              graph()->start());
             }
           }
           ReplaceNode(node, new_node);
         }
       }
       break;
     }
+    case IrOpcode::kLoad:
+    case IrOpcode::kUnalignedLoad: {
+      MachineRepresentation rep;
+      if (node->opcode() == IrOpcode::kLoad) {
+        rep = LoadRepresentationOf(node->op()).representation();
+      } else {
+        DCHECK(node->opcode() == IrOpcode::kUnalignedLoad);
+        rep = UnalignedLoadRepresentationOf(node->op()).representation();
+      }
+
+      if (rep == MachineRepresentation::kSimd128) {
+        Node* base = node->InputAt(0);
+        Node* index = node->InputAt(1);
+        Node* indices[kMaxLanes];
+        GetIndexNodes(index, indices);
+        const Operator* load_op;
+        if (node->opcode() == IrOpcode::kLoad && rep_type == SimdType::kInt32) {

[titzer, 2016/11/15 09:21:17]

Can you factor out a subroutines GetLoadOp() and GetStoreOp()?

[aseemgarg, 2016/11/19 01:56:54]

Done.

+          load_op = machine()->Load(MachineType::Int32());
+        } else if (node->opcode() == IrOpcode::kLoad &&
+                   rep_type == SimdType::kFloat32) {
+          load_op = machine()->Load(MachineType::Float32());
+        } else if (node->opcode() == IrOpcode::kUnalignedLoad &&
+                   rep_type == SimdType::kInt32) {
+          load_op = machine()->UnalignedLoad(MachineType::Int32());
+        } else if (node->opcode() == IrOpcode::kUnalignedLoad &&
+                   rep_type == SimdType::kFloat32) {
+          load_op = machine()->UnalignedLoad(MachineType::Float32());
+        }
+
+        Node* rep_nodes[kMaxLanes];
+        rep_nodes[0] = node;
+        NodeProperties::ChangeOp(rep_nodes[0], load_op);
+        if (node->InputCount() > 2) {
+          Node* effect_high = node->InputAt(2);
+          Node* control_high = node->InputAt(3);

[Mircea Trofin, 2016/11/15 16:21:27]

aren't inputs 0 indexed? in that case, wouldn't InputAt(3) potentially fail?

[aseemgarg, 2016/11/19 01:56:54]

The check for greater than 2 just tells that there are control and effect nodes
(apart from base and index).

+          rep_nodes[3] = graph()->NewNode(load_op, base, indices[3],

[Mircea Trofin, 2016/11/15 16:21:27]

This all hinges on kMaxLanes > 3 (a DCHECK?)

[aseemgarg, 2016/11/19 01:56:54]

Done.

+                                          effect_high, control_high);
+          rep_nodes[2] = graph()->NewNode(load_op, base, indices[2],
+                                          rep_nodes[3], control_high);
+          rep_nodes[1] = graph()->NewNode(load_op, base, indices[1],
+                                          rep_nodes[2], control_high);
+          rep_nodes[0]->ReplaceInput(2, rep_nodes[1]);
+        } else {
+          for (size_t i = 1; i < kMaxLanes; i++) {

[Mircea Trofin, 2016/11/15 16:21:27]

++i

[aseemgarg, 2016/11/19 01:56:54]

Done.

+            rep_nodes[i] = graph()->NewNode(load_op, base, indices[i]);
+          }
+        }
+        ReplaceNode(node, rep_nodes);
+      } else {
+        DefaultLowering(node);
+      }
+      break;
+    }
+    case IrOpcode::kStore:
+    case IrOpcode::kUnalignedStore: {
+      MachineRepresentation rep;
+      if (node->opcode() == IrOpcode::kStore) {
+        rep = StoreRepresentationOf(node->op()).representation();
+      } else {
+        DCHECK(node->opcode() == IrOpcode::kUnalignedStore);
+        rep = UnalignedStoreRepresentationOf(node->op());
+      }
+
+      if (rep == MachineRepresentation::kSimd128) {
+        Node* base = node->InputAt(0);
+        Node* index = node->InputAt(1);
+        Node* indices[4];

[Mircea Trofin, 2016/11/15 16:21:26]

what's "4"?

[aseemgarg, 2016/11/19 01:56:54]

changed to kMaxLanes

+        GetIndexNodes(index, indices);
+        Node* value = node->InputAt(2);

[Mircea Trofin, 2016/11/15 16:21:27]

DCHECK node's input count

[aseemgarg, 2016/11/19 01:56:54]

Done.

+        DCHECK(HasReplacement(1, value));
+        const Operator* store_op;
+        if (node->opcode() == IrOpcode::kStore) {
+          WriteBarrierKind write_barrier_kind =
+              StoreRepresentationOf(node->op()).write_barrier_kind();
+          if (rep_type == SimdType::kInt32) {
+            store_op = machine()->Store(StoreRepresentation(
+                MachineRepresentation::kWord32, write_barrier_kind));
+          } else {
+            store_op = machine()->Store(StoreRepresentation(
+                MachineRepresentation::kFloat32, write_barrier_kind));
+          }
+        } else {
+          if (rep_type == SimdType::kInt32) {
+            store_op =
+                machine()->UnalignedStore(MachineRepresentation::kWord32);
+          } else {
+            store_op =
+                machine()->UnalignedStore(MachineRepresentation::kFloat32);
+          }
+        }
+
+        Node* rep_nodes[kMaxLanes];
+        rep_nodes[0] = node;
+        Node** rep_inputs = GetReplacementsWithType(value, rep_type);
+        rep_nodes[0]->ReplaceInput(2, rep_inputs[0]);
+        NodeProperties::ChangeOp(node, store_op);
+        if (node->InputCount() > 3) {
+          Node* effect_high = node->InputAt(3);

[Mircea Trofin, 2016/11/15 16:21:27]

same question about input being 0 indexed

[aseemgarg, 2016/11/19 01:56:54]

see above. Added Dcheck

[titzer, 2016/11/22 12:01:52]

Can you just name this effect_input and control_input?

[aseemgarg, 2016/11/23 23:07:56]

Done.

+          Node* control_high = node->InputAt(4);
+          rep_nodes[3] =
+              graph()->NewNode(store_op, base, indices[3], rep_inputs[3],
+                               effect_high, control_high);
+          rep_nodes[2] =
+              graph()->NewNode(store_op, base, indices[2], rep_inputs[2],
+                               rep_nodes[3], control_high);
+          rep_nodes[1] =
+              graph()->NewNode(store_op, base, indices[1], rep_inputs[1],
+                               rep_nodes[2], control_high);
+          rep_nodes[0]->ReplaceInput(3, rep_nodes[1]);

[titzer, 2016/11/15 09:21:17]

I think the effect chain is messed up here. I think you should feed the effect
ouput of rep_nodes[0] into the effect of rep_nodes[3] instead of effect_high
above.

[aseemgarg, 2016/11/19 01:56:54]

I am not quite sure I understand how effect nodes work. As I understand, effect
input basically ensures that effect node is scheduled before the current node.
In that case, it shouldn't really matter how the 4 replacements are ordered.
With the current system if we had something like effect->node->Others, it would
turn to effect->rep[3]->rep[2]->rep[1]->rep[0]->Others

[titzer, 2016/11/22 12:01:52]

Let me clarify a bit here. There isn't really an "effect node", there are effect
edges. Nodes can be "effectful" which means that they either have incoming or
outgoing effect edges (usually both).
Yes.
Yep.
Ok. I see. I usually think of the edges going in the other direction:

effect<-rep[3]<-rep[2]<-rep[1]<-rep[0]<-Others

Upon a second look (not sure if you changed anything here), yes, this should
work. I was concerned that the effect chain you had left one of them out.

[aseemgarg, 2016/11/23 23:07:56]

Didn't change anything. Leaving as is

On 2016/11/22 12:01:52, titzer wrote:

+
+        } else {
+          for (size_t i = 1; i < kMaxLanes; i++) {
+            rep_nodes[i] =
+                graph()->NewNode(store_op, base, indices[i], rep_inputs[i]);
+          }
+        }
+
+        ReplaceNode(node, rep_nodes);
+      } else {
+        DefaultLowering(node);
+      }
+      break;
+    }
     case IrOpcode::kReturn: {
       DefaultLowering(node);
       int new_return_count = GetReturnCountAfterLowering(signature());
       if (static_cast<int>(signature()->return_count()) != new_return_count) {
         NodeProperties::ChangeOp(node, common()->Return(new_return_count));
       }
       break;
     }
     case IrOpcode::kCall: {
       // TODO(turbofan): Make WASM code const-correct wrt. CallDescriptor.
@@ skipping 31 matching lines @@
               GetReplacementsWithType(node->InputAt(i), rep_type);
           for (int j = 0; j < kMaxLanes; j++) {
             rep_node[j]->ReplaceInput(i, rep_input[j]);
           }
         }
       } else {
         DefaultLowering(node);
       }
       break;
     }
-
-    case IrOpcode::kInt32x4Add: {
-      DCHECK(node->InputCount() == 2);
-      Node** rep_left = GetReplacementsWithType(node->InputAt(0), rep_type);
-      Node** rep_right = GetReplacementsWithType(node->InputAt(1), rep_type);
+#define BINOP_CASE(opcode, replacementOp)                                     \

[titzer, 2016/11/15 09:21:17]

Can you make this into a call to a subroutine? AFAICT the inline code is mostly
the same.

[aseemgarg, 2016/11/19 01:56:54]

Done.

+  case IrOpcode::opcode: {                                                    \
+    DCHECK(node->InputCount() == 2);                                          \
+    Node** rep_left = GetReplacementsWithType(node->InputAt(0), rep_type);    \
+    Node** rep_right = GetReplacementsWithType(node->InputAt(1), rep_type);   \
+    Node* rep_node[kMaxLanes];                                                \
+    for (int i = 0; i < kMaxLanes; i++) {                                     \

[Mircea Trofin, 2016/11/15 16:21:27]

++i

[aseemgarg, 2016/11/19 01:56:54]

Done.

+      rep_node[i] = graph()->NewNode(machine()->replacementOp(), rep_left[i], \
+                                     rep_right[i]);                           \
+    }                                                                         \
+    ReplaceNode(node, rep_node);                                              \
+    break;                                                                    \
+  }
+      BINOP_CASE(kInt32x4Add, Int32Add)
+      BINOP_CASE(kFloat32x4Add, Float32Add)
+#undef BINOP
+    case IrOpcode::kCreateInt32x4:
+    case IrOpcode::kCreateFloat32x4: {
       Node* rep_node[kMaxLanes];
       for (int i = 0; i < kMaxLanes; i++) {

[Mircea Trofin, 2016/11/15 16:21:27]

++i

[aseemgarg, 2016/11/19 01:56:54]

Done.

-        rep_node[i] =
-            graph()->NewNode(machine()->Int32Add(), rep_left[i], rep_right[i]);
+        if (HasReplacement(0, node->InputAt(i))) {
+          rep_node[i] = GetReplacements(node->InputAt(i))[0];
+        } else {
+          rep_node[i] = node->InputAt(i);
+        }
       }
       ReplaceNode(node, rep_node);
       break;
     }
-
-    case IrOpcode::kCreateInt32x4: {
-      Node* rep_node[kMaxLanes];
-      for (int i = 0; i < kMaxLanes; i++) {
-        DCHECK(!HasReplacement(1, node->InputAt(i)));
-        rep_node[i] = node->InputAt(i);
-      }
-      ReplaceNode(node, rep_node);
-      break;
-    }
-
-    case IrOpcode::kInt32x4ExtractLane: {
+    case IrOpcode::kInt32x4ExtractLane:
+    case IrOpcode::kFloat32x4ExtractLane: {
       Node* laneNode = node->InputAt(1);
       DCHECK_EQ(laneNode->opcode(), IrOpcode::kInt32Constant);
       int32_t lane = OpParameter<int32_t>(laneNode);
       Node* rep_node[kMaxLanes] = {
           GetReplacementsWithType(node->InputAt(0), rep_type)[lane], nullptr,
           nullptr, nullptr};
       ReplaceNode(node, rep_node);
       break;
     }
-
-    case IrOpcode::kFloat32x4Add: {
-      DCHECK(node->InputCount() == 2);
-      Node** rep_left = GetReplacementsWithType(node->InputAt(0), rep_type);
-      Node** rep_right = GetReplacementsWithType(node->InputAt(1), rep_type);
-      Node* rep_node[kMaxLanes];
-      for (int i = 0; i < kMaxLanes; i++) {
-        rep_node[i] = graph()->NewNode(machine()->Float32Add(), rep_left[i],
-                                       rep_right[i]);
+    case IrOpcode::kInt32x4ReplaceLane:
+    case IrOpcode::kFloat32x4ReplaceLane: {
+      Node* laneNode = node->InputAt(1);
+      Node* repNode = node->InputAt(2);

[Mircea Trofin, 2016/11/15 16:21:27]

dcheck for input count

[aseemgarg, 2016/11/19 01:56:54]

Done.

+      DCHECK_EQ(laneNode->opcode(), IrOpcode::kInt32Constant);
+      int32_t lane = OpParameter<int32_t>(laneNode);

[Mircea Trofin, 2016/11/15 16:21:27]

dcheck lane is in bounds for rep_node

[aseemgarg, 2016/11/19 01:56:54]

Done.

+      Node** rep_node = GetReplacementsWithType(node->InputAt(0), rep_type);
+      if (HasReplacement(0, repNode)) {
+        rep_node[lane] = GetReplacements(repNode)[0];
+      } else {
+        rep_node[lane] = repNode;
       }
       ReplaceNode(node, rep_node);
       break;
     }
-
-    case IrOpcode::kCreateFloat32x4: {
-      Node* rep_node[kMaxLanes];
-      for (int i = 0; i < kMaxLanes; i++) {
-        DCHECK(!HasReplacement(1, node->InputAt(i)));
-        rep_node[i] = node->InputAt(i);
-      }
-      ReplaceNode(node, rep_node);
-      break;
-    }
-
-    case IrOpcode::kFloat32x4ExtractLane: {
-      Node* laneNode = node->InputAt(1);
-      DCHECK_EQ(laneNode->opcode(), IrOpcode::kInt32Constant);
-      int32_t lane = OpParameter<int32_t>(laneNode);
-      Node* rep_node[kMaxLanes] = {
-          GetReplacementsWithType(node->InputAt(0), rep_type)[lane], nullptr,
-          nullptr, nullptr};
-      ReplaceNode(node, rep_node);
-      break;
-    }
-
     default: { DefaultLowering(node); }
   }
 }
 
 bool SimdScalarLowering::DefaultLowering(Node* node) {
   bool something_changed = false;
   for (int i = NodeProperties::PastValueIndex(node) - 1; i >= 0; i--) {
     Node* input = node->InputAt(i);
     if (HasReplacement(0, input)) {
       something_changed = true;
@@ skipping 93 matching lines @@
       } else {
         UNREACHABLE();
       }
     }
     ReplaceNode(phi, rep_nodes);
   }
 }
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8