From 5e8539957218264ebf2c2acd706d3d6f413e5f6b Mon Sep 17 00:00:00 2001 From: "bmeurer@chromium.org" Date: Fri, 19 Jul 2013 11:52:42 +0000 Subject: [PATCH] Cleanup StringAddFlags. Avoid duplication of StringAddFlags in the platform specific code stubs header files. Fix the inverted flag logic, replacing it with a scheme that is easier to understand. Depends on: https://codereview.chromium.org/19541003 R=mvstanton@chromium.org Review URL: https://codereview.chromium.org/19492006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15775 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 52 +++++++++++++++++------------------ src/arm/code-stubs-arm.h | 15 ---------- src/arm/full-codegen-arm.cc | 2 +- src/code-stubs.h | 16 +++++++++++ src/hydrogen-instructions.h | 2 +- src/hydrogen.cc | 2 +- src/ia32/code-stubs-ia32.cc | 46 +++++++++++++++---------------- src/ia32/code-stubs-ia32.h | 14 ---------- src/ia32/full-codegen-ia32.cc | 2 +- src/mips/code-stubs-mips.cc | 52 +++++++++++++++++------------------ src/mips/code-stubs-mips.h | 15 ---------- src/mips/full-codegen-mips.cc | 2 +- src/x64/code-stubs-x64.cc | 48 ++++++++++++++++---------------- src/x64/code-stubs-x64.h | 15 ---------- src/x64/full-codegen-x64.cc | 2 +- 15 files changed, 121 insertions(+), 164 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 1e1a2fc51e..7773667b7e 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -1897,8 +1897,8 @@ void BinaryOpStub::GenerateBothStringStub(MacroAssembler* masm) { __ CompareObjectType(right, r2, r2, FIRST_NONSTRING_TYPE); __ b(ge, &call_runtime); - StringAddStub string_add_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_IN_STUB)); + StringAddStub string_add_stub( + (StringAddFlags)(STRING_ADD_CHECK_NONE | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_stub); @@ -2256,8 +2256,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ CompareObjectType(left, r2, r2, FIRST_NONSTRING_TYPE); __ b(ge, &left_not_string); - StringAddStub string_add_left_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_LEFT_IN_STUB)); + StringAddStub string_add_left_stub( + (StringAddFlags)(STRING_ADD_CHECK_RIGHT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_left_stub); @@ -2267,8 +2267,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ CompareObjectType(right, r2, r2, FIRST_NONSTRING_TYPE); __ b(ge, &call_runtime); - StringAddStub string_add_right_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_RIGHT_IN_STUB)); + StringAddStub string_add_right_stub( + (StringAddFlags)(STRING_ADD_CHECK_LEFT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_right_stub); @@ -5494,7 +5494,11 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ ldr(r1, MemOperand(sp, 0 * kPointerSize)); // Second argument. // Make sure that both arguments are strings if not known in advance. - if ((flags_ & NO_STRING_ADD_FLAGS) != 0) { + // Otherwise, at least one of the arguments is definitely a string, + // and we convert the one that is not known to be a string. + if ((flags_ & STRING_ADD_CHECK_BOTH) == STRING_ADD_CHECK_BOTH) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT); + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT); __ JumpIfEitherSmi(r0, r1, &call_runtime); // Load instance types. __ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset)); @@ -5506,20 +5510,16 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ tst(r4, Operand(kIsNotStringMask)); __ tst(r5, Operand(kIsNotStringMask), eq); __ b(ne, &call_runtime); - } else { - // Here at least one of the arguments is definitely a string. - // We convert the one that is not known to be a string. - if ((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) != 0); - GenerateConvertArgument( - masm, 1 * kPointerSize, r0, r2, r3, r4, r5, &call_builtin); - builtin_id = Builtins::STRING_ADD_RIGHT; - } else if ((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) != 0); - GenerateConvertArgument( - masm, 0 * kPointerSize, r1, r2, r3, r4, r5, &call_builtin); - builtin_id = Builtins::STRING_ADD_LEFT; - } + } else if ((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) { + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == 0); + GenerateConvertArgument( + masm, 1 * kPointerSize, r0, r2, r3, r4, r5, &call_builtin); + builtin_id = Builtins::STRING_ADD_RIGHT; + } else if ((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == 0); + GenerateConvertArgument( + masm, 0 * kPointerSize, r1, r2, r3, r4, r5, &call_builtin); + builtin_id = Builtins::STRING_ADD_LEFT; } // Both arguments are strings. @@ -5567,7 +5567,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ b(ne, &longer_than_two); // Check that both strings are non-external ASCII strings. - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset)); __ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset)); __ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset)); @@ -5615,7 +5615,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // If result is not supposed to be flat, allocate a cons string object. // If both strings are ASCII the result is an ASCII cons string. - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset)); __ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset)); __ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset)); @@ -5698,7 +5698,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // r6: sum of lengths. Label first_prepared, second_prepared; __ bind(&string_add_flat_result); - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset)); __ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset)); __ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset)); @@ -5786,7 +5786,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // Just jump to runtime to add the two strings. __ bind(&call_runtime); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm); // Build a frame { @@ -5801,7 +5801,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { if (call_builtin.is_linked()) { __ bind(&call_builtin); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm); // Build a frame { diff --git a/src/arm/code-stubs-arm.h b/src/arm/code-stubs-arm.h index ae9f3c4d23..6eab8d128e 100644 --- a/src/arm/code-stubs-arm.h +++ b/src/arm/code-stubs-arm.h @@ -144,21 +144,6 @@ class StringHelper : public AllStatic { }; -// Flag that indicates how to generate code for the stub StringAddStub. -enum StringAddFlags { - NO_STRING_ADD_FLAGS = 1 << 0, - // Omit left string check in stub (left is definitely a string). - NO_STRING_CHECK_LEFT_IN_STUB = 1 << 1, - // Omit right string check in stub (right is definitely a string). - NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 2, - // Stub needs a frame before calling the runtime - ERECT_FRAME = 1 << 3, - // Omit both string checks in stub. - NO_STRING_CHECK_IN_STUB = - NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB -}; - - class StringAddStub: public PlatformCodeStub { public: explicit StringAddStub(StringAddFlags flags) : flags_(flags) {} diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 9040a75a31..1760fb8cf1 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -3719,7 +3719,7 @@ void FullCodeGenerator::EmitStringAdd(CallRuntime* expr) { VisitForStackValue(args->at(0)); VisitForStackValue(args->at(1)); - StringAddStub stub(NO_STRING_ADD_FLAGS); + StringAddStub stub(STRING_ADD_CHECK_BOTH); __ CallStub(&stub); context()->Plug(r0); } diff --git a/src/code-stubs.h b/src/code-stubs.h index 131887d4e4..33593544d6 100644 --- a/src/code-stubs.h +++ b/src/code-stubs.h @@ -393,6 +393,22 @@ class RuntimeCallHelper { DISALLOW_COPY_AND_ASSIGN(RuntimeCallHelper); }; + +// TODO(bmeurer): Move to the StringAddStub declaration once we're +// done with the translation to a hydrogen code stub. +enum StringAddFlags { + // Omit both parameter checks. + STRING_ADD_CHECK_NONE = 0, + // Check left parameter. + STRING_ADD_CHECK_LEFT = 1 << 0, + // Check right parameter. + STRING_ADD_CHECK_RIGHT = 1 << 1, + // Check both parameters. + STRING_ADD_CHECK_BOTH = STRING_ADD_CHECK_LEFT | STRING_ADD_CHECK_RIGHT, + // Stub needs a frame before calling the runtime + STRING_ADD_ERECT_FRAME = 1 << 2 +}; + } } // namespace v8::internal #if V8_TARGET_ARCH_IA32 diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 6ab02dc7ad..2c971ab9ac 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -6182,7 +6182,7 @@ class HStringAdd: public HBinaryOperation { HValue* context, HValue* left, HValue* right, - StringAddFlags flags = NO_STRING_CHECK_IN_STUB); + StringAddFlags flags = STRING_ADD_CHECK_NONE); StringAddFlags flags() const { return flags_; } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 2802773b0f..ddd47c10b6 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -9083,7 +9083,7 @@ void HOptimizedGraphBuilder::GenerateStringAdd(CallRuntime* call) { HValue* left = Pop(); HValue* context = environment()->LookupContext(); HInstruction* result = HStringAdd::New( - zone(), context, left, right, NO_STRING_ADD_FLAGS); + zone(), context, left, right, STRING_ADD_CHECK_BOTH); return ast_context()->ReturnInstruction(result, call->id()); } diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index 490091e9da..548cbaace7 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -1353,8 +1353,8 @@ void BinaryOpStub::GenerateBothStringStub(MacroAssembler* masm) { __ CmpObjectType(right, FIRST_NONSTRING_TYPE, ecx); __ j(above_equal, &call_runtime, Label::kNear); - StringAddStub string_add_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_IN_STUB)); + StringAddStub string_add_stub( + (StringAddFlags)(STRING_ADD_CHECK_NONE | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_stub); @@ -1999,8 +1999,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ CmpObjectType(left, FIRST_NONSTRING_TYPE, ecx); __ j(above_equal, &left_not_string, Label::kNear); - StringAddStub string_add_left_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_LEFT_IN_STUB)); + StringAddStub string_add_left_stub( + (StringAddFlags)(STRING_ADD_CHECK_RIGHT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_left_stub); @@ -2010,8 +2010,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ CmpObjectType(right, FIRST_NONSTRING_TYPE, ecx); __ j(above_equal, &call_runtime, Label::kNear); - StringAddStub string_add_right_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_RIGHT_IN_STUB)); + StringAddStub string_add_right_stub( + (StringAddFlags)(STRING_ADD_CHECK_LEFT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_right_stub); @@ -5376,7 +5376,11 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ mov(edx, Operand(esp, 1 * kPointerSize)); // Second argument. // Make sure that both arguments are strings if not known in advance. - if ((flags_ & NO_STRING_ADD_FLAGS) != 0) { + // Otherwise, at least one of the arguments is definitely a string, + // and we convert the one that is not known to be a string. + if ((flags_ & STRING_ADD_CHECK_BOTH) == STRING_ADD_CHECK_BOTH) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT); + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT); __ JumpIfSmi(eax, &call_runtime); __ CmpObjectType(eax, FIRST_NONSTRING_TYPE, ebx); __ j(above_equal, &call_runtime); @@ -5385,20 +5389,16 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ JumpIfSmi(edx, &call_runtime); __ CmpObjectType(edx, FIRST_NONSTRING_TYPE, ebx); __ j(above_equal, &call_runtime); - } else { - // Here at least one of the arguments is definitely a string. - // We convert the one that is not known to be a string. - if ((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) != 0); - GenerateConvertArgument(masm, 2 * kPointerSize, eax, ebx, ecx, edi, - &call_builtin); - builtin_id = Builtins::STRING_ADD_RIGHT; - } else if ((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) != 0); - GenerateConvertArgument(masm, 1 * kPointerSize, edx, ebx, ecx, edi, - &call_builtin); - builtin_id = Builtins::STRING_ADD_LEFT; - } + } else if ((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) { + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == 0); + GenerateConvertArgument(masm, 2 * kPointerSize, eax, ebx, ecx, edi, + &call_builtin); + builtin_id = Builtins::STRING_ADD_RIGHT; + } else if ((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == 0); + GenerateConvertArgument(masm, 1 * kPointerSize, edx, ebx, ecx, edi, + &call_builtin); + builtin_id = Builtins::STRING_ADD_LEFT; } // Both arguments are strings. @@ -5684,7 +5684,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ Drop(2); // Just jump to runtime to add the two strings. __ bind(&call_runtime); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm, ecx); // Build a frame { @@ -5699,7 +5699,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { if (call_builtin.is_linked()) { __ bind(&call_builtin); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm, ecx); // Build a frame { diff --git a/src/ia32/code-stubs-ia32.h b/src/ia32/code-stubs-ia32.h index 62f866489c..e80acc6ccf 100644 --- a/src/ia32/code-stubs-ia32.h +++ b/src/ia32/code-stubs-ia32.h @@ -144,20 +144,6 @@ class StringHelper : public AllStatic { }; -enum StringAddFlags { - NO_STRING_ADD_FLAGS = 1 << 0, - // Omit left string check in stub (left is definitely a string). - NO_STRING_CHECK_LEFT_IN_STUB = 1 << 1, - // Omit right string check in stub (right is definitely a string). - NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 2, - // Stub needs a frame before calling the runtime - ERECT_FRAME = 1 << 3, - // Omit both string checks in stub. - NO_STRING_CHECK_IN_STUB = - NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB -}; - - class StringAddStub: public PlatformCodeStub { public: explicit StringAddStub(StringAddFlags flags) : flags_(flags) {} diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 5b14afbf03..66a7c1c080 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -3685,7 +3685,7 @@ void FullCodeGenerator::EmitStringAdd(CallRuntime* expr) { VisitForStackValue(args->at(0)); VisitForStackValue(args->at(1)); - StringAddStub stub(NO_STRING_ADD_FLAGS); + StringAddStub stub(STRING_ADD_CHECK_BOTH); __ CallStub(&stub); context()->Plug(eax); } diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index f1c6c39b05..83ea24770c 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -2140,8 +2140,8 @@ void BinaryOpStub::GenerateBothStringStub(MacroAssembler* masm) { __ GetObjectType(right, a2, a2); __ Branch(&call_runtime, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_IN_STUB)); + StringAddStub string_add_stub( + (StringAddFlags)(STRING_ADD_CHECK_NONE | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_stub); @@ -2558,8 +2558,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ GetObjectType(left, a2, a2); __ Branch(&left_not_string, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_left_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_LEFT_IN_STUB)); + StringAddStub string_add_left_stub( + (StringAddFlags)(STRING_ADD_CHECK_RIGHT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_left_stub); @@ -2569,8 +2569,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ GetObjectType(right, a2, a2); __ Branch(&call_runtime, ge, a2, Operand(FIRST_NONSTRING_TYPE)); - StringAddStub string_add_right_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_RIGHT_IN_STUB)); + StringAddStub string_add_right_stub( + (StringAddFlags)(STRING_ADD_CHECK_LEFT | STRING_ADD_ERECT_FRAME)); GenerateRegisterArgsPush(masm); __ TailCallStub(&string_add_right_stub); @@ -5872,7 +5872,11 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ lw(a1, MemOperand(sp, 0 * kPointerSize)); // Second argument. // Make sure that both arguments are strings if not known in advance. - if ((flags_ & NO_STRING_ADD_FLAGS) != 0) { + // Otherwise, at least one of the arguments is definitely a string, + // and we convert the one that is not known to be a string. + if ((flags_ & STRING_ADD_CHECK_BOTH) == STRING_ADD_CHECK_BOTH) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT); + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT); __ JumpIfEitherSmi(a0, a1, &call_runtime); // Load instance types. __ lw(t0, FieldMemOperand(a0, HeapObject::kMapOffset)); @@ -5884,20 +5888,16 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ Or(t4, t0, Operand(t1)); __ And(t4, t4, Operand(kIsNotStringMask)); __ Branch(&call_runtime, ne, t4, Operand(zero_reg)); - } else { - // Here at least one of the arguments is definitely a string. - // We convert the one that is not known to be a string. - if ((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) != 0); - GenerateConvertArgument( - masm, 1 * kPointerSize, a0, a2, a3, t0, t1, &call_builtin); - builtin_id = Builtins::STRING_ADD_RIGHT; - } else if ((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) != 0); - GenerateConvertArgument( - masm, 0 * kPointerSize, a1, a2, a3, t0, t1, &call_builtin); - builtin_id = Builtins::STRING_ADD_LEFT; - } + } else if ((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) { + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == 0); + GenerateConvertArgument( + masm, 1 * kPointerSize, a0, a2, a3, t0, t1, &call_builtin); + builtin_id = Builtins::STRING_ADD_RIGHT; + } else if ((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == 0); + GenerateConvertArgument( + masm, 0 * kPointerSize, a1, a2, a3, t0, t1, &call_builtin); + builtin_id = Builtins::STRING_ADD_LEFT; } // Both arguments are strings. @@ -5948,7 +5948,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ Branch(&longer_than_two, ne, t2, Operand(2)); // Check that both strings are non-external ASCII strings. - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ lw(t0, FieldMemOperand(a0, HeapObject::kMapOffset)); __ lw(t1, FieldMemOperand(a1, HeapObject::kMapOffset)); __ lbu(t0, FieldMemOperand(t0, Map::kInstanceTypeOffset)); @@ -5992,7 +5992,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // If result is not supposed to be flat, allocate a cons string object. // If both strings are ASCII the result is an ASCII cons string. - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ lw(t0, FieldMemOperand(a0, HeapObject::kMapOffset)); __ lw(t1, FieldMemOperand(a1, HeapObject::kMapOffset)); __ lbu(t0, FieldMemOperand(t0, Map::kInstanceTypeOffset)); @@ -6075,7 +6075,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // t2: sum of lengths. Label first_prepared, second_prepared; __ bind(&string_add_flat_result); - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ lw(t0, FieldMemOperand(a0, HeapObject::kMapOffset)); __ lw(t1, FieldMemOperand(a1, HeapObject::kMapOffset)); __ lbu(t0, FieldMemOperand(t0, Map::kInstanceTypeOffset)); @@ -6161,7 +6161,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // Just jump to runtime to add the two strings. __ bind(&call_runtime); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm); // Build a frame. { @@ -6176,7 +6176,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { if (call_builtin.is_linked()) { __ bind(&call_builtin); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm); // Build a frame. { diff --git a/src/mips/code-stubs-mips.h b/src/mips/code-stubs-mips.h index 77a02be219..1ae1d3454f 100644 --- a/src/mips/code-stubs-mips.h +++ b/src/mips/code-stubs-mips.h @@ -145,21 +145,6 @@ class StringHelper : public AllStatic { }; -// Flag that indicates how to generate code for the stub StringAddStub. -enum StringAddFlags { - NO_STRING_ADD_FLAGS = 1 << 0, - // Omit left string check in stub (left is definitely a string). - NO_STRING_CHECK_LEFT_IN_STUB = 1 << 1, - // Omit right string check in stub (right is definitely a string). - NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 2, - // Stub needs a frame before calling the runtime - ERECT_FRAME = 1 << 3, - // Omit both string checks in stub. - NO_STRING_CHECK_IN_STUB = - NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB -}; - - class StringAddStub: public PlatformCodeStub { public: explicit StringAddStub(StringAddFlags flags) : flags_(flags) {} diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 6a59b1b1c9..51b71a671c 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -3746,7 +3746,7 @@ void FullCodeGenerator::EmitStringAdd(CallRuntime* expr) { VisitForStackValue(args->at(0)); VisitForStackValue(args->at(1)); - StringAddStub stub(NO_STRING_ADD_FLAGS); + StringAddStub stub(STRING_ADD_CHECK_BOTH); __ CallStub(&stub); context()->Plug(v0); } diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index 6ed6af6698..652a4db7a6 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -1014,8 +1014,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ JumpIfSmi(left, &left_not_string, Label::kNear); __ CmpObjectType(left, FIRST_NONSTRING_TYPE, rcx); __ j(above_equal, &left_not_string, Label::kNear); - StringAddStub string_add_left_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_LEFT_IN_STUB)); + StringAddStub string_add_left_stub( + (StringAddFlags)(STRING_ADD_CHECK_RIGHT | STRING_ADD_ERECT_FRAME)); BinaryOpStub_GenerateRegisterArgsPushUnderReturn(masm); __ TailCallStub(&string_add_left_stub); @@ -1025,8 +1025,8 @@ void BinaryOpStub::GenerateAddStrings(MacroAssembler* masm) { __ CmpObjectType(right, FIRST_NONSTRING_TYPE, rcx); __ j(above_equal, &call_runtime, Label::kNear); - StringAddStub string_add_right_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_RIGHT_IN_STUB)); + StringAddStub string_add_right_stub( + (StringAddFlags)(STRING_ADD_CHECK_LEFT | STRING_ADD_ERECT_FRAME)); BinaryOpStub_GenerateRegisterArgsPushUnderReturn(masm); __ TailCallStub(&string_add_right_stub); @@ -1101,8 +1101,8 @@ void BinaryOpStub::GenerateBothStringStub(MacroAssembler* masm) { __ CmpObjectType(right, FIRST_NONSTRING_TYPE, rcx); __ j(above_equal, &call_runtime); - StringAddStub string_add_stub((StringAddFlags) - (ERECT_FRAME | NO_STRING_CHECK_IN_STUB)); + StringAddStub string_add_stub( + (StringAddFlags)(STRING_ADD_CHECK_NONE | STRING_ADD_ERECT_FRAME)); BinaryOpStub_GenerateRegisterArgsPushUnderReturn(masm); __ TailCallStub(&string_add_stub); @@ -4516,7 +4516,11 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ movq(rdx, Operand(rsp, 1 * kPointerSize)); // Second argument (right). // Make sure that both arguments are strings if not known in advance. - if ((flags_ & NO_STRING_ADD_FLAGS) != 0) { + // Otherwise, at least one of the arguments is definitely a string, + // and we convert the one that is not known to be a string. + if ((flags_ & STRING_ADD_CHECK_BOTH) == STRING_ADD_CHECK_BOTH) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT); + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT); __ JumpIfSmi(rax, &call_runtime); __ CmpObjectType(rax, FIRST_NONSTRING_TYPE, r8); __ j(above_equal, &call_runtime); @@ -4525,20 +4529,16 @@ void StringAddStub::Generate(MacroAssembler* masm) { __ JumpIfSmi(rdx, &call_runtime); __ CmpObjectType(rdx, FIRST_NONSTRING_TYPE, r9); __ j(above_equal, &call_runtime); - } else { - // Here at least one of the arguments is definitely a string. - // We convert the one that is not known to be a string. - if ((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) != 0); - GenerateConvertArgument(masm, 2 * kPointerSize, rax, rbx, rcx, rdi, - &call_builtin); - builtin_id = Builtins::STRING_ADD_RIGHT; - } else if ((flags_ & NO_STRING_CHECK_RIGHT_IN_STUB) == 0) { - ASSERT((flags_ & NO_STRING_CHECK_LEFT_IN_STUB) != 0); - GenerateConvertArgument(masm, 1 * kPointerSize, rdx, rbx, rcx, rdi, - &call_builtin); - builtin_id = Builtins::STRING_ADD_LEFT; - } + } else if ((flags_ & STRING_ADD_CHECK_LEFT) == STRING_ADD_CHECK_LEFT) { + ASSERT((flags_ & STRING_ADD_CHECK_RIGHT) == 0); + GenerateConvertArgument(masm, 2 * kPointerSize, rax, rbx, rcx, rdi, + &call_builtin); + builtin_id = Builtins::STRING_ADD_RIGHT; + } else if ((flags_ & STRING_ADD_CHECK_RIGHT) == STRING_ADD_CHECK_RIGHT) { + ASSERT((flags_ & STRING_ADD_CHECK_LEFT) == 0); + GenerateConvertArgument(masm, 1 * kPointerSize, rdx, rbx, rcx, rdi, + &call_builtin); + builtin_id = Builtins::STRING_ADD_LEFT; } // Both arguments are strings. @@ -4574,7 +4574,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // If arguments where known to be strings, maps are not loaded to r8 and r9 // by the code above. - if (flags_ != NO_STRING_ADD_FLAGS) { + if ((flags_ & STRING_ADD_CHECK_BOTH) != STRING_ADD_CHECK_BOTH) { __ movq(r8, FieldOperand(rax, HeapObject::kMapOffset)); __ movq(r9, FieldOperand(rdx, HeapObject::kMapOffset)); } @@ -4794,7 +4794,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { // Just jump to runtime to add the two strings. __ bind(&call_runtime); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm, rcx); // Build a frame { @@ -4809,7 +4809,7 @@ void StringAddStub::Generate(MacroAssembler* masm) { if (call_builtin.is_linked()) { __ bind(&call_builtin); - if ((flags_ & ERECT_FRAME) != 0) { + if ((flags_ & STRING_ADD_ERECT_FRAME) != 0) { GenerateRegisterArgsPop(masm, rcx); // Build a frame { diff --git a/src/x64/code-stubs-x64.h b/src/x64/code-stubs-x64.h index af72f59cbf..e430bf2c80 100644 --- a/src/x64/code-stubs-x64.h +++ b/src/x64/code-stubs-x64.h @@ -134,21 +134,6 @@ class StringHelper : public AllStatic { }; -// Flag that indicates how to generate code for the stub StringAddStub. -enum StringAddFlags { - NO_STRING_ADD_FLAGS = 1 << 0, - // Omit left string check in stub (left is definitely a string). - NO_STRING_CHECK_LEFT_IN_STUB = 1 << 1, - // Omit right string check in stub (right is definitely a string). - NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 2, - // Stub needs a frame before calling the runtime - ERECT_FRAME = 1 << 3, - // Omit both string checks in stub. - NO_STRING_CHECK_IN_STUB = - NO_STRING_CHECK_LEFT_IN_STUB | NO_STRING_CHECK_RIGHT_IN_STUB -}; - - class StringAddStub: public PlatformCodeStub { public: explicit StringAddStub(StringAddFlags flags) : flags_(flags) {} diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 66bc38b260..121ab2d1a0 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -3645,7 +3645,7 @@ void FullCodeGenerator::EmitStringAdd(CallRuntime* expr) { VisitForStackValue(args->at(0)); VisitForStackValue(args->at(1)); - StringAddStub stub(NO_STRING_ADD_FLAGS); + StringAddStub stub(STRING_ADD_CHECK_BOTH); __ CallStub(&stub); context()->Plug(rax); }