Revision: 6882
Author: [email protected]
Date: Tue Feb 22 01:35:37 2011
Log: ARM: Add support for just one string argument in the string add stub
Having a string add stub which can handle a string in just one of the
arguments is essencial for the type recording binary operation stub when
expecting strings. Otherwise string added with e.g. smi will always call
the runtime for a type transition which will be back to the same types as
the transition code keeps it in string arguments when one argument is a
string.
This fixes the regression (especially Delta Blue) caused by replacing the
generic binary operation stub with the type recording binary operation stub
in the ARM lithium code generator.
Review URL: http://codereview.chromium.org/6551008
http://code.google.com/p/v8/source/detail?r=6882
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/arm/code-stubs-arm.h
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Mon Feb 21 03:29:45
2011
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Tue Feb 22 01:35:37
2011
@@ -2961,22 +2961,26 @@
Register left = r1;
Register right = r0;
- Label call_runtime;
-
- // Check if first argument is a string.
- __ JumpIfSmi(left, &call_runtime);
+ Label left_not_string, call_runtime;
+
+ // Check if left argument is a string.
+ __ JumpIfSmi(left, &left_not_string);
__ CompareObjectType(left, r2, r2, FIRST_NONSTRING_TYPE);
- __ b(ge, &call_runtime);
-
- // First argument is a a string, test second.
+ __ b(ge, &left_not_string);
+
+ StringAddStub string_add_left_stub(NO_STRING_CHECK_LEFT_IN_STUB);
+ GenerateRegisterArgsPush(masm);
+ __ TailCallStub(&string_add_left_stub);
+
+ // Left operand is not a string, test right.
+ __ bind(&left_not_string);
__ JumpIfSmi(right, &call_runtime);
__ CompareObjectType(right, r2, r2, FIRST_NONSTRING_TYPE);
__ b(ge, &call_runtime);
- // First and second argument are strings.
- StringAddStub string_add_stub(NO_STRING_CHECK_IN_STUB);
+ StringAddStub string_add_right_stub(NO_STRING_CHECK_RIGHT_IN_STUB);
GenerateRegisterArgsPush(masm);
- __ TailCallStub(&string_add_stub);
+ __ TailCallStub(&string_add_right_stub);
// At least one argument is not a string.
__ bind(&call_runtime);
@@ -5442,18 +5446,19 @@
void StringAddStub::Generate(MacroAssembler* masm) {
- Label string_add_runtime;
+ Label string_add_runtime, call_builtin;
+ Builtins::JavaScript builtin_id = Builtins::ADD;
+
// Stack on entry:
- // sp[0]: second argument.
- // sp[4]: first argument.
+ // sp[0]: second argument (right).
+ // sp[4]: first argument (left).
// Load the two arguments.
__ ldr(r0, MemOperand(sp, 1 * kPointerSize)); // First argument.
__ ldr(r1, MemOperand(sp, 0 * kPointerSize)); // Second argument.
// Make sure that both arguments are strings if not known in advance.
- if (string_check_) {
- STATIC_ASSERT(kSmiTag == 0);
+ if (flags_ == NO_STRING_ADD_FLAGS) {
__ JumpIfEitherSmi(r0, r1, &string_add_runtime);
// Load instance types.
__ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset));
@@ -5465,13 +5470,27 @@
__ tst(r4, Operand(kIsNotStringMask));
__ tst(r5, Operand(kIsNotStringMask), eq);
__ b(ne, &string_add_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;
+ }
}
// Both arguments are strings.
// r0: first string
// r1: second string
- // r4: first string instance type (if string_check_)
- // r5: second string instance type (if string_check_)
+ // r4: first string instance type (if flags_ == NO_STRING_ADD_FLAGS)
+ // r5: second string instance type (if flags_ == NO_STRING_ADD_FLAGS)
{
Label strings_not_empty;
// Check if either of the strings are empty. In that case return the
other.
@@ -5499,8 +5518,8 @@
// r1: second string
// r2: length of first string
// r3: length of second string
- // r4: first string instance type (if string_check_)
- // r5: second string instance type (if string_check_)
+ // r4: first string instance type (if flags_ == NO_STRING_ADD_FLAGS)
+ // r5: second string instance type (if flags_ == NO_STRING_ADD_FLAGS)
// Look at the length of the result of adding the two strings.
Label string_add_flat_result, longer_than_two;
// Adding two lengths can't overflow.
@@ -5512,7 +5531,7 @@
__ b(ne, &longer_than_two);
// Check that both strings are non-external ascii strings.
- if (!string_check_) {
+ if (flags_ != NO_STRING_ADD_FLAGS) {
__ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset));
__ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset));
__ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset));
@@ -5560,7 +5579,7 @@
// 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 (!string_check_) {
+ if (flags_ != NO_STRING_ADD_FLAGS) {
__ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset));
__ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset));
__ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset));
@@ -5608,11 +5627,11 @@
// r1: second string
// r2: length of first string
// r3: length of second string
- // r4: first string instance type (if string_check_)
- // r5: second string instance type (if string_check_)
+ // r4: first string instance type (if flags_ == NO_STRING_ADD_FLAGS)
+ // r5: second string instance type (if flags_ == NO_STRING_ADD_FLAGS)
// r6: sum of lengths.
__ bind(&string_add_flat_result);
- if (!string_check_) {
+ if (flags_ != NO_STRING_ADD_FLAGS) {
__ ldr(r4, FieldMemOperand(r0, HeapObject::kMapOffset));
__ ldr(r5, FieldMemOperand(r1, HeapObject::kMapOffset));
__ ldrb(r4, FieldMemOperand(r4, Map::kInstanceTypeOffset));
@@ -5710,6 +5729,60 @@
// Just jump to runtime to add the two strings.
__ bind(&string_add_runtime);
__ TailCallRuntime(Runtime::kStringAdd, 2, 1);
+
+ if (call_builtin.is_linked()) {
+ __ bind(&call_builtin);
+ __ InvokeBuiltin(builtin_id, JUMP_JS);
+ }
+}
+
+
+void StringAddStub::GenerateConvertArgument(MacroAssembler* masm,
+ int stack_offset,
+ Register arg,
+ Register scratch1,
+ Register scratch2,
+ Register scratch3,
+ Register scratch4,
+ Label* slow) {
+ // First check if the argument is already a string.
+ Label not_string, done;
+ __ JumpIfSmi(arg, ¬_string);
+ __ CompareObjectType(arg, scratch1, scratch1, FIRST_NONSTRING_TYPE);
+ __ b(lt, &done);
+
+ // Check the number to string cache.
+ Label not_cached;
+ __ bind(¬_string);
+ // Puts the cached result into scratch1.
+ NumberToStringStub::GenerateLookupNumberStringCache(masm,
+ arg,
+ scratch1,
+ scratch2,
+ scratch3,
+ scratch4,
+ false,
+ ¬_cached);
+ __ mov(arg, scratch1);
+ __ str(arg, MemOperand(sp, stack_offset));
+ __ jmp(&done);
+
+ // Check if the argument is a safe string wrapper.
+ __ bind(¬_cached);
+ __ JumpIfSmi(arg, slow);
+ __ CompareObjectType(
+ arg, scratch1, scratch2, JS_VALUE_TYPE); // map -> scratch1.
+ __ b(ne, slow);
+ __ ldrb(scratch2, FieldMemOperand(scratch1, Map::kBitField2Offset));
+ __ and_(scratch2,
+ scratch2, Operand(1 <<
Map::kStringWrapperSafeForDefaultValueOf));
+ __ cmp(scratch2,
+ Operand(1 << Map::kStringWrapperSafeForDefaultValueOf));
+ __ b(ne, slow);
+ __ ldr(arg, FieldMemOperand(arg, JSValue::kValueOffset));
+ __ str(arg, MemOperand(sp, stack_offset));
+
+ __ bind(&done);
}
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.h Tue Feb 15 05:53:51
2011
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.h Tue Feb 22 01:35:37
2011
@@ -335,24 +335,36 @@
// Flag that indicates how to generate code for the stub StringAddStub.
enum StringAddFlags {
NO_STRING_ADD_FLAGS = 0,
- NO_STRING_CHECK_IN_STUB = 1 << 0 // Omit string check in stub.
+ // Omit left string check in stub (left is definitely a string).
+ NO_STRING_CHECK_LEFT_IN_STUB = 1 << 0,
+ // Omit right string check in stub (right is definitely a string).
+ NO_STRING_CHECK_RIGHT_IN_STUB = 1 << 1,
+ // 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 CodeStub {
public:
- explicit StringAddStub(StringAddFlags flags) {
- string_check_ = ((flags & NO_STRING_CHECK_IN_STUB) == 0);
- }
+ explicit StringAddStub(StringAddFlags flags) : flags_(flags) {}
private:
Major MajorKey() { return StringAdd; }
- int MinorKey() { return string_check_ ? 0 : 1; }
+ int MinorKey() { return flags_; }
void Generate(MacroAssembler* masm);
- // Should the stub check whether arguments are strings?
- bool string_check_;
+ void GenerateConvertArgument(MacroAssembler* masm,
+ int stack_offset,
+ Register arg,
+ Register scratch1,
+ Register scratch2,
+ Register scratch3,
+ Register scratch4,
+ Label* slow);
+
+ const StringAddFlags flags_;
};
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
