From 5a54b7030dffe3f5836b28db3cb2700e0995041d Mon Sep 17 00:00:00 2001 From: "erik.corry@gmail.com" Date: Thu, 1 Jul 2010 15:06:24 +0000 Subject: [PATCH] ARM: Don't emit a write barrier for an inlined keyed load if the right hand side is a literal like true, false, etc. Also if the value is not a likely Smi we inline the newspace check. Review URL: http://codereview.chromium.org/2833048 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4999 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 90 +++++++++++++++++++++++++++++++----------- src/arm/codegen-arm.h | 5 ++- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 122bb721b..d8a615f1e 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -1745,11 +1745,15 @@ void CodeGenerator::VisitDeclaration(Declaration* node) { val = node->fun(); // NULL if we don't have a function } + if (val != NULL) { + WriteBarrierCharacter wb_info = + val->type()->IsLikelySmi() ? LIKELY_SMI : UNLIKELY_SMI; + if (val->AsLiteral() != NULL) wb_info = NEVER_NEWSPACE; // Set initial value. Reference target(this, node->proxy()); Load(val); - target.SetValue(NOT_CONST_INIT); + target.SetValue(NOT_CONST_INIT, wb_info); // Get rid of the assigned value (declarations are statements). frame_->Drop(); @@ -2485,13 +2489,13 @@ void CodeGenerator::VisitForInStatement(ForInStatement* node) { if (each.size() > 0) { __ ldr(r0, frame_->ElementAt(each.size())); frame_->EmitPush(r0); - each.SetValue(NOT_CONST_INIT); + each.SetValue(NOT_CONST_INIT, UNLIKELY_SMI); frame_->Drop(2); } else { // If the reference was to a slot we rely on the convenient property // that it doesn't matter whether a value (eg, r3 pushed above) is // right on top of or right underneath a zero-sized reference. - each.SetValue(NOT_CONST_INIT); + each.SetValue(NOT_CONST_INIT, UNLIKELY_SMI); frame_->Drop(); } } @@ -3646,6 +3650,8 @@ void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { // Evaluate the receiver subexpression. Load(prop->obj()); + WriteBarrierCharacter wb_info; + // Change to slow case in the beginning of an initialization block to // avoid the quadratic behavior of repeatedly adding fast properties. if (node->starts_initialization_block()) { @@ -3667,7 +3673,7 @@ void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { // [tos] : key // [tos+1] : receiver // [tos+2] : receiver if at the end of an initialization block - + // // Evaluate the right-hand side. if (node->is_compound()) { // For a compound assignment the right-hand side is a binary operation @@ -3699,9 +3705,13 @@ void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { overwrite_value ? OVERWRITE_RIGHT : NO_OVERWRITE, inline_smi); } + wb_info = node->type()->IsLikelySmi() ? LIKELY_SMI : UNLIKELY_SMI; } else { // For non-compound assignment just load the right-hand side. Load(node->value()); + wb_info = node->value()->AsLiteral() != NULL ? + NEVER_NEWSPACE : + (node->value()->type()->IsLikelySmi() ? LIKELY_SMI : UNLIKELY_SMI); } // Stack layout: @@ -3713,7 +3723,7 @@ void CodeGenerator::EmitKeyedPropertyAssignment(Assignment* node) { // Perform the assignment. It is safe to ignore constants here. ASSERT(node->op() != Token::INIT_CONST); CodeForSourcePosition(node->position()); - EmitKeyedStore(prop->key()->type()); + EmitKeyedStore(prop->key()->type(), wb_info); frame_->EmitPush(r0); // Stack layout: @@ -5509,7 +5519,7 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) { __ sub(value, value, Operand(Smi::FromInt(1))); } frame_->EmitPush(value); - target.SetValue(NOT_CONST_INIT); + target.SetValue(NOT_CONST_INIT, LIKELY_SMI); if (is_postfix) frame_->Pop(); ASSERT_EQ(original_height + 1, frame_->height()); return; @@ -5608,7 +5618,7 @@ void CodeGenerator::VisitCountOperation(CountOperation* node) { // Set the target with the result, leaving the result on // top of the stack. Removes the target from the stack if // it has a non-zero size. - if (!is_const) target.SetValue(NOT_CONST_INIT); + if (!is_const) target.SetValue(NOT_CONST_INIT, LIKELY_SMI); } // Postfix: Discard the new value and use the old. @@ -6341,7 +6351,8 @@ void CodeGenerator::EmitKeyedLoad() { } -void CodeGenerator::EmitKeyedStore(StaticType* key_type) { +void CodeGenerator::EmitKeyedStore(StaticType* key_type, + WriteBarrierCharacter wb_info) { // Generate inlined version of the keyed store if the code is in a loop // and the key is likely to be a smi. if (loop_nesting() > 0 && key_type->IsLikelySmi()) { @@ -6357,25 +6368,45 @@ void CodeGenerator::EmitKeyedStore(StaticType* key_type) { __ IncrementCounter(&Counters::keyed_store_inline, 1, scratch1, scratch2); + + // Load the value, key and receiver from the stack. + bool value_is_harmless = frame_->KnownSmiAt(0); + if (wb_info == NEVER_NEWSPACE) value_is_harmless = true; + bool key_is_smi = frame_->KnownSmiAt(1); Register value = frame_->PopToRegister(); Register key = frame_->PopToRegister(value); VirtualFrame::SpilledScope spilled(frame_); Register receiver = r2; frame_->EmitPop(receiver); +#ifdef DEBUG + bool we_remembered_the_write_barrier = value_is_harmless; +#endif + // The deferred code expects value, key and receiver in registers. DeferredReferenceSetKeyedValue* deferred = new DeferredReferenceSetKeyedValue(value, key, receiver); // Check that the value is a smi. As this inlined code does not set the // write barrier it is only possible to store smi values. - __ tst(value, Operand(kSmiTagMask)); - deferred->Branch(ne); + if (!value_is_harmless) { + // If the value is not likely to be a Smi then let's test the fixed array + // for new space instead. See below. + if (wb_info == LIKELY_SMI) { + __ tst(value, Operand(kSmiTagMask)); + deferred->Branch(ne); +#ifdef DEBUG + we_remembered_the_write_barrier = true; +#endif + } + } - // Check that the key is a smi. - __ tst(key, Operand(kSmiTagMask)); - deferred->Branch(ne); + if (!key_is_smi) { + // Check that the key is a smi. + __ tst(key, Operand(kSmiTagMask)); + deferred->Branch(ne); + } // Check that the receiver is a heap object. __ tst(receiver, Operand(kSmiTagMask)); @@ -6391,24 +6422,35 @@ void CodeGenerator::EmitKeyedStore(StaticType* key_type) { __ cmp(scratch1, key); deferred->Branch(ls); // Unsigned less equal. + // Get the elements array from the receiver. + __ ldr(scratch1, FieldMemOperand(receiver, JSObject::kElementsOffset)); + if (!value_is_harmless && wb_info != LIKELY_SMI) { + Label ok; + __ and_(scratch2, scratch1, Operand(ExternalReference::new_space_mask())); + __ cmp(scratch2, Operand(ExternalReference::new_space_start())); + __ tst(value, Operand(kSmiTagMask), ne); + deferred->Branch(ne); +#ifdef DEBUG + we_remembered_the_write_barrier = true; +#endif + } + // Check that the elements array is not a dictionary. + __ ldr(scratch2, FieldMemOperand(scratch1, JSObject::kMapOffset)); // The following instructions are the part of the inlined store keyed // property code which can be patched. Therefore the exact number of // instructions generated need to be fixed, so the constant pool is blocked // while generating this code. { Assembler::BlockConstPoolScope block_const_pool(masm_); - // Get the elements array from the receiver and check that it - // is not a dictionary. - __ ldr(scratch1, FieldMemOperand(receiver, JSObject::kElementsOffset)); - __ ldr(scratch2, FieldMemOperand(scratch1, JSObject::kMapOffset)); +#ifdef DEBUG + Label check_inlined_codesize; + masm_->bind(&check_inlined_codesize); +#endif + // Read the fixed array map from the constant pool (not from the root // array) so that the value can be patched. When debugging, we patch this // comparison to always fail so that we will hit the IC call in the // deferred code which will allow the debugger to break for fast case // stores. -#ifdef DEBUG - Label check_inlined_codesize; - masm_->bind(&check_inlined_codesize); -#endif __ mov(scratch3, Operand(Factory::fixed_array_map())); __ cmp(scratch2, scratch3); deferred->Branch(ne); @@ -6425,6 +6467,8 @@ void CodeGenerator::EmitKeyedStore(StaticType* key_type) { masm_->InstructionsGeneratedSince(&check_inlined_codesize)); } + ASSERT(we_remembered_the_write_barrier); + deferred->BindExit(); } else { frame()->CallKeyedStoreIC(); @@ -6522,7 +6566,7 @@ void Reference::GetValue() { } -void Reference::SetValue(InitState init_state) { +void Reference::SetValue(InitState init_state, WriteBarrierCharacter wb_info) { ASSERT(!is_illegal()); ASSERT(!cgen_->has_cc()); MacroAssembler* masm = cgen_->masm(); @@ -6554,7 +6598,7 @@ void Reference::SetValue(InitState init_state) { Property* property = expression_->AsProperty(); ASSERT(property != NULL); cgen_->CodeForSourcePosition(property->position()); - cgen_->EmitKeyedStore(property->key()->type()); + cgen_->EmitKeyedStore(property->key()->type(), wb_info); frame->EmitPush(r0); set_unloaded(); break; diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h index 2d8a93520..855723d9c 100644 --- a/src/arm/codegen-arm.h +++ b/src/arm/codegen-arm.h @@ -44,6 +44,7 @@ class RegisterFile; enum InitState { CONST_INIT, NOT_CONST_INIT }; enum TypeofState { INSIDE_TYPEOF, NOT_INSIDE_TYPEOF }; enum GenerateInlineSmi { DONT_GENERATE_INLINE_SMI, GENERATE_INLINE_SMI }; +enum WriteBarrierCharacter { UNLIKELY_SMI, LIKELY_SMI, NEVER_NEWSPACE }; // ------------------------------------------------------------------------- @@ -100,7 +101,7 @@ class Reference BASE_EMBEDDED { // on the expression stack. The value is stored in the location specified // by the reference, and is left on top of the stack, after the reference // is popped from beneath it (unloaded). - void SetValue(InitState init_state); + void SetValue(InitState init_state, WriteBarrierCharacter wb); // This is in preparation for something that uses the reference on the stack. // If we need this reference afterwards get then dup it now. Otherwise mark @@ -384,7 +385,7 @@ class CodeGenerator: public AstVisitor { // Store a keyed property. Key and receiver are on the stack and the value is // in r0. Result is returned in r0. - void EmitKeyedStore(StaticType* key_type); + void EmitKeyedStore(StaticType* key_type, WriteBarrierCharacter wb_info); void LoadFromGlobalSlotCheckExtensions(Slot* slot, TypeofState typeof_state, -- 2.34.1