From 4542a2d4cc2866a8097d10f11768683bc5ccb224 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Mon, 21 Sep 2015 22:46:36 -0700 Subject: [PATCH] [crankshaft] Re-add fast-case for string add left/right. 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 | 68 ++++++++++++++++++++++++++++++++++++++++++-- src/code-stubs.cc | 6 ++++ src/code-stubs.h | 14 +++++---- src/hydrogen-instructions.cc | 6 ++-- src/hydrogen-instructions.h | 21 ++++++++------ src/hydrogen.cc | 25 +++++++--------- 6 files changed, 107 insertions(+), 33 deletions(-) diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc index 9f9b14e..43cf265 100644 --- a/src/code-stubs-hydrogen.cc +++ b/src/code-stubs-hydrogen.cc @@ -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 BinaryOpWithAllocationSiteStub::GenerateCode() { } +HValue* CodeStubGraphBuilderBase::CheckString(HValue* input, bool convert) { + if (!convert) return BuildCheckString(input); + IfBuilder if_inputissmi(this); + HValue* inputissmi = if_inputissmi.If(input); + if_inputissmi.Then(); + { + // Convert the input smi to a string. + Push(BuildNumberToString(input, Type::SignedSmall())); + } + if_inputissmi.Else(); + { + HValue* input_map = + Add(input, inputissmi, HObjectAccess::ForMap()); + HValue* input_instance_type = Add( + input_map, inputissmi, HObjectAccess::ForMapInstanceType()); + IfBuilder if_inputisstring(this); + if_inputisstring.If( + input_instance_type, Add(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( + input_instance_type, Add(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(input); + Push(Add(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( + Add(stub.GetCode()), 0, descriptor, + Vector(values, arraysize(values)))); + } + if_inputisstring.End(); + } + if_inputissmi.End(); + return Pop(); +} + + template <> HValue* CodeStubGraphBuilder::BuildCodeInitializedStub() { StringAddStub* stub = casted_stub(); @@ -1467,10 +1529,12 @@ HValue* CodeStubGraphBuilder::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)); diff --git a/src/code-stubs.cc b/src/code-stubs.cc index 8bd7c54..06e7a65 100644 --- a/src/code-stubs.cc +++ b/src/code-stubs.cc @@ -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; diff --git a/src/code-stubs.h b/src/code-stubs.h index 20da57f..4b259b6 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -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 {}; - class PretenureFlagBits : public BitField {}; + class StringAddFlagsBits : public BitField {}; + class PretenureFlagBits : public BitField {}; 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 {}; - class PretenureFlagBits: public BitField {}; + class StringAddFlagsBits : public BitField {}; + class PretenureFlagBits : public BitField {}; void PrintBaseName(std::ostream& os) const override; // NOLINT diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 1d9d3ac..4482155 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -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 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); } diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index a59cd2f..be03186 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -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 allocation_site = Handle::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 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_; diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 949562f..b24890a 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -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(left, right); - return AddUncasted(Runtime::FunctionForId(Runtime::kAdd), - 2); + return AddUncasted( + 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(left, right); - return AddUncasted(Runtime::FunctionForId(Runtime::kAdd), - 2); + return AddUncasted( + 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( - 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( - 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(left, right, strength(function_language_mode())); + HInstruction* result = NewUncasted(left, right); return ast_context()->ReturnInstruction(result, call->id()); } -- 2.7.4