[crankshaft] Re-add fast-case for string add left/right.
authorbmeurer <bmeurer@chromium.org>
Tue, 15 Sep 2015 06:00:33 +0000 (23:00 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 15 Sep 2015 06:00:49 +0000 (06:00 +0000)
Now the StringAddStub can optionally convert it's parameters to strings
(following the rules for the addition operator). This could be further
optimized once we have a ToPrimitiveStub, but it should be sufficient
for the moment.

Also removed the unused Strength parameter to the HStringAdd operator,
because string addition does not depend on language mode.

CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_win_nosnap_shared_rel,v8_linux_nosnap_dbg
R=mstarzinger@chromium.org
BUG=v8:4307
LOG=n

Review URL: https://codereview.chromium.org/1339053002

Cr-Commit-Position: refs/heads/master@{#30726}

src/code-stubs-hydrogen.cc
src/code-stubs.cc
src/code-stubs.h
src/hydrogen-instructions.cc
src/hydrogen-instructions.h
src/hydrogen.cc

index 9f9b14e..43cf265 100644 (file)
@@ -113,6 +113,8 @@ class CodeStubGraphBuilderBase : public HGraphBuilder {
                                         HValue* shared_info,
                                         HValue* native_context);
 
+  HValue* CheckString(HValue* input, bool convert);
+
  private:
   HValue* BuildArraySingleArgumentConstructor(JSArrayBuilder* builder);
   HValue* BuildArrayNArgumentsConstructor(JSArrayBuilder* builder,
@@ -1456,6 +1458,66 @@ Handle<Code> BinaryOpWithAllocationSiteStub::GenerateCode() {
 }
 
 
+HValue* CodeStubGraphBuilderBase::CheckString(HValue* input, bool convert) {
+  if (!convert) return BuildCheckString(input);
+  IfBuilder if_inputissmi(this);
+  HValue* inputissmi = if_inputissmi.If<HIsSmiAndBranch>(input);
+  if_inputissmi.Then();
+  {
+    // Convert the input smi to a string.
+    Push(BuildNumberToString(input, Type::SignedSmall()));
+  }
+  if_inputissmi.Else();
+  {
+    HValue* input_map =
+        Add<HLoadNamedField>(input, inputissmi, HObjectAccess::ForMap());
+    HValue* input_instance_type = Add<HLoadNamedField>(
+        input_map, inputissmi, HObjectAccess::ForMapInstanceType());
+    IfBuilder if_inputisstring(this);
+    if_inputisstring.If<HCompareNumericAndBranch>(
+        input_instance_type, Add<HConstant>(FIRST_NONSTRING_TYPE), Token::LT);
+    if_inputisstring.Then();
+    {
+      // The input is already a string.
+      Push(input);
+    }
+    if_inputisstring.Else();
+    {
+      // Convert to primitive first (if necessary), see
+      // ES6 section 12.7.3 The Addition operator.
+      IfBuilder if_inputisprimitive(this);
+      STATIC_ASSERT(FIRST_PRIMITIVE_TYPE == FIRST_TYPE);
+      if_inputisprimitive.If<HCompareNumericAndBranch>(
+          input_instance_type, Add<HConstant>(LAST_PRIMITIVE_TYPE), Token::LTE);
+      if_inputisprimitive.Then();
+      {
+        // The input is already a primitive.
+        Push(input);
+      }
+      if_inputisprimitive.Else();
+      {
+        // TODO(bmeurer): Add support for fast ToPrimitive conversion using
+        // a dedicated ToPrimitiveStub.
+        Add<HPushArguments>(input);
+        Push(Add<HCallRuntime>(Runtime::FunctionForId(Runtime::kToPrimitive),
+                               1));
+      }
+      if_inputisprimitive.End();
+      // Convert the primitive to a string value.
+      ToStringDescriptor descriptor(isolate());
+      ToStringStub stub(isolate());
+      HValue* values[] = {context(), Pop()};
+      Push(AddUncasted<HCallWithDescriptor>(
+          Add<HConstant>(stub.GetCode()), 0, descriptor,
+          Vector<HValue*>(values, arraysize(values))));
+    }
+    if_inputisstring.End();
+  }
+  if_inputissmi.End();
+  return Pop();
+}
+
+
 template <>
 HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() {
   StringAddStub* stub = casted_stub();
@@ -1467,10 +1529,12 @@ HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() {
 
   // Make sure that both arguments are strings if not known in advance.
   if ((flags & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) {
-    left = BuildCheckString(left);
+    left =
+        CheckString(left, (flags & STRING_ADD_CONVERT) == STRING_ADD_CONVERT);
   }
   if ((flags & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) {
-    right = BuildCheckString(right);
+    right =
+        CheckString(right, (flags & STRING_ADD_CONVERT) == STRING_ADD_CONVERT);
   }
 
   return BuildStringAdd(left, right, HAllocationMode(pretenure_flag));
index e9fddf9..db335fc 100644 (file)
@@ -324,6 +324,12 @@ std::ostream& operator<<(std::ostream& os, const StringAddFlags& flags) {
       return os << "CheckRight";
     case STRING_ADD_CHECK_BOTH:
       return os << "CheckBoth";
+    case STRING_ADD_CONVERT_LEFT:
+      return os << "ConvertLeft";
+    case STRING_ADD_CONVERT_RIGHT:
+      return os << "ConvertRight";
+    case STRING_ADD_CONVERT:
+      break;
   }
   UNREACHABLE();
   return os;
index f793e6e..b875b94 100644 (file)
@@ -676,7 +676,11 @@ enum StringAddFlags {
   // Check right parameter.
   STRING_ADD_CHECK_RIGHT = 1 << 1,
   // Check both parameters.
-  STRING_ADD_CHECK_BOTH = STRING_ADD_CHECK_LEFT | STRING_ADD_CHECK_RIGHT
+  STRING_ADD_CHECK_BOTH = STRING_ADD_CHECK_LEFT | STRING_ADD_CHECK_RIGHT,
+  // Convert parameters when check fails (instead of throwing an exception).
+  STRING_ADD_CONVERT = 1 << 2,
+  STRING_ADD_CONVERT_LEFT = STRING_ADD_CHECK_LEFT | STRING_ADD_CONVERT,
+  STRING_ADD_CONVERT_RIGHT = STRING_ADD_CHECK_RIGHT | STRING_ADD_CONVERT
 };
 
 
@@ -701,8 +705,8 @@ class StringAddTFStub : public TurboFanCodeStub {
   }
 
  private:
-  class StringAddFlagsBits : public BitField<StringAddFlags, 0, 2> {};
-  class PretenureFlagBits : public BitField<PretenureFlag, 2, 1> {};
+  class StringAddFlagsBits : public BitField<StringAddFlags, 0, 3> {};
+  class PretenureFlagBits : public BitField<PretenureFlag, 3, 1> {};
 
   void PrintBaseName(std::ostream& os) const override;  // NOLINT
 
@@ -1647,8 +1651,8 @@ class StringAddStub final : public HydrogenCodeStub {
   static const int kRight = 1;
 
  private:
-  class StringAddFlagsBits: public BitField<StringAddFlags, 0, 2> {};
-  class PretenureFlagBits: public BitField<PretenureFlag, 2, 1> {};
+  class StringAddFlagsBits : public BitField<StringAddFlags, 0, 3> {};
+  class PretenureFlagBits : public BitField<PretenureFlag, 3, 1> {};
 
   void PrintBaseName(std::ostream& os) const override;  // NOLINT
 
index f65b0c2..f2d9c0a 100644 (file)
@@ -3997,7 +3997,7 @@ DEFINE_NEW_H_SIMPLE_ARITHMETIC_INSTR(HSub, -)
 
 
 HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context,
-                              HValue* left, HValue* right, Strength strength,
+                              HValue* left, HValue* right,
                               PretenureFlag pretenure_flag,
                               StringAddFlags flags,
                               Handle<AllocationSite> allocation_site) {
@@ -4015,8 +4015,8 @@ HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context,
       }
     }
   }
-  return new (zone) HStringAdd(context, left, right, strength, pretenure_flag,
-                               flags, allocation_site);
+  return new (zone)
+      HStringAdd(context, left, right, pretenure_flag, flags, allocation_site);
 }
 
 
index f81d877..9e68296 100644 (file)
@@ -7355,8 +7355,7 @@ class HStringAdd final : public HBinaryOperation {
  public:
   static HInstruction* New(
       Isolate* isolate, Zone* zone, HValue* context, HValue* left,
-      HValue* right, Strength strength = Strength::WEAK,
-      PretenureFlag pretenure_flag = NOT_TENURED,
+      HValue* right, PretenureFlag pretenure_flag = NOT_TENURED,
       StringAddFlags flags = STRING_ADD_CHECK_BOTH,
       Handle<AllocationSite> allocation_site = Handle<AllocationSite>::null());
 
@@ -7378,10 +7377,10 @@ class HStringAdd final : public HBinaryOperation {
   }
 
  private:
-  HStringAdd(HValue* context, HValue* left, HValue* right, Strength strength,
+  HStringAdd(HValue* context, HValue* left, HValue* right,
              PretenureFlag pretenure_flag, StringAddFlags flags,
              Handle<AllocationSite> allocation_site)
-      : HBinaryOperation(context, left, right, strength, HType::String()),
+      : HBinaryOperation(context, left, right, Strength::WEAK, HType::String()),
         flags_(flags),
         pretenure_flag_(pretenure_flag) {
     set_representation(Representation::Tagged());
index f417485..86d1a05 100644 (file)
@@ -10995,11 +10995,9 @@ HValue* HGraphBuilder::BuildBinaryOperation(
         left = BuildNumberToString(left, left_type);
       } else if (!left_type->Is(Type::String())) {
         DCHECK(right_type->Is(Type::String()));
-        // TODO(bmeurer): We might want to optimize this, because we already
-        // know that the right hand side is a string.
-        Add<HPushArguments>(left, right);
-        return AddUncasted<HCallRuntime>(Runtime::FunctionForId(Runtime::kAdd),
-                                         2);
+        return AddUncasted<HStringAdd>(
+            left, right, allocation_mode.GetPretenureMode(),
+            STRING_ADD_CONVERT_LEFT, allocation_mode.feedback_site());
       }
 
       // Convert right argument as necessary.
@@ -11008,11 +11006,9 @@ HValue* HGraphBuilder::BuildBinaryOperation(
         right = BuildNumberToString(right, right_type);
       } else if (!right_type->Is(Type::String())) {
         DCHECK(left_type->Is(Type::String()));
-        // TODO(bmeurer): We might want to optimize this, because we already
-        // know that the left hand side is a string.
-        Add<HPushArguments>(left, right);
-        return AddUncasted<HCallRuntime>(Runtime::FunctionForId(Runtime::kAdd),
-                                         2);
+        return AddUncasted<HStringAdd>(
+            left, right, allocation_mode.GetPretenureMode(),
+            STRING_ADD_CONVERT_RIGHT, allocation_mode.feedback_site());
       }
     }
 
@@ -11029,7 +11025,7 @@ HValue* HGraphBuilder::BuildBinaryOperation(
     if (!right_string.is_null() && right_string->length() == 0) return left;
     if (!left_string.is_null() && !right_string.is_null()) {
       return AddUncasted<HStringAdd>(
-          left, right, strength, allocation_mode.GetPretenureMode(),
+          left, right, allocation_mode.GetPretenureMode(),
           STRING_ADD_CHECK_NONE, allocation_mode.feedback_site());
     }
 
@@ -11058,8 +11054,8 @@ HValue* HGraphBuilder::BuildBinaryOperation(
 
     // Fallback to using the string add stub.
     return AddUncasted<HStringAdd>(
-        left, right, strength, allocation_mode.GetPretenureMode(),
-        STRING_ADD_CHECK_NONE, allocation_mode.feedback_site());
+        left, right, allocation_mode.GetPretenureMode(), STRING_ADD_CHECK_NONE,
+        allocation_mode.feedback_site());
   }
 
   if (graph()->info()->IsStub()) {
@@ -12472,8 +12468,7 @@ void HOptimizedGraphBuilder::GenerateStringAdd(CallRuntime* call) {
   CHECK_ALIVE(VisitForValue(call->arguments()->at(1)));
   HValue* right = Pop();
   HValue* left = Pop();
-  HInstruction* result =
-      NewUncasted<HStringAdd>(left, right, strength(function_language_mode()));
+  HInstruction* result = NewUncasted<HStringAdd>(left, right);
   return ast_context()->ReturnInstruction(result, call->id());
 }