[interpreter] Collect type feedback from bitwise binary ops handlers.

src/interpreter/interpreter.cc

Index: src/interpreter/interpreter.cc
diff --git a/src/interpreter/interpreter.cc b/src/interpreter/interpreter.cc
index acc69c43967019bac0b29301ed753a55330c5c41..f9d4d286d1181ac3bf854185c45e6fbe3d38a6de 100644
--- a/src/interpreter/interpreter.cc
+++ b/src/interpreter/interpreter.cc
@@ -776,6 +776,87 @@ void Interpreter::DoBinaryOpWithFeedback(InterpreterAssembler* assembler) {
   __ Dispatch();
 }
 
+template <class Generator>
+void Interpreter::DoBitwiseBinaryOpAndCollectFeedback(
+    InterpreterAssembler* assembler) {
+  Node* reg_index = __ BytecodeOperandReg(0);
+  Node* lhs = __ LoadRegister(reg_index);
+  Node* rhs = __ GetAccumulator();
+  Node* context = __ GetContext();
+  Node* slot_index = __ BytecodeOperandIdx(1);
+  Node* type_feedback_vector = __ LoadTypeFeedbackVector();
+
+  typedef InterpreterAssembler::Label Label;
+  typedef InterpreterAssembler::Variable Variable;
+
+  Variable var_type_feedback(assembler, MachineRepresentation::kWord32);
+  Label lhs_is_smi(assembler), lhs_is_not_smi(assembler),
+      collect_rhs_type(assembler);
+
+  __ BranchIf(__ WordIsSmi(lhs), &lhs_is_smi, &lhs_is_not_smi);
+  __ Bind(&lhs_is_smi);
+  {
+    var_type_feedback.Bind(
+        __ Int32Constant(BinaryOperationFeedback::kSignedSmall));
+    __ Goto(&collect_rhs_type);
+  }
+
+  __ Bind(&lhs_is_not_smi);
+  {
+    Node* lhs_map = __ LoadMap(lhs);
+    Node* type = __ Select(__ WordEqual(lhs_map, __ HeapNumberMapConstant()),
+                           __ Int32Constant(BinaryOperationFeedback::kNumber),

[Benedikt Meurer, 2016/08/11 09:32:21]

We should probably recognize kSigned32 here as well. This could be especially
relevant on 32-bit architectures.
At least we should have a TODO here that we need to revisit this.

[epertoso, 2016/08/11 12:18:57]

We don't have kSigned32 in BinaryOperationFeedback.

+                           __ Int32Constant(BinaryOperationFeedback::kAny));
+    var_type_feedback.Bind(type);
+    __ Goto(&collect_rhs_type);
+  }
+
+  __ Bind(&collect_rhs_type);
+  Label rhs_is_smi(assembler), rhs_is_not_smi(assembler),
+      do_binary_op(assembler);
+  __ BranchIf(__ WordIsSmi(rhs), &rhs_is_smi, &rhs_is_not_smi);
+  __ Bind(&rhs_is_smi);
+  {
+    var_type_feedback.Bind(
+        __ Word32Or(var_type_feedback.value(),
+                    __ Int32Constant(BinaryOperationFeedback::kSignedSmall)));
+    __ Goto(&do_binary_op);
+  }
+
+  __ Bind(&rhs_is_not_smi);
+  {
+    Node* rhs_map = __ LoadMap(rhs);
+    Node* type = __ Select(__ WordEqual(rhs_map, __ HeapNumberMapConstant()),
+                           __ Int32Constant(BinaryOperationFeedback::kNumber),

[Benedikt Meurer, 2016/08/11 09:32:21]

See comment above.

[epertoso, 2016/08/11 12:18:57]

See answer above.

+                           __ Int32Constant(BinaryOperationFeedback::kAny));
+    var_type_feedback.Bind(__ Word32Or(var_type_feedback.value(), type));
+    __ Goto(&do_binary_op);
+  }
+
+  __ Bind(&do_binary_op);
+  Node* result = Generator::Generate(assembler, lhs, rhs, context);
+
+  // Bitwise binary ops return 32-bit values.
+  Node* result_type =
+      __ Select(__ WordIsSmi(result),
+                __ Int32Constant(BinaryOperationFeedback::kSignedSmall),
+                __ Int32Constant(BinaryOperationFeedback::kNumber));

[Benedikt Meurer, 2016/08/11 09:32:21]

s/kNumber/kSigned32

[epertoso, 2016/08/11 12:18:57]

There's no kSigned32 constant.

+
+  if (FLAG_debug_code) {
+    Label ok(assembler);
+    __ GotoIf(__ WordIsSmi(result), &ok);
+    Node* result_map = __ LoadMap(result);
+    __ AbortIfWordNotEqual(result_map, __ HeapNumberMapConstant(),
+                           kExpectedHeapNumber);
+    __ Bind(&ok);
+  }
+  __ UpdateFeedback(__ Word32Or(result_type, var_type_feedback.value()),
+                    type_feedback_vector, slot_index);
+
+  __ SetAccumulator(result);
+  __ Dispatch();
+}
+
 // Add <src>
 //
 // Add register <src> to accumulator.
@@ -815,21 +896,21 @@ void Interpreter::DoMod(InterpreterAssembler* assembler) {
 //
 // BitwiseOr register <src> to accumulator.
 void Interpreter::DoBitwiseOr(InterpreterAssembler* assembler) {
-  DoBinaryOp<BitwiseOrStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<BitwiseOrStub>(assembler);
 }
 
 // BitwiseXor <src>
 //
 // BitwiseXor register <src> to accumulator.
 void Interpreter::DoBitwiseXor(InterpreterAssembler* assembler) {
-  DoBinaryOp<BitwiseXorStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<BitwiseXorStub>(assembler);
 }
 
 // BitwiseAnd <src>
 //
 // BitwiseAnd register <src> to accumulator.
 void Interpreter::DoBitwiseAnd(InterpreterAssembler* assembler) {
-  DoBinaryOp<BitwiseAndStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<BitwiseAndStub>(assembler);
 }
 
 // ShiftLeft <src>
@@ -839,7 +920,7 @@ void Interpreter::DoBitwiseAnd(InterpreterAssembler* assembler) {
 // before the operation. 5 lsb bits from the accumulator are used as count
 // i.e. <src> << (accumulator & 0x1F).
 void Interpreter::DoShiftLeft(InterpreterAssembler* assembler) {
-  DoBinaryOp<ShiftLeftStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<ShiftLeftStub>(assembler);
 }
 
 // ShiftRight <src>
@@ -849,7 +930,7 @@ void Interpreter::DoShiftLeft(InterpreterAssembler* assembler) {
 // accumulator to uint32 before the operation. 5 lsb bits from the accumulator
 // are used as count i.e. <src> >> (accumulator & 0x1F).
 void Interpreter::DoShiftRight(InterpreterAssembler* assembler) {
-  DoBinaryOp<ShiftRightStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<ShiftRightStub>(assembler);
 }
 
 // ShiftRightLogical <src>
@@ -859,7 +940,7 @@ void Interpreter::DoShiftRight(InterpreterAssembler* assembler) {
 // uint32 before the operation 5 lsb bits from the accumulator are used as
 // count i.e. <src> << (accumulator & 0x1F).
 void Interpreter::DoShiftRightLogical(InterpreterAssembler* assembler) {
-  DoBinaryOp<ShiftRightLogicalStub>(assembler);
+  DoBitwiseBinaryOpAndCollectFeedback<ShiftRightLogicalStub>(assembler);
 }
 
 // AddSmi <imm> <reg>