Commit c5a4c398 authored by machenbach's avatar machenbach Committed by Commit bot

Revert of [crankshaft] Re-add fast-case for string add left/right. (patchset...

Revert of [crankshaft] Re-add fast-case for string add left/right. (patchset #1 id:1 of https://codereview.chromium.org/1339053002/ )

Reason for revert:
[Sheriff] Fails mozilla with deadcode:
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20deadcode/builds/5357

Original issue's description:
> [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.
>
> CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_win_nosnap_shared_rel,v8_linux_nosnap_dbg
> R=mstarzinger@chromium.org
> BUG=v8:4307
> LOG=n
>
> Committed: https://crrev.com/d261849e53fbf8c36efae42d478271f87acff70f
> Cr-Commit-Position: refs/heads/master@{#30726}

TBR=mstarzinger@chromium.org,jarin@chromium.org,bmeurer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4307

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

Cr-Commit-Position: refs/heads/master@{#30727}
parent d261849e
...@@ -113,8 +113,6 @@ class CodeStubGraphBuilderBase : public HGraphBuilder { ...@@ -113,8 +113,6 @@ class CodeStubGraphBuilderBase : public HGraphBuilder {
HValue* shared_info, HValue* shared_info,
HValue* native_context); HValue* native_context);
HValue* CheckString(HValue* input, bool convert);
private: private:
HValue* BuildArraySingleArgumentConstructor(JSArrayBuilder* builder); HValue* BuildArraySingleArgumentConstructor(JSArrayBuilder* builder);
HValue* BuildArrayNArgumentsConstructor(JSArrayBuilder* builder, HValue* BuildArrayNArgumentsConstructor(JSArrayBuilder* builder,
...@@ -1458,66 +1456,6 @@ Handle<Code> BinaryOpWithAllocationSiteStub::GenerateCode() { ...@@ -1458,66 +1456,6 @@ 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 <> template <>
HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() { HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() {
StringAddStub* stub = casted_stub(); StringAddStub* stub = casted_stub();
...@@ -1529,12 +1467,10 @@ HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() { ...@@ -1529,12 +1467,10 @@ HValue* CodeStubGraphBuilder<StringAddStub>::BuildCodeInitializedStub() {
// Make sure that both arguments are strings if not known in advance. // Make sure that both arguments are strings if not known in advance.
if ((flags & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) { if ((flags & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) {
left = left = BuildCheckString(left);
CheckString(left, (flags & STRING_ADD_CONVERT) == STRING_ADD_CONVERT);
} }
if ((flags & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) { if ((flags & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) {
right = right = BuildCheckString(right);
CheckString(right, (flags & STRING_ADD_CONVERT) == STRING_ADD_CONVERT);
} }
return BuildStringAdd(left, right, HAllocationMode(pretenure_flag)); return BuildStringAdd(left, right, HAllocationMode(pretenure_flag));
......
...@@ -324,12 +324,6 @@ std::ostream& operator<<(std::ostream& os, const StringAddFlags& flags) { ...@@ -324,12 +324,6 @@ std::ostream& operator<<(std::ostream& os, const StringAddFlags& flags) {
return os << "CheckRight"; return os << "CheckRight";
case STRING_ADD_CHECK_BOTH: case STRING_ADD_CHECK_BOTH:
return os << "CheckBoth"; 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(); UNREACHABLE();
return os; return os;
......
...@@ -676,11 +676,7 @@ enum StringAddFlags { ...@@ -676,11 +676,7 @@ enum StringAddFlags {
// Check right parameter. // Check right parameter.
STRING_ADD_CHECK_RIGHT = 1 << 1, STRING_ADD_CHECK_RIGHT = 1 << 1,
// Check both parameters. // 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
}; };
...@@ -705,8 +701,8 @@ class StringAddTFStub : public TurboFanCodeStub { ...@@ -705,8 +701,8 @@ class StringAddTFStub : public TurboFanCodeStub {
} }
private: private:
class StringAddFlagsBits : public BitField<StringAddFlags, 0, 3> {}; class StringAddFlagsBits : public BitField<StringAddFlags, 0, 2> {};
class PretenureFlagBits : public BitField<PretenureFlag, 3, 1> {}; class PretenureFlagBits : public BitField<PretenureFlag, 2, 1> {};
void PrintBaseName(std::ostream& os) const override; // NOLINT void PrintBaseName(std::ostream& os) const override; // NOLINT
...@@ -1651,8 +1647,8 @@ class StringAddStub final : public HydrogenCodeStub { ...@@ -1651,8 +1647,8 @@ class StringAddStub final : public HydrogenCodeStub {
static const int kRight = 1; static const int kRight = 1;
private: private:
class StringAddFlagsBits : public BitField<StringAddFlags, 0, 3> {}; class StringAddFlagsBits: public BitField<StringAddFlags, 0, 2> {};
class PretenureFlagBits : public BitField<PretenureFlag, 3, 1> {}; class PretenureFlagBits: public BitField<PretenureFlag, 2, 1> {};
void PrintBaseName(std::ostream& os) const override; // NOLINT void PrintBaseName(std::ostream& os) const override; // NOLINT
......
...@@ -3997,7 +3997,7 @@ DEFINE_NEW_H_SIMPLE_ARITHMETIC_INSTR(HSub, -) ...@@ -3997,7 +3997,7 @@ DEFINE_NEW_H_SIMPLE_ARITHMETIC_INSTR(HSub, -)
HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context, HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context,
HValue* left, HValue* right, HValue* left, HValue* right, Strength strength,
PretenureFlag pretenure_flag, PretenureFlag pretenure_flag,
StringAddFlags flags, StringAddFlags flags,
Handle<AllocationSite> allocation_site) { Handle<AllocationSite> allocation_site) {
...@@ -4015,8 +4015,8 @@ HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context, ...@@ -4015,8 +4015,8 @@ HInstruction* HStringAdd::New(Isolate* isolate, Zone* zone, HValue* context,
} }
} }
} }
return new (zone) return new (zone) HStringAdd(context, left, right, strength, pretenure_flag,
HStringAdd(context, left, right, pretenure_flag, flags, allocation_site); flags, allocation_site);
} }
......
...@@ -7355,7 +7355,8 @@ class HStringAdd final : public HBinaryOperation { ...@@ -7355,7 +7355,8 @@ class HStringAdd final : public HBinaryOperation {
public: public:
static HInstruction* New( static HInstruction* New(
Isolate* isolate, Zone* zone, HValue* context, HValue* left, Isolate* isolate, Zone* zone, HValue* context, HValue* left,
HValue* right, PretenureFlag pretenure_flag = NOT_TENURED, HValue* right, Strength strength = Strength::WEAK,
PretenureFlag pretenure_flag = NOT_TENURED,
StringAddFlags flags = STRING_ADD_CHECK_BOTH, StringAddFlags flags = STRING_ADD_CHECK_BOTH,
Handle<AllocationSite> allocation_site = Handle<AllocationSite>::null()); Handle<AllocationSite> allocation_site = Handle<AllocationSite>::null());
...@@ -7377,10 +7378,10 @@ class HStringAdd final : public HBinaryOperation { ...@@ -7377,10 +7378,10 @@ class HStringAdd final : public HBinaryOperation {
} }
private: private:
HStringAdd(HValue* context, HValue* left, HValue* right, HStringAdd(HValue* context, HValue* left, HValue* right, Strength strength,
PretenureFlag pretenure_flag, StringAddFlags flags, PretenureFlag pretenure_flag, StringAddFlags flags,
Handle<AllocationSite> allocation_site) Handle<AllocationSite> allocation_site)
: HBinaryOperation(context, left, right, Strength::WEAK, HType::String()), : HBinaryOperation(context, left, right, strength, HType::String()),
flags_(flags), flags_(flags),
pretenure_flag_(pretenure_flag) { pretenure_flag_(pretenure_flag) {
set_representation(Representation::Tagged()); set_representation(Representation::Tagged());
......
...@@ -10995,9 +10995,11 @@ HValue* HGraphBuilder::BuildBinaryOperation( ...@@ -10995,9 +10995,11 @@ HValue* HGraphBuilder::BuildBinaryOperation(
left = BuildNumberToString(left, left_type); left = BuildNumberToString(left, left_type);
} else if (!left_type->Is(Type::String())) { } else if (!left_type->Is(Type::String())) {
DCHECK(right_type->Is(Type::String())); DCHECK(right_type->Is(Type::String()));
return AddUncasted<HStringAdd>( // TODO(bmeurer): We might want to optimize this, because we already
left, right, allocation_mode.GetPretenureMode(), // know that the right hand side is a string.
STRING_ADD_CONVERT_LEFT, allocation_mode.feedback_site()); Add<HPushArguments>(left, right);
return AddUncasted<HCallRuntime>(Runtime::FunctionForId(Runtime::kAdd),
2);
} }
// Convert right argument as necessary. // Convert right argument as necessary.
...@@ -11006,9 +11008,11 @@ HValue* HGraphBuilder::BuildBinaryOperation( ...@@ -11006,9 +11008,11 @@ HValue* HGraphBuilder::BuildBinaryOperation(
right = BuildNumberToString(right, right_type); right = BuildNumberToString(right, right_type);
} else if (!right_type->Is(Type::String())) { } else if (!right_type->Is(Type::String())) {
DCHECK(left_type->Is(Type::String())); DCHECK(left_type->Is(Type::String()));
return AddUncasted<HStringAdd>( // TODO(bmeurer): We might want to optimize this, because we already
left, right, allocation_mode.GetPretenureMode(), // know that the left hand side is a string.
STRING_ADD_CONVERT_RIGHT, allocation_mode.feedback_site()); Add<HPushArguments>(left, right);
return AddUncasted<HCallRuntime>(Runtime::FunctionForId(Runtime::kAdd),
2);
} }
} }
...@@ -11025,7 +11029,7 @@ HValue* HGraphBuilder::BuildBinaryOperation( ...@@ -11025,7 +11029,7 @@ HValue* HGraphBuilder::BuildBinaryOperation(
if (!right_string.is_null() && right_string->length() == 0) return left; if (!right_string.is_null() && right_string->length() == 0) return left;
if (!left_string.is_null() && !right_string.is_null()) { if (!left_string.is_null() && !right_string.is_null()) {
return AddUncasted<HStringAdd>( return AddUncasted<HStringAdd>(
left, right, allocation_mode.GetPretenureMode(), left, right, strength, allocation_mode.GetPretenureMode(),
STRING_ADD_CHECK_NONE, allocation_mode.feedback_site()); STRING_ADD_CHECK_NONE, allocation_mode.feedback_site());
} }
...@@ -11054,8 +11058,8 @@ HValue* HGraphBuilder::BuildBinaryOperation( ...@@ -11054,8 +11058,8 @@ HValue* HGraphBuilder::BuildBinaryOperation(
// Fallback to using the string add stub. // Fallback to using the string add stub.
return AddUncasted<HStringAdd>( return AddUncasted<HStringAdd>(
left, right, allocation_mode.GetPretenureMode(), STRING_ADD_CHECK_NONE, left, right, strength, allocation_mode.GetPretenureMode(),
allocation_mode.feedback_site()); STRING_ADD_CHECK_NONE, allocation_mode.feedback_site());
} }
if (graph()->info()->IsStub()) { if (graph()->info()->IsStub()) {
...@@ -12468,7 +12472,8 @@ void HOptimizedGraphBuilder::GenerateStringAdd(CallRuntime* call) { ...@@ -12468,7 +12472,8 @@ void HOptimizedGraphBuilder::GenerateStringAdd(CallRuntime* call) {
CHECK_ALIVE(VisitForValue(call->arguments()->at(1))); CHECK_ALIVE(VisitForValue(call->arguments()->at(1)));
HValue* right = Pop(); HValue* right = Pop();
HValue* left = Pop(); HValue* left = Pop();
HInstruction* result = NewUncasted<HStringAdd>(left, right); HInstruction* result =
NewUncasted<HStringAdd>(left, right, strength(function_language_mode()));
return ast_context()->ReturnInstruction(result, call->id()); return ast_context()->ReturnInstruction(result, call->id());
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment