Reland of [interpreter] Add string type feedback to add

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/ast/ast.h"
 #include "src/bootstrapper.h"
@@ skipping 1030 matching lines @@
 compiler::Node* AddWithFeedbackStub::Generate(
     CodeStubAssembler* assembler, compiler::Node* lhs, compiler::Node* rhs,
     compiler::Node* slot_id, compiler::Node* type_feedback_vector,
     compiler::Node* context) {
   typedef CodeStubAssembler::Label Label;
   typedef compiler::Node Node;
   typedef CodeStubAssembler::Variable Variable;
 
   // Shared entry for floating point addition.
   Label do_fadd(assembler), end(assembler),
-      call_add_stub(assembler, Label::kDeferred);
+      do_add_any(assembler, Label::kDeferred), call_add_stub(assembler);
   Variable var_fadd_lhs(assembler, MachineRepresentation::kFloat64),
       var_fadd_rhs(assembler, MachineRepresentation::kFloat64),
       var_type_feedback(assembler, MachineRepresentation::kWord32),
       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);
@@ skipping 27 matching lines @@
         assembler->Goto(&end);
       }
     }
 
     assembler->Bind(&if_rhsisnotsmi);
     {
       // Load the map of {rhs}.
       Node* rhs_map = assembler->LoadMap(rhs);
 
       // Check if the {rhs} is a HeapNumber.
-      assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
-                            &call_add_stub);
+      assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
 
       var_fadd_lhs.Bind(assembler->SmiToFloat64(lhs));
       var_fadd_rhs.Bind(assembler->LoadHeapNumberValue(rhs));
       assembler->Goto(&do_fadd);
     }
   }
 
   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);
     assembler->Branch(assembler->WordIsSmi(rhs), &if_rhsissmi, &if_rhsisnotsmi);
 
     assembler->Bind(&if_rhsissmi);
     {
       var_fadd_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
       var_fadd_rhs.Bind(assembler->SmiToFloat64(rhs));
       assembler->Goto(&do_fadd);
     }
 
     assembler->Bind(&if_rhsisnotsmi);
     {
       // Load the map of {rhs}.
       Node* rhs_map = assembler->LoadMap(rhs);
 
       // Check if the {rhs} is a HeapNumber.
-      assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map),
-                            &call_add_stub);
+      assembler->GotoUnless(assembler->IsHeapNumberMap(rhs_map), &do_add_any);
 
       var_fadd_lhs.Bind(assembler->LoadHeapNumberValue(lhs));
       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), &do_add_any);
+
+      Node* lhs_instance_type = assembler->LoadMapInstanceType(lhs_map);
+
+      // Exit unless {lhs} is a string
+      assembler->GotoUnless(assembler->Int32LessThan(
+                                lhs_instance_type,
+                                assembler->Int32Constant(FIRST_NONSTRING_TYPE)),
+                            &do_add_any);

[rmcilroy, 2016/10/05 11:03:12]

Could you use IsStringInstanceType (as added in
https://codereview.chromium.org/2395453002/).

[Leszek Swirski, 2016/10/05 13:10:55]

Done.

+
+      Node* rhs_instance_type = assembler->LoadInstanceType(rhs);
+
+      // Exit unless {rhs} is a string
+      assembler->GotoUnless(assembler->Int32LessThan(
+                                rhs_instance_type,
+                                assembler->Int32Constant(FIRST_NONSTRING_TYPE)),
+                            &do_add_any);

[rmcilroy, 2016/10/05 11:03:12]

ditto

[Leszek Swirski, 2016/10/05 13:10:55]

Done.

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

[mythria, 2016/10/05 08:28:00]

I am not sure if it makes sense, but would it be possible to directly have code
to add two strings rather than going to calling the add stub. If the operation
is too complex it may increase the code size, but if it is simple enough we may
avoid some checks. 

[Leszek Swirski, 2016/10/05 13:10:55]

Nice idea, I'll punt on this for this CL and will investigate for a subsequent
one.

+    }
   }
 
   assembler->Bind(&do_fadd);
   {
     var_type_feedback.Bind(
         assembler->Int32Constant(BinaryOperationFeedback::kNumber));
     Node* value =
         assembler->Float64Add(var_fadd_lhs.value(), var_fadd_rhs.value());
     Node* result = assembler->ChangeFloat64ToTagged(value);
     var_result.Bind(result);
     assembler->Goto(&end);
   }
 
+  assembler->Bind(&do_add_any);
+  {
+    var_type_feedback.Bind(
+        assembler->Int32Constant(BinaryOperationFeedback::kAny));
+    assembler->Goto(&call_add_stub);
+  }
+
   assembler->Bind(&call_add_stub);
   {
-    var_type_feedback.Bind(
-        assembler->Int32Constant(BinaryOperationFeedback::kAny));
     Callable callable = CodeFactory::Add(assembler->isolate());
     var_result.Bind(assembler->CallStub(callable, context, lhs, rhs));
     assembler->Goto(&end);
   }
 
   assembler->Bind(&end);
   assembler->UpdateFeedback(var_type_feedback.value(), type_feedback_vector,
                             slot_id);
   return var_result.value();
 }
@@ skipping 4704 matching lines @@
 
   if (type == MachineType::Pointer()) {
     return Representation::External();
   }
 
   return Representation::Tagged();
 }
 
 }  // namespace internal
 }  // namespace v8