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

src/compiler/simd-scalar-lowering.cc

Index: src/compiler/simd-scalar-lowering.cc
diff --git a/src/compiler/simd-scalar-lowering.cc b/src/compiler/simd-scalar-lowering.cc
index c5a94b4297e35d39d40714caf6bff06d9cf87554..9ac1716a7eca5643dcc653f8aa7a5ea3d11d36a7 100644
--- a/src/compiler/simd-scalar-lowering.cc
+++ b/src/compiler/simd-scalar-lowering.cc
@@ -131,6 +131,15 @@ static int GetReturnCountAfterLowering(
   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()) {
@@ -175,6 +184,129 @@ void SimdScalarLowering::LowerNode(Node* 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());
@@ -226,31 +358,37 @@ void SimdScalarLowering::LowerNode(Node* 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);
-      Node* rep_node[kMaxLanes];
-      for (int i = 0; i < kMaxLanes; i++) {
-        rep_node[i] =
-            graph()->NewNode(machine()->Int32Add(), rep_left[i], rep_right[i]);
-      }
-      ReplaceNode(node, rep_node);
-      break;
-    }
-
-    case IrOpcode::kCreateInt32x4: {
+#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.

-        DCHECK(!HasReplacement(1, node->InputAt(i)));
-        rep_node[i] = node->InputAt(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::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);
@@ -260,41 +398,21 @@ void SimdScalarLowering::LowerNode(Node* node) {
       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]);
-      }
-      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: {
+    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[kMaxLanes] = {
-          GetReplacementsWithType(node->InputAt(0), rep_type)[lane], nullptr,
-          nullptr, nullptr};
+      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;
     }
-
     default: { DefaultLowering(node); }
   }
 }