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

src/code-stubs.cc

 // Copyright 2012 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/code-stubs.h"
 
 #include <sstream>
 
 #include "src/bootstrapper.h"
 #include "src/code-factory.h"
@@ skipping 892 matching lines @@
     assembler->Goto(&end);
   }
   assembler->Bind(&end);
   return var_result.value();
 }
 
 // static
 compiler::Node* SubtractStub::Generate(CodeStubAssembler* assembler,
                                        compiler::Node* left,
                                        compiler::Node* right,
-                                       compiler::Node* context) {
+                                       compiler::Node* context,
+                                       compiler::Node* type_feedback_vector,
+                                       compiler::Node* slot_id) {
   typedef CodeStubAssembler::Label Label;
   typedef compiler::Node Node;
   typedef CodeStubAssembler::Variable Variable;
 
+  bool collect_type_feedback = (type_feedback_vector != NULL);
+
   // Shared entry for floating point subtraction.
   Label do_fsub(assembler), end(assembler);
   Variable var_fsub_lhs(assembler, MachineRepresentation::kFloat64),
       var_fsub_rhs(assembler, MachineRepresentation::kFloat64);
+  Variable var_type_feedback(assembler, MachineRepresentation::kWord32);

[Benedikt Meurer, 2016/08/08 04:56:13]

I'm not sure if this will scale, because

(a) it'll affect the code generated for the non feedback case, as we cannot get
rid of the dead feedback collection code, and
(b) makes these already tricky stubs a bit harder to read.

I'm tempted to suggest that we just make copies of the code for feedback; also
since we only need the fast case for feedback anyways (i.e. both sides at least
number), we could also only inline that into the bytecode handler and call out
to the stub for everything else (setting the feedback to any before).

[mythria, 2016/08/08 07:17:36]

Thanks Benedikt, I add a subtract stub that deals with Numbers that collects
type feedback. I did not try optimizing so we can directly jump to ToNumber
case. Done.

 
   // We might need to loop several times due to ToPrimitive and/or ToNumber
   // conversions.
   Variable var_lhs(assembler, MachineRepresentation::kTagged),
       var_rhs(assembler, MachineRepresentation::kTagged),
       var_result(assembler, MachineRepresentation::kTagged);
-  Variable* loop_vars[2] = {&var_lhs, &var_rhs};
-  Label loop(assembler, 2, loop_vars);
+  Variable* loop_vars[3] = {&var_lhs, &var_rhs, &var_type_feedback};
+  Label loop(assembler, 3, loop_vars);
   var_lhs.Bind(left);
   var_rhs.Bind(right);
+  // Start with Smi and keep growing up the lattice. This would avoid
+  // incorrectly collecting Smi type after the ToNumber conversion.
+  var_type_feedback.Bind(
+      assembler->Int32Constant(BinaryOpTypeFeedback::SmiType::encode(1)));
   assembler->Goto(&loop);
+
   assembler->Bind(&loop);
   {
     // Load the current {lhs} and {rhs} values.
     Node* lhs = var_lhs.value();
     Node* rhs = var_rhs.value();
+    Node* type_feedback = var_type_feedback.value();
 
     // 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.
+          Node* current_feedback = assembler->Int32Constant(
+              BinaryOpTypeFeedback::NumberType::encode(1));
+          var_type_feedback.Bind(
+              assembler->Word32Or(type_feedback, current_feedback));
           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.
+        Node* smi_feedback =
+            assembler->Int32Constant(BinaryOpTypeFeedback::SmiType::encode(1));
+        var_type_feedback.Bind(
+            assembler->Word32Or(type_feedback, smi_feedback));
         var_result.Bind(assembler->Projection(0, pair));
         assembler->Goto(&end);
       }
 
       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);
         {
+          Node* current_feedback = assembler->Int32Constant(
+              BinaryOpTypeFeedback::NumberType::encode(1));
+          var_type_feedback.Bind(
+              assembler->Word32Or(type_feedback, current_feedback));
           // 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);
         {
+          Node* current_feedback = assembler->Int32Constant(
+              BinaryOpTypeFeedback::AnyType::encode(1));
+          var_type_feedback.Bind(
+              assembler->Word32Or(type_feedback, current_feedback));
           // Convert the {rhs} to a Number first.
           Callable callable =
               CodeFactory::NonNumberToNumber(assembler->isolate());
           var_rhs.Bind(assembler->CallStub(callable, context, rhs));
           assembler->Goto(&loop);
         }
       }
     }
 
     assembler->Bind(&if_lhsisnotsmi);
@@ skipping 10 matching lines @@
 
       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);
         {
+          Node* current_feedback = assembler->Int32Constant(
+              BinaryOpTypeFeedback::NumberType::encode(1));
+          var_type_feedback.Bind(
+              assembler->Word32Or(type_feedback, current_feedback));
           // 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);
           {
+            Node* current_feedback = assembler->Int32Constant(
+                BinaryOpTypeFeedback::NumberType::encode(1));
+            var_type_feedback.Bind(
+                assembler->Word32Or(type_feedback, current_feedback));
             // 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);
           {
+            Node* current_feedback = assembler->Int32Constant(
+                BinaryOpTypeFeedback::AnyType::encode(1));
+            var_type_feedback.Bind(
+                assembler->Word32Or(type_feedback, current_feedback));
             // Convert the {rhs} to a Number first.
             Callable callable =
                 CodeFactory::NonNumberToNumber(assembler->isolate());
             var_rhs.Bind(assembler->CallStub(callable, context, rhs));
             assembler->Goto(&loop);
           }
         }
       }
 
       assembler->Bind(&if_lhsisnotnumber);
       {
+        Node* current_feedback =
+            assembler->Int32Constant(BinaryOpTypeFeedback::AnyType::encode(1));
+        var_type_feedback.Bind(
+            assembler->Word32Or(type_feedback, current_feedback));
         // Convert the {lhs} to a Number first.
         Callable callable =
             CodeFactory::NonNumberToNumber(assembler->isolate());
         var_lhs.Bind(assembler->CallStub(callable, context, lhs));
         assembler->Goto(&loop);
       }
     }
   }
 
   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(&end);
   }
+
   assembler->Bind(&end);
+  if (collect_type_feedback) {
+    // Combine the current type with previous feedback and store it into the
+    // feedback slot.
+    Node* previous_feedback_smi =
+        assembler->LoadFixedArrayElement(type_feedback_vector, slot_id);
+    // TODO(mythria): Add a DCHECK to verify the loaded value is an Smi.
+    // ASSERT (WordIsSmi(previous_feedback));
+    Node* previous_feedback = assembler->SmiUntag(previous_feedback_smi);
+    Node* combined_feedback =
+        assembler->Word32Or(previous_feedback, var_type_feedback.value());
+    assembler->StoreFixedArrayElement(type_feedback_vector, slot_id,
+                                      assembler->SmiTag(combined_feedback),
+                                      SKIP_WRITE_BARRIER);
+  }
   return var_result.value();
 }
 
 // static
 compiler::Node* MultiplyStub::Generate(CodeStubAssembler* assembler,
                                        compiler::Node* left,
                                        compiler::Node* right,
                                        compiler::Node* context) {
   using compiler::Node;
   typedef CodeStubAssembler::Label Label;
@@ skipping 3963 matching lines @@
   if (type->Is(Type::UntaggedPointer())) {
     return Representation::External();
   }
 
   DCHECK(!type->Is(Type::Untagged()));
   return Representation::Tagged();
 }
 
 }  // namespace internal
 }  // namespace v8