[Interpreter] Collect type feedback for subtract operation and pass it to turbofan.

Collect type feedback for subtract operation in interpreter. Also use it in
bytecode-graph-bulder to set the correct Hint for subtract operation.

BUG=v8:5273
LOG=N

Review-Url: https://codereview.chromium.org/2221833002
Cr-Commit-Position: refs/heads/master@{#38467}

src/code-stubs.cc

@@ -1081,6 +1081,212 @@ compiler::Node* SubtractStub::Generate(CodeStubAssembler* assembler,
   return var_result.value();
 }
 
+// static
+compiler::Node* SubtractWithFeedbackStub::Generate(
+    CodeStubAssembler* assembler, compiler::Node* lhs, compiler::Node* rhs,
+    compiler::Node* context, compiler::Node* type_feedback_vector,
+    compiler::Node* slot_id) {
+  typedef CodeStubAssembler::Label Label;
+  typedef compiler::Node Node;
+  typedef CodeStubAssembler::Variable Variable;
+
+  // Shared entry for floating point subtraction.
+  Label do_fsub(assembler), record_feedback(assembler),
+      call_subtract_stub(assembler, Label::kDeferred);
+  Variable var_fsub_lhs(assembler, MachineRepresentation::kFloat64),
+      var_fsub_rhs(assembler, MachineRepresentation::kFloat64);
+  Variable var_type_feedback(assembler, MachineRepresentation::kWord32);
+  Variable var_result(assembler, MachineRepresentation::kTagged);
+
+  // Check if the {lhs} is a Smi or a HeapObject.
+  Label if_lhsissmi(assembler), if_lhsisnotsmi(assembler);
+  assembler->Branch(assembler->WordIsSmi(lhs), &if_lhsissmi, &if_lhsisnotsmi);
+
+  assembler->Bind(&if_lhsissmi);
+  {
+    // Check if the {rhs} is also a Smi.
+    Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler);
+    assembler->Branch(assembler->WordIsSmi(rhs), &if_rhsissmi, &if_rhsisnotsmi);
+
+    assembler->Bind(&if_rhsissmi);
+    {
+      // Try a fast Smi subtraction first.
+      Node* pair = assembler->SmiSubWithOverflow(lhs, rhs);
+      Node* overflow = assembler->Projection(1, pair);
+
+      // Check if the Smi subtraction overflowed.
+      Label if_overflow(assembler), if_notoverflow(assembler);
+      assembler->Branch(overflow, &if_overflow, &if_notoverflow);
+
+      assembler->Bind(&if_overflow);
+      {
+        // lhs, rhs - smi and result - number. combined - number.
+        // The result doesn't fit into Smi range.
+        var_type_feedback.Bind(
+            assembler->Int32Constant(BinaryOperationFeedback::kNumber));
+        var_fsub_lhs.Bind(assembler->SmiToFloat64(lhs));
+        var_fsub_rhs.Bind(assembler->SmiToFloat64(rhs));
+        assembler->Goto(&do_fsub);
+      }
+
+      assembler->Bind(&if_notoverflow);
+      // lhs, rhs, result smi. combined - smi.
+      var_type_feedback.Bind(
+          assembler->Int32Constant(BinaryOperationFeedback::kSignedSmall));
+      var_result.Bind(assembler->Projection(0, pair));
+      assembler->Goto(&record_feedback);
+    }
+
+    assembler->Bind(&if_rhsisnotsmi);
+    {
+      // Load the map of the {rhs}.
+      Node* rhs_map = assembler->LoadMap(rhs);
+
+      // Check if {rhs} is a HeapNumber.
+      Label if_rhsisnumber(assembler),
+          if_rhsisnotnumber(assembler, Label::kDeferred);
+      Node* number_map = assembler->HeapNumberMapConstant();
+      assembler->Branch(assembler->WordEqual(rhs_map, number_map),
+                        &if_rhsisnumber, &if_rhsisnotnumber);
+
+      assembler->Bind(&if_rhsisnumber);
+      {
+        var_type_feedback.Bind(
+            assembler->Int32Constant(BinaryOperationFeedback::kNumber));
+        // Perform a floating point subtraction.
+        var_fsub_lhs.Bind(assembler->SmiToFloat64(lhs));
+        var_fsub_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
+        assembler->Goto(&do_fsub);
+      }
+
+      assembler->Bind(&if_rhsisnotnumber);
+      {
+        var_type_feedback.Bind(
+            assembler->Int32Constant(BinaryOperationFeedback::kAny));
+        assembler->Goto(&call_subtract_stub);
+      }
+    }
+  }
+
+  assembler->Bind(&if_lhsisnotsmi);
+  {
+    // Load the map of the {lhs}.
+    Node* lhs_map = assembler->LoadMap(lhs);
+
+    // Check if the {lhs} is a HeapNumber.
+    Label if_lhsisnumber(assembler),
+        if_lhsisnotnumber(assembler, Label::kDeferred);
+    Node* number_map = assembler->HeapNumberMapConstant();
+    assembler->Branch(assembler->WordEqual(lhs_map, number_map),
+                      &if_lhsisnumber, &if_lhsisnotnumber);
+
+    assembler->Bind(&if_lhsisnumber);
+    {
+      // Check if the {rhs} is a Smi.
+      Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler);
+      assembler->Branch(assembler->WordIsSmi(rhs), &if_rhsissmi,
+                        &if_rhsisnotsmi);
+
+      assembler->Bind(&if_rhsissmi);
+      {
+        var_type_feedback.Bind(
+            assembler->Int32Constant(BinaryOperationFeedback::kNumber));
+        // Perform a floating point subtraction.
+        var_fsub_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
+        var_fsub_rhs.Bind(assembler->SmiToFloat64(rhs));
+        assembler->Goto(&do_fsub);
+      }
+
+      assembler->Bind(&if_rhsisnotsmi);
+      {
+        // Load the map of the {rhs}.
+        Node* rhs_map = assembler->LoadMap(rhs);
+
+        // Check if the {rhs} is a HeapNumber.
+        Label if_rhsisnumber(assembler),
+            if_rhsisnotnumber(assembler, Label::kDeferred);
+        assembler->Branch(assembler->WordEqual(rhs_map, number_map),
+                          &if_rhsisnumber, &if_rhsisnotnumber);
+
+        assembler->Bind(&if_rhsisnumber);
+        {
+          var_type_feedback.Bind(
+              assembler->Int32Constant(BinaryOperationFeedback::kNumber));
+          // Perform a floating point subtraction.
+          var_fsub_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
+          var_fsub_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
+          assembler->Goto(&do_fsub);
+        }
+
+        assembler->Bind(&if_rhsisnotnumber);
+        {
+          var_type_feedback.Bind(
+              assembler->Int32Constant(BinaryOperationFeedback::kAny));
+          assembler->Goto(&call_subtract_stub);
+        }
+      }
+    }
+
+    assembler->Bind(&if_lhsisnotnumber);
+    {
+      var_type_feedback.Bind(
+          assembler->Int32Constant(BinaryOperationFeedback::kAny));
+      assembler->Goto(&call_subtract_stub);
+    }
+  }
+
+  assembler->Bind(&do_fsub);
+  {
+    Node* lhs_value = var_fsub_lhs.value();
+    Node* rhs_value = var_fsub_rhs.value();
+    Node* value = assembler->Float64Sub(lhs_value, rhs_value);
+    var_result.Bind(assembler->ChangeFloat64ToTagged(value));
+    assembler->Goto(&record_feedback);
+  }
+
+  assembler->Bind(&call_subtract_stub);
+  {
+    Callable callable = CodeFactory::Subtract(assembler->isolate());
+    var_result.Bind(assembler->CallStub(callable, context, lhs, rhs));
+    assembler->Goto(&record_feedback);
+  }
+
+  assembler->Bind(&record_feedback);
+  Label combine_feedback(assembler), initialize_feedback(assembler),
+      return_value(assembler);
+
+  Node* previous_feedback =
+      assembler->LoadFixedArrayElement(type_feedback_vector, slot_id);
+  Node* is_uninitialized = assembler->WordEqual(
+      previous_feedback,
+      assembler->HeapConstant(
+          TypeFeedbackVector::UninitializedSentinel(assembler->isolate())));
+  assembler->BranchIf(is_uninitialized, &initialize_feedback,
+                      &combine_feedback);
+
+  assembler->Bind(&initialize_feedback);
+  {
+    assembler->StoreFixedArrayElement(
+        type_feedback_vector, slot_id,
+        assembler->SmiTag(var_type_feedback.value()), SKIP_WRITE_BARRIER);
+    assembler->Goto(&return_value);
+  }
+
+  assembler->Bind(&combine_feedback);
+  {
+    Node* previous_feedback_int = assembler->SmiUntag(previous_feedback);
+    Node* combined_feedback =
+        assembler->Word32Or(previous_feedback_int, var_type_feedback.value());
+    assembler->StoreFixedArrayElement(type_feedback_vector, slot_id,
+                                      assembler->SmiTag(combined_feedback),
+                                      SKIP_WRITE_BARRIER);
+    assembler->Goto(&return_value);
+  }
+
+  assembler->Bind(&return_value);
+  return var_result.value();
+}
+
 // static
 compiler::Node* MultiplyStub::Generate(CodeStubAssembler* assembler,
                                        compiler::Node* left,

src/code-stubs.h

@@ -122,6 +122,7 @@ namespace internal {
   V(StringLength)                             \
   V(Add)                                      \
   V(Subtract)                                 \
+  V(SubtractWithFeedback)                     \
   V(Multiply)                                 \
   V(Divide)                                   \
   V(Modulus)                                  \
@@ -432,6 +433,20 @@ class CodeStub BASE_EMBEDDED {
   }                                                                            \
   DEFINE_CODE_STUB(NAME, SUPER)
 
+#define DEFINE_TURBOFAN_BINARY_OP_CODE_STUB_WITH_FEEDBACK(NAME, SUPER)        \
+ public:                                                                      \
+  static compiler::Node* Generate(                                            \
+      CodeStubAssembler* assembler, compiler::Node* left,                     \
+      compiler::Node* right, compiler::Node* context,                         \
+      compiler::Node* type_feedback_vector, compiler::Node* slot_id);         \
+  void GenerateAssembly(CodeStubAssembler* assembler) const override {        \
+    assembler->Return(                                                        \
+        Generate(assembler, assembler->Parameter(0), assembler->Parameter(1), \
+                 assembler->Parameter(2), assembler->Parameter(3),            \
+                 assembler->Parameter(4)));                                   \
+  }                                                                           \
+  DEFINE_CODE_STUB(NAME, SUPER)
+
 #define DEFINE_TURBOFAN_UNARY_OP_CODE_STUB(NAME, SUPER)                \
  public:                                                               \
   static compiler::Node* Generate(CodeStubAssembler* assembler,        \
@@ -749,6 +764,16 @@ class SubtractStub final : public TurboFanCodeStub {
   DEFINE_TURBOFAN_BINARY_OP_CODE_STUB(Subtract, TurboFanCodeStub);
 };
 
+class SubtractWithFeedbackStub final : public TurboFanCodeStub {
+ public:
+  explicit SubtractWithFeedbackStub(Isolate* isolate)
+      : TurboFanCodeStub(isolate) {}
+
+  DEFINE_CALL_INTERFACE_DESCRIPTOR(BinaryOp);
+  DEFINE_TURBOFAN_BINARY_OP_CODE_STUB_WITH_FEEDBACK(SubtractWithFeedback,
+                                                    TurboFanCodeStub);
+};
+
 class MultiplyStub final : public TurboFanCodeStub {
  public:
   explicit MultiplyStub(Isolate* isolate) : TurboFanCodeStub(isolate) {}

src/compiler/bytecode-graph-builder.cc

@@ -1166,14 +1166,27 @@ void BytecodeGraphBuilder::BuildBinaryOp(const Operator* js_op) {
   environment()->BindAccumulator(node, &states);
 }
 
+// Helper function to create binary operation hint from the recorded type
+// feedback.
+BinaryOperationHints BytecodeGraphBuilder::GetBinaryOperationHint() {
+  FeedbackVectorSlot slot =
+      feedback_vector()->ToSlot(bytecode_iterator().GetIndexOperand(1));
+  DCHECK_EQ(FeedbackVectorSlotKind::GENERAL, feedback_vector()->GetKind(slot));
+  Object* feedback = feedback_vector()->Get(slot);
+  BinaryOperationHints::Hint hint = BinaryOperationHints::Hint::kAny;
+  if (feedback->IsSmi()) {
+    hint = BinaryOperationHintFromFeedback((Smi::cast(feedback))->value());
+  }
+  return BinaryOperationHints(hint, hint, hint);
+}
+
 void BytecodeGraphBuilder::VisitAdd() {
   BinaryOperationHints hints = BinaryOperationHints::Any();
   BuildBinaryOp(javascript()->Add(hints));
 }
 
 void BytecodeGraphBuilder::VisitSub() {
-  BinaryOperationHints hints = BinaryOperationHints::Any();
-  BuildBinaryOp(javascript()->Subtract(hints));
+  BuildBinaryOp(javascript()->Subtract(GetBinaryOperationHint()));
 }
 
 void BytecodeGraphBuilder::VisitMul() {

src/compiler/bytecode-graph-builder.h

@@ -9,6 +9,7 @@
 #include "src/compiler/bytecode-branch-analysis.h"
 #include "src/compiler/bytecode-loop-analysis.h"
 #include "src/compiler/js-graph.h"
+#include "src/compiler/type-hint-analyzer.h"
 #include "src/interpreter/bytecode-array-iterator.h"
 #include "src/interpreter/bytecode-flags.h"
 #include "src/interpreter/bytecodes.h"
@@ -134,6 +135,10 @@ class BytecodeGraphBuilder {
   void BuildForInNext();
   void BuildInvokeIntrinsic();
 
+  // Helper function to create binary operation hint from the recorded
+  // type feedback.
+  BinaryOperationHints GetBinaryOperationHint();
+
   // Control flow plumbing.
   void BuildJump();
   void BuildConditionalJump(Node* condition);

src/compiler/type-hint-analyzer.cc

@@ -139,6 +139,20 @@ TypeHintAnalysis* TypeHintAnalyzer::Analyze(Handle<Code> code) {
   return new (zone()) TypeHintAnalysis(infos, zone());
 }
 
+// Helper function to transform the feedback to BinaryOperationHints
+BinaryOperationHints::Hint BinaryOperationHintFromFeedback(int type_feedback) {
+  switch (type_feedback) {
+    case BinaryOperationFeedback::kSignedSmall:
+      return BinaryOperationHints::kSigned32;
+    case BinaryOperationFeedback::kNumber:
+      return BinaryOperationHints::kNumberOrOddball;
+    case BinaryOperationFeedback::kAny:
+    default:
+      return BinaryOperationHints::kAny;
+  }
+  return BinaryOperationHints::kAny;
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/type-hint-analyzer.h

@@ -50,6 +50,8 @@ class TypeHintAnalyzer final {
   DISALLOW_COPY_AND_ASSIGN(TypeHintAnalyzer);
 };
 
+BinaryOperationHints::Hint BinaryOperationHintFromFeedback(int type_feedback);
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/globals.h

@@ -1148,6 +1148,15 @@ inline uint32_t ObjectHash(Address address) {
                                kPointerSizeLog2);
 }
 
+// Type feedbac is encoded in such a way that, we can combine the feedback
+// at different points by performing an 'OR' operation. Type feedback moves
+// to a more generic type when we combine feedback.
+// kSignedSmall -> kNumber  -> kAny
+class BinaryOperationFeedback {
+ public:
+  enum { kNone = 0x00, kSignedSmall = 0x01, kNumber = 0x3, kAny = 0x7 };
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/interpreter/interpreter.cc

@@ -749,6 +749,7 @@ void Interpreter::DoPopContext(InterpreterAssembler* assembler) {
   __ Dispatch();
 }
 
+// TODO(mythria): Remove this function once all BinaryOps record type feedback.
 template <class Generator>
 void Interpreter::DoBinaryOp(InterpreterAssembler* assembler) {
   Node* reg_index = __ BytecodeOperandReg(0);
@@ -760,6 +761,20 @@ void Interpreter::DoBinaryOp(InterpreterAssembler* assembler) {
   __ Dispatch();
 }
 
+template <class Generator>
+void Interpreter::DoBinaryOpWithFeedback(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();
+  Node* result = Generator::Generate(assembler, lhs, rhs, context,
+                                     type_feedback_vector, slot_index);
+  __ SetAccumulator(result);
+  __ Dispatch();
+}
+
 // Add <src>
 //
 // Add register <src> to accumulator.
@@ -771,7 +786,7 @@ void Interpreter::DoAdd(InterpreterAssembler* assembler) {
 //
 // Subtract register <src> from accumulator.
 void Interpreter::DoSub(InterpreterAssembler* assembler) {
-  DoBinaryOp<SubtractStub>(assembler);
+  DoBinaryOpWithFeedback<SubtractWithFeedbackStub>(assembler);
 }
 
 // Mul <src>

src/interpreter/interpreter.h

@@ -79,6 +79,10 @@ class Interpreter {
   template <class Generator>
   void DoBinaryOp(InterpreterAssembler* assembler);
 
+  // Generates code to perform the binary operation via |Generator|.
+  template <class Generator>
+  void DoBinaryOpWithFeedback(InterpreterAssembler* assembler);
+
   // Generates code to perform the binary operation via |Generator| using
   // an immediate value rather the accumulator as the rhs operand.
   template <class Generator>