[crankshaft] Re-add fast-case for string add left/right.
authorbmeurer <bmeurer@chromium.org>
Tue, 22 Sep 2015 05:46:36 +0000 (22:46 -0700)
committerCommit bot <commit-bot@chromium.org>
Tue, 22 Sep 2015 05:46:46 +0000 (05:46 +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.

R=jarin@chromium.org
BUG=v8:4307, chromium:532524
LOG=n

Committed: https://crrev.com/d261849e53fbf8c36efae42d478271f87acff70f
Cr-Commit-Position: refs/heads/master@{#30726}

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

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

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 8bd7c54..06e7a65 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 20da57f..4b259b6 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 1d9d3ac..4482155 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 a59cd2f..be03186 100644 (file)
@@ -7348,8 +7348,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());
 
@@ -7371,16 +7370,21 @@ 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());
-    SetFlag(kUseGVN);
+    if ((flags & STRING_ADD_CONVERT) == STRING_ADD_CONVERT) {
+      SetAllSideEffects();
+      ClearFlag(kUseGVN);
+    } else {
+      SetChangesFlag(kNewSpacePromotion);
+      SetFlag(kUseGVN);
+    }
     SetDependsOnFlag(kMaps);
-    SetChangesFlag(kNewSpacePromotion);
     if (FLAG_trace_pretenuring) {
       PrintF("HStringAdd with AllocationSite %p %s\n",
              allocation_site.is_null()
@@ -7390,8 +7394,9 @@ class HStringAdd final : public HBinaryOperation {
     }
   }
 
-  // No side-effects except possible allocation:
-  bool IsDeletable() const override { return true; }
+  bool IsDeletable() const final {
+    return (flags_ & STRING_ADD_CONVERT) != STRING_ADD_CONVERT;
+  }
 
   const StringAddFlags flags_;
   const PretenureFlag pretenure_flag_;
index 949562f..b24890a 100644 (file)
@@ -11014,11 +11014,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.
@@ -11027,11 +11025,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());
       }
     }
 
@@ -11048,7 +11044,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());
     }
 
@@ -11077,8 +11073,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()) {
@@ -12536,8 +12532,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());
 }