From cb10ceb19da812e971a4056bb6b2e79710f25dc8 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Thu, 12 Sep 2013 16:17:58 +0000 Subject: [PATCH] Reland "Clean up after r16292 (disable optimization for StringWrappers)." BUG= R=ulan@chromium.org Review URL: https://codereview.chromium.org/23619036 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16691 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/code-stubs-arm.cc | 20 +------------------- src/arm/full-codegen-arm.cc | 18 ++++++++++++------ src/ia32/code-stubs-ia32.cc | 16 +--------------- src/ia32/full-codegen-ia32.cc | 15 ++++++++++----- src/mips/code-stubs-mips.cc | 17 +---------------- src/mips/full-codegen-mips.cc | 18 ++++++++++++------ src/x64/code-stubs-x64.cc | 16 +--------------- src/x64/full-codegen-x64.cc | 29 ++++++++++++++++------------- test/mjsunit/regress/regress-2855.js | 16 ++++++++++++++++ 9 files changed, 70 insertions(+), 95 deletions(-) diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 3a1d128..8ec0591 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -5790,7 +5790,6 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, __ b(lt, &done); // Check the number to string cache. - Label not_cached; __ bind(¬_string); // Puts the cached result into scratch1. NumberToStringStub::GenerateLookupNumberStringCache(masm, @@ -5799,26 +5798,9 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, scratch2, scratch3, scratch4, - ¬_cached); + slow); __ 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); } diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 28ec1a2..b6fb70b 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -3007,7 +3007,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( VisitForAccumulatorValue(args->at(0)); - Label materialize_true, materialize_false; + Label materialize_true, materialize_false, skip_lookup; Label* if_true = NULL; Label* if_false = NULL; Label* fall_through = NULL; @@ -3019,7 +3019,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ ldr(r1, FieldMemOperand(r0, HeapObject::kMapOffset)); __ ldrb(ip, FieldMemOperand(r1, Map::kBitField2Offset)); __ tst(ip, Operand(1 << Map::kStringWrapperSafeForDefaultValueOf)); - __ b(ne, if_true); + __ b(ne, &skip_lookup); // Check for fast case object. Generate false result for slow case object. __ ldr(r2, FieldMemOperand(r0, JSObject::kPropertiesOffset)); @@ -3065,6 +3065,14 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ b(ne, &loop); __ bind(&done); + + // Set the bit in the map to indicate that there is no local valueOf field. + __ ldrb(r2, FieldMemOperand(r1, Map::kBitField2Offset)); + __ orr(r2, r2, Operand(1 << Map::kStringWrapperSafeForDefaultValueOf)); + __ strb(r2, FieldMemOperand(r1, Map::kBitField2Offset)); + + __ bind(&skip_lookup); + // If a valueOf property is not found on the object check that its // prototype is the un-modified String prototype. If not result is false. __ ldr(r2, FieldMemOperand(r1, Map::kPrototypeOffset)); @@ -3074,11 +3082,9 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ ldr(r3, FieldMemOperand(r3, GlobalObject::kNativeContextOffset)); __ ldr(r3, ContextOperand(r3, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX)); __ cmp(r2, r3); - __ b(ne, if_false); - - __ jmp(if_true); - PrepareForBailoutBeforeSplit(expr, true, if_true, if_false); + Split(eq, if_true, if_false, fall_through); + context()->Plug(if_true, if_false); } diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index ee9e535..fd58f00 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -5515,7 +5515,6 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, __ j(below, &done); // Check the number to string cache. - Label not_cached; __ bind(¬_string); // Puts the cached result into scratch1. NumberToStringStub::GenerateLookupNumberStringCache(masm, @@ -5523,22 +5522,9 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, scratch1, scratch2, scratch3, - ¬_cached); + slow); __ mov(arg, scratch1); __ mov(Operand(esp, stack_offset), arg); - __ jmp(&done); - - // Check if the argument is a safe string wrapper. - __ bind(¬_cached); - __ JumpIfSmi(arg, slow); - __ CmpObjectType(arg, JS_VALUE_TYPE, scratch1); // map -> scratch1. - __ j(not_equal, slow); - __ test_b(FieldOperand(scratch1, Map::kBitField2Offset), - 1 << Map::kStringWrapperSafeForDefaultValueOf); - __ j(zero, slow); - __ mov(arg, FieldOperand(arg, JSValue::kValueOffset)); - __ mov(Operand(esp, stack_offset), arg); - __ bind(&done); } diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index db82b39..6d39cc1 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -2956,7 +2956,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( VisitForAccumulatorValue(args->at(0)); - Label materialize_true, materialize_false; + Label materialize_true, materialize_false, skip_lookup; Label* if_true = NULL; Label* if_false = NULL; Label* fall_through = NULL; @@ -2970,7 +2970,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset)); __ test_b(FieldOperand(ebx, Map::kBitField2Offset), 1 << Map::kStringWrapperSafeForDefaultValueOf); - __ j(not_zero, if_true); + __ j(not_zero, &skip_lookup); // Check for fast case object. Return false for slow case objects. __ mov(ecx, FieldOperand(eax, JSObject::kPropertiesOffset)); @@ -3016,6 +3016,12 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( // Reload map as register ebx was used as temporary above. __ mov(ebx, FieldOperand(eax, HeapObject::kMapOffset)); + // Set the bit in the map to indicate that there is no local valueOf field. + __ or_(FieldOperand(ebx, Map::kBitField2Offset), + Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf)); + + __ bind(&skip_lookup); + // If a valueOf property is not found on the object check that its // prototype is the un-modified String prototype. If not result is false. __ mov(ecx, FieldOperand(ebx, Map::kPrototypeOffset)); @@ -3027,10 +3033,9 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ cmp(ecx, ContextOperand(edx, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX)); - __ j(not_equal, if_false); - __ jmp(if_true); - PrepareForBailoutBeforeSplit(expr, true, if_true, if_false); + Split(equal, if_true, if_false, fall_through); + context()->Plug(if_true, if_false); } diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index b536209..68b0c5f 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -5861,7 +5861,6 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, __ Branch(&done, lt, scratch1, Operand(FIRST_NONSTRING_TYPE)); // Check the number to string cache. - Label not_cached; __ bind(¬_string); // Puts the cached result into scratch1. NumberToStringStub::GenerateLookupNumberStringCache(masm, @@ -5870,23 +5869,9 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, scratch2, scratch3, scratch4, - ¬_cached); + slow); __ mov(arg, scratch1); __ sw(arg, MemOperand(sp, stack_offset)); - __ jmp(&done); - - // Check if the argument is a safe string wrapper. - __ bind(¬_cached); - __ JumpIfSmi(arg, slow); - __ GetObjectType(arg, scratch1, scratch2); // map -> scratch1. - __ Branch(slow, ne, scratch2, Operand(JS_VALUE_TYPE)); - __ lbu(scratch2, FieldMemOperand(scratch1, Map::kBitField2Offset)); - __ li(scratch4, 1 << Map::kStringWrapperSafeForDefaultValueOf); - __ And(scratch2, scratch2, scratch4); - __ Branch(slow, ne, scratch2, Operand(scratch4)); - __ lw(arg, FieldMemOperand(arg, JSValue::kValueOffset)); - __ sw(arg, MemOperand(sp, stack_offset)); - __ bind(&done); } diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index bf6f1c8..df3f417 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -3028,7 +3028,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( VisitForAccumulatorValue(args->at(0)); - Label materialize_true, materialize_false; + Label materialize_true, materialize_false, skip_lookup; Label* if_true = NULL; Label* if_false = NULL; Label* fall_through = NULL; @@ -3040,7 +3040,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ lw(a1, FieldMemOperand(v0, HeapObject::kMapOffset)); __ lbu(t0, FieldMemOperand(a1, Map::kBitField2Offset)); __ And(t0, t0, 1 << Map::kStringWrapperSafeForDefaultValueOf); - __ Branch(if_true, ne, t0, Operand(zero_reg)); + __ Branch(&skip_lookup, ne, t0, Operand(zero_reg)); // Check for fast case object. Generate false result for slow case object. __ lw(a2, FieldMemOperand(v0, JSObject::kPropertiesOffset)); @@ -3086,6 +3086,14 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ Branch(&loop, ne, t0, Operand(a2)); __ bind(&done); + + // Set the bit in the map to indicate that there is no local valueOf field. + __ lbu(a2, FieldMemOperand(a1, Map::kBitField2Offset)); + __ Or(a2, a2, Operand(1 << Map::kStringWrapperSafeForDefaultValueOf)); + __ sb(a2, FieldMemOperand(a1, Map::kBitField2Offset)); + + __ bind(&skip_lookup); + // If a valueOf property is not found on the object check that its // prototype is the un-modified String prototype. If not result is false. __ lw(a2, FieldMemOperand(a1, Map::kPrototypeOffset)); @@ -3094,11 +3102,9 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ lw(a3, ContextOperand(cp, Context::GLOBAL_OBJECT_INDEX)); __ lw(a3, FieldMemOperand(a3, GlobalObject::kNativeContextOffset)); __ lw(a3, ContextOperand(a3, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX)); - __ Branch(if_false, ne, a2, Operand(a3)); - - __ jmp(if_true); - PrepareForBailoutBeforeSplit(expr, true, if_true, if_false); + Split(eq, a2, Operand(a3), if_true, if_false, fall_through); + context()->Plug(if_true, if_false); } diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index f6cfb4f..12f6c17 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -4644,7 +4644,6 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, __ j(below, &done); // Check the number to string cache. - Label not_cached; __ bind(¬_string); // Puts the cached result into scratch1. NumberToStringStub::GenerateLookupNumberStringCache(masm, @@ -4652,22 +4651,9 @@ void StringAddStub::GenerateConvertArgument(MacroAssembler* masm, scratch1, scratch2, scratch3, - ¬_cached); + slow); __ movq(arg, scratch1); __ movq(Operand(rsp, stack_offset), arg); - __ jmp(&done); - - // Check if the argument is a safe string wrapper. - __ bind(¬_cached); - __ JumpIfSmi(arg, slow); - __ CmpObjectType(arg, JS_VALUE_TYPE, scratch1); // map -> scratch1. - __ j(not_equal, slow); - __ testb(FieldOperand(scratch1, Map::kBitField2Offset), - Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf)); - __ j(zero, slow); - __ movq(arg, FieldOperand(arg, JSValue::kValueOffset)); - __ movq(Operand(rsp, stack_offset), arg); - __ bind(&done); } diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index c182dd0..c24512e 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -2935,7 +2935,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( VisitForAccumulatorValue(args->at(0)); - Label materialize_true, materialize_false; + Label materialize_true, materialize_false, skip_lookup; Label* if_true = NULL; Label* if_false = NULL; Label* fall_through = NULL; @@ -2949,7 +2949,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ movq(rbx, FieldOperand(rax, HeapObject::kMapOffset)); __ testb(FieldOperand(rbx, Map::kBitField2Offset), Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf)); - __ j(not_zero, if_true); + __ j(not_zero, &skip_lookup); // Check for fast case object. Generate false result for slow case object. __ movq(rcx, FieldOperand(rax, JSObject::kPropertiesOffset)); @@ -2967,7 +2967,7 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ cmpq(rcx, Immediate(0)); __ j(equal, &done); - __ LoadInstanceDescriptors(rbx, rbx); + __ LoadInstanceDescriptors(rbx, r8); // rbx: descriptor array. // rcx: valid entries in the descriptor array. // Calculate the end of the descriptor array. @@ -2975,24 +2975,28 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( SmiIndex index = masm_->SmiToIndex(rdx, rcx, kPointerSizeLog2); __ lea(rcx, Operand( - rbx, index.reg, index.scale, DescriptorArray::kFirstOffset)); + r8, index.reg, index.scale, DescriptorArray::kFirstOffset)); // Calculate location of the first key name. - __ addq(rbx, Immediate(DescriptorArray::kFirstOffset)); + __ addq(r8, Immediate(DescriptorArray::kFirstOffset)); // Loop through all the keys in the descriptor array. If one of these is the // internalized string "valueOf" the result is false. __ jmp(&entry); __ bind(&loop); - __ movq(rdx, FieldOperand(rbx, 0)); + __ movq(rdx, FieldOperand(r8, 0)); __ Cmp(rdx, isolate()->factory()->value_of_string()); __ j(equal, if_false); - __ addq(rbx, Immediate(DescriptorArray::kDescriptorSize * kPointerSize)); + __ addq(r8, Immediate(DescriptorArray::kDescriptorSize * kPointerSize)); __ bind(&entry); - __ cmpq(rbx, rcx); + __ cmpq(r8, rcx); __ j(not_equal, &loop); __ bind(&done); - // Reload map as register rbx was used as temporary above. - __ movq(rbx, FieldOperand(rax, HeapObject::kMapOffset)); + + // Set the bit in the map to indicate that there is no local valueOf field. + __ or_(FieldOperand(rbx, Map::kBitField2Offset), + Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf)); + + __ bind(&skip_lookup); // If a valueOf property is not found on the object check that its // prototype is the un-modified String prototype. If not result is false. @@ -3004,10 +3008,9 @@ void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf( __ movq(rdx, FieldOperand(rdx, GlobalObject::kNativeContextOffset)); __ cmpq(rcx, ContextOperand(rdx, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX)); - __ j(not_equal, if_false); - __ jmp(if_true); - PrepareForBailoutBeforeSplit(expr, true, if_true, if_false); + Split(equal, if_true, if_false, fall_through); + context()->Plug(if_true, if_false); } diff --git a/test/mjsunit/regress/regress-2855.js b/test/mjsunit/regress/regress-2855.js index ac721af..24ec452 100644 --- a/test/mjsunit/regress/regress-2855.js +++ b/test/mjsunit/regress/regress-2855.js @@ -39,3 +39,19 @@ for (var i = 0; i < 4; ++i) { } assertEquals(expected, "" + foo("hello")); } + +// Make sure we look up "valueOf" only once. +var count = 0; +var x = new String("foo"); +Object.defineProperty(x, "valueOf", + { get: function() { + count++; + return function() { + return 11; + } + } + }); +for (var i = 0; i < 3; i++) { + assertEquals("11", "" + x); + assertEquals(i + 1, count); +} -- 2.7.4