Reland of [interpreter] Add string type feedback to add

src/code-stubs.cc

Index: src/code-stubs.cc
diff --git a/src/code-stubs.cc b/src/code-stubs.cc
index de18700d5ee7856c0b302fdb7ef516f99e088fda..5b9f1d8e03c71ad21bb8a67a7fe101a19557a951 100644
--- a/src/code-stubs.cc
+++ b/src/code-stubs.cc
@@ -1107,12 +1107,14 @@ compiler::Node* AddWithFeedbackStub::Generate(
 
   assembler->Bind(&if_lhsisnotsmi);
   {
+    Label check_string(assembler);
+
     // Load the map of {lhs}.
     Node* lhs_map = assembler->LoadMap(lhs);
 
     // Check if {lhs} is a HeapNumber.
     Label if_lhsisnumber(assembler), if_lhsisnotnumber(assembler);
-    assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &call_add_stub);
+    assembler->GotoUnless(assembler->IsHeapNumberMap(lhs_map), &check_string);
 
     // Check if the {rhs} is Smi.
     Label if_rhsissmi(assembler), if_rhsisnotsmi(assembler);
@@ -1138,6 +1140,32 @@ compiler::Node* AddWithFeedbackStub::Generate(
       var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
       assembler->Goto(&do_fadd);
     }
+
+    assembler->Bind(&check_string);
+    {
+      // Check if the {rhs} is a smi, and exit the string check early if it is.
+      assembler->GotoIf(assembler->WordIsSmi(rhs), &call_add_stub);

[rmcilroy, 2016/10/04 08:05:31]

We should still count this as a string if the lhs is a string even if rhs is a
SMI (since the output will be string). Here we know that the lhs is a non-number
heap object, but we don't know anything about the rhs, so the code should be
something like:

assembler->Bind(&check_string);
 {
   // Check if lhs is a String.
   Node* lhs_instance_type = assembler->LoadMapInstanceType(lhs_map);
   assembler->GotoUnless(assembler->Int32LessThan(rhs_instance_type,
       assembler->Int32Constant(FIRST_NONSTRING_TYPE), &has_any_type);

   // Check if rhs is a Smi, HeapNumber or String.
   assembler->GotoIf(assembler->WordIsSmi(rhs), &has_string_type);
   Node* rhs_map = assembler->LoadMap(rhs);
   assembler->GotoIf(assembler->IsHeapNumberMap(rhs_map),
       &has_string_type);
   assembler->Branch(assembler->Int32LessThan(
                                rhs_instance_type,
                                assembler->Int32Constant(FIRST_NONSTRING_TYPE)
       &has_string_type,
       &has_any_type);

  assembler->Bind(&has_string_type);
  {
     var_type_feedback.Bind(
        assembler->Int32Constant(BinaryOperationFeedback::kString));
     assembler->Goto(&call_add_stub);
   }
 ...

[Leszek Swirski, 2016/10/04 08:30:40]

As above.

+
+      Node* lhs_instance_type = assembler->LoadMapInstanceType(lhs_map);
+
+      assembler->GotoUnless(assembler->Int32LessThan(
+                                lhs_instance_type,
+                                assembler->Int32Constant(FIRST_NONSTRING_TYPE)),
+                            &call_add_stub);
+
+      Node* rhs_instance_type = assembler->LoadInstanceType(rhs);
+
+      assembler->GotoUnless(assembler->Int32LessThan(
+                                rhs_instance_type,
+                                assembler->Int32Constant(FIRST_NONSTRING_TYPE)),
+                            &call_add_stub);
+
+      var_type_feedback.Bind(
+          assembler->Int32Constant(BinaryOperationFeedback::kString));
+      Callable callable = CodeFactory::Add(assembler->isolate());
+      var_result.Bind(assembler->CallStub(callable, context, lhs, rhs));

[rmcilroy, 2016/10/04 08:05:31]

Let's keep the calling of the stub and updating of the type feedback in one
place (see comments above).

[Leszek Swirski, 2016/10/04 08:30:40]

Not sure about this, calling the stub is a small amount of code and I think it
might be better to inline it to skip a jump (no pun intended).

+      assembler->Goto(&end);
+    }
   }
 
   assembler->Bind(&do_fadd);
@@ -1153,6 +1181,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
 
   assembler->Bind(&call_add_stub);
   {
+    assembler->Comment("Calling the stub");
     var_type_feedback.Bind(
         assembler->Int32Constant(BinaryOperationFeedback::kAny));
     Callable callable = CodeFactory::Add(assembler->isolate());
@@ -1161,6 +1190,7 @@ compiler::Node* AddWithFeedbackStub::Generate(
   }
 
   assembler->Bind(&end);
+  assembler->Comment("Updating the feedback");
   assembler->UpdateFeedback(var_type_feedback.value(), type_feedback_vector,
                             slot_id);
   return var_result.value();