Revision: 3313 Author: [email protected] Date: Mon Nov 16 15:11:19 2009 Log: Re-enable using push instructions for syncing the virtual frame.
This change fixes the problem with the original version of this approach (r3032) that may lead to a corrupted stack if we would invoke spilling during syncing a large SMI constant (unsafe SMIs) in the virtual frame. The new code for storing unsafe SMI constants does not use an extra temporary register. This prevents the compiler from ever having to spill during a virutal frame sync operation. For storing a large SMI constant we previously generated: mov ecx, (large_smi & 0x0000ffff) xor ecx, (large_smi & 0xffff0000) push ecx we now generate: push (large_smi & 0x0000ffff) or [esp], (large_smi & 0xffff0000) Not using a temporary register avoids spilling within an nvocation of VirtualFrame::SyncRange. Review URL: http://codereview.chromium.org/391079 http://code.google.com/p/v8/source/detail?r=3313 Modified: /branches/bleeding_edge/src/ia32/codegen-ia32.cc /branches/bleeding_edge/src/ia32/codegen-ia32.h /branches/bleeding_edge/src/ia32/register-allocator-ia32.cc /branches/bleeding_edge/src/ia32/virtual-frame-ia32.cc ======================================= --- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Mon Nov 16 13:59:31 2009 +++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Mon Nov 16 15:11:19 2009 @@ -3939,12 +3939,28 @@ } -void CodeGenerator::LoadUnsafeSmi(Register target, Handle<Object> value) { +void CodeGenerator::PushUnsafeSmi(Handle<Object> value) { + ASSERT(value->IsSmi()); + int bits = reinterpret_cast<int>(*value); + __ push(Immediate(bits & 0x0000FFFF)); + __ or_(Operand(esp, 0), Immediate(bits & 0xFFFF0000)); +} + + +void CodeGenerator::StoreUnsafeSmiToLocal(int offset, Handle<Object> value) { + ASSERT(value->IsSmi()); + int bits = reinterpret_cast<int>(*value); + __ mov(Operand(ebp, offset), Immediate(bits & 0x0000FFFF)); + __ or_(Operand(ebp, offset), Immediate(bits & 0xFFFF0000)); +} + + +void CodeGenerator::MoveUnsafeSmi(Register target, Handle<Object> value) { ASSERT(target.is_valid()); ASSERT(value->IsSmi()); int bits = reinterpret_cast<int>(*value); __ Set(target, Immediate(bits & 0x0000FFFF)); - __ xor_(target, bits & 0xFFFF0000); + __ or_(target, bits & 0xFFFF0000); } ======================================= --- /branches/bleeding_edge/src/ia32/codegen-ia32.h Mon Nov 16 13:59:31 2009 +++ /branches/bleeding_edge/src/ia32/codegen-ia32.h Mon Nov 16 15:11:19 2009 @@ -468,9 +468,11 @@ // than 16 bits. static const int kMaxSmiInlinedBits = 16; bool IsUnsafeSmi(Handle<Object> value); - // Load an integer constant x into a register target using + // Load an integer constant x into a register target or into the stack using // at most 16 bits of user-controlled data per assembly operation. - void LoadUnsafeSmi(Register target, Handle<Object> value); + void MoveUnsafeSmi(Register target, Handle<Object> value); + void StoreUnsafeSmiToLocal(int offset, Handle<Object> value); + void PushUnsafeSmi(Handle<Object> value); void CallWithArguments(ZoneList<Expression*>* arguments, int position); ======================================= --- /branches/bleeding_edge/src/ia32/register-allocator-ia32.cc Wed Sep 9 03:49:40 2009 +++ /branches/bleeding_edge/src/ia32/register-allocator-ia32.cc Mon Nov 16 15:11:19 2009 @@ -42,7 +42,7 @@ Result fresh = CodeGeneratorScope::Current()->allocator()->Allocate(); ASSERT(fresh.is_valid()); if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) { - CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle()); + CodeGeneratorScope::Current()->MoveUnsafeSmi(fresh.reg(), handle()); } else { CodeGeneratorScope::Current()->masm()->Set(fresh.reg(), Immediate(handle())); @@ -64,7 +64,7 @@ } else { ASSERT(is_constant()); if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) { - CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle()); + CodeGeneratorScope::Current()->MoveUnsafeSmi(fresh.reg(), handle()); } else { CodeGeneratorScope::Current()->masm()->Set(fresh.reg(), Immediate(handle())); ======================================= --- /branches/bleeding_edge/src/ia32/virtual-frame-ia32.cc Tue Nov 10 06:58:16 2009 +++ /branches/bleeding_edge/src/ia32/virtual-frame-ia32.cc Mon Nov 16 15:11:19 2009 @@ -75,10 +75,7 @@ case FrameElement::CONSTANT: if (cgen()->IsUnsafeSmi(element.handle())) { - Result temp = cgen()->allocator()->Allocate(); - ASSERT(temp.is_valid()); - cgen()->LoadUnsafeSmi(temp.reg(), element.handle()); - __ mov(Operand(ebp, fp_relative(index)), temp.reg()); + cgen()->StoreUnsafeSmiToLocal(fp_relative(index), element.handle()); } else { __ Set(Operand(ebp, fp_relative(index)), Immediate(element.handle())); @@ -127,10 +124,7 @@ case FrameElement::CONSTANT: if (cgen()->IsUnsafeSmi(element.handle())) { - Result temp = cgen()->allocator()->Allocate(); - ASSERT(temp.is_valid()); - cgen()->LoadUnsafeSmi(temp.reg(), element.handle()); - __ push(temp.reg()); + cgen()->PushUnsafeSmi(element.handle()); } else { __ push(Immediate(element.handle())); } @@ -161,15 +155,16 @@ // on the stack. int start = Min(begin, stack_pointer_ + 1); - // If positive we have to adjust the stack pointer. - int delta = end - stack_pointer_; - if (delta > 0) { - stack_pointer_ = end; - __ sub(Operand(esp), Immediate(delta * kPointerSize)); - } - + // Emit normal push instructions for elements above stack pointer + // and use mov instructions if we are below stack pointer. for (int i = start; i <= end; i++) { - if (!elements_[i].is_synced()) SyncElementBelowStackPointer(i); + if (!elements_[i].is_synced()) { + if (i <= stack_pointer_) { + SyncElementBelowStackPointer(i); + } else { + SyncElementByPushing(i); + } + } } } @@ -198,7 +193,7 @@ // Emit a move. if (element.is_constant()) { if (cgen()->IsUnsafeSmi(element.handle())) { - cgen()->LoadUnsafeSmi(fresh.reg(), element.handle()); + cgen()->MoveUnsafeSmi(fresh.reg(), element.handle()); } else { __ Set(fresh.reg(), Immediate(element.handle())); } @@ -299,7 +294,7 @@ if (!source.is_synced()) { if (cgen()->IsUnsafeSmi(source.handle())) { esi_caches = i; - cgen()->LoadUnsafeSmi(esi, source.handle()); + cgen()->MoveUnsafeSmi(esi, source.handle()); __ mov(Operand(ebp, fp_relative(i)), esi); } else { __ Set(Operand(ebp, fp_relative(i)), Immediate(source.handle())); @@ -407,7 +402,7 @@ case FrameElement::CONSTANT: if (cgen()->IsUnsafeSmi(source.handle())) { - cgen()->LoadUnsafeSmi(target_reg, source.handle()); + cgen()->MoveUnsafeSmi(target_reg, source.handle()); } else { __ Set(target_reg, Immediate(source.handle())); } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
