From 0c4fd55c577e7ebcc7fe4202d700287bba022c56 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Fri, 12 Sep 2008 08:24:57 +0000 Subject: [PATCH] Change the code generator state constructor to implicitly push the state on stack, rather than explicitly saving and restoring it. Review URL: http://codereview.chromium.org/3002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@294 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/codegen-arm.cc | 132 ++++++++++++++++++++++++++++++++++++---------------- src/codegen-ia32.cc | 129 ++++++++++++++++++++++++++++++++++---------------- 2 files changed, 180 insertions(+), 81 deletions(-) diff --git a/src/codegen-arm.cc b/src/codegen-arm.cc index 44c33fa..f24473c 100644 --- a/src/codegen-arm.cc +++ b/src/codegen-arm.cc @@ -101,9 +101,18 @@ class Reference BASE_EMBEDDED { }; -// ----------------------------------------------------------------------------- +// ------------------------------------------------------------------------- // Code generation state +// The state is passed down the AST by the code generator. It is passed +// implicitly (in a member variable) to the non-static code generator member +// functions, and explicitly (as an argument) to the static member functions +// and the AST node member functions. +// +// The state is threaded through the call stack. Constructing a state +// implicitly pushes it on the owning code generator's stack of states, and +// destroying one implicitly pops it. + class CodeGenState BASE_EMBEDDED { public: enum AccessType { @@ -112,22 +121,26 @@ class CodeGenState BASE_EMBEDDED { LOAD_TYPEOF_EXPR }; - CodeGenState() - : access_(UNDEFINED), - ref_(NULL), - true_target_(NULL), - false_target_(NULL) { - } + // Create an initial code generator state. Destroying the initial state + // leaves the code generator with a NULL state. + CodeGenState(ArmCodeGenerator* owner); - CodeGenState(AccessType access, - Reference* ref, + // Create a code generator state based on a code generator's current + // state. The new state has its own access type and pair of branch + // labels, and no reference. + CodeGenState(ArmCodeGenerator* owner, + AccessType access, Label* true_target, - Label* false_target) - : access_(access), - ref_(ref), - true_target_(true_target), - false_target_(false_target) { - } + Label* false_target); + + // Create a code generator state based on a code generator's current + // state. The new state has an access type of LOAD, its own reference, + // and inherits the pair of branch labels of the current state. + CodeGenState(ArmCodeGenerator* owner, Reference* ref); + + // Destroy a code generator state and restore the owning code generator's + // previous state. + ~CodeGenState(); AccessType access() const { return access_; } Reference* ref() const { return ref_; } @@ -135,10 +148,12 @@ class CodeGenState BASE_EMBEDDED { Label* false_target() const { return false_target_; } private: + ArmCodeGenerator* owner_; AccessType access_; Reference* ref_; Label* true_target_; Label* false_target_; + CodeGenState* previous_; }; @@ -153,6 +168,9 @@ class ArmCodeGenerator: public CodeGenerator { MacroAssembler* masm() { return masm_; } + CodeGenState* state() { return state_; } + void set_state(CodeGenState* state) { state_ = state; } + private: // Assembler MacroAssembler* masm_; // to generate code @@ -243,7 +261,12 @@ class ArmCodeGenerator: public CodeGenerator { // Generate code to fetch the value of a reference. The reference is // expected to be on top of the expression stack. It is left in place and // its value is pushed on top of it. - void GetValue(Reference* ref); + void GetValue(Reference* ref) { + ASSERT(!has_cc()); + ASSERT(!ref->is_illegal()); + CodeGenState new_state(this, ref); + Visit(ref->expression()); + } // Generate code to store a value in a reference. The stored value is // expected on top of the expression stack, with the reference immediately @@ -341,6 +364,51 @@ class ArmCodeGenerator: public CodeGenerator { }; +// ------------------------------------------------------------------------- +// CodeGenState implementation. + +CodeGenState::CodeGenState(ArmCodeGenerator* owner) + : owner_(owner), + access_(UNDEFINED), + ref_(NULL), + true_target_(NULL), + false_target_(NULL), + previous_(NULL) { + owner_->set_state(this); +} + + +CodeGenState::CodeGenState(ArmCodeGenerator* owner, + AccessType access, + Label* true_target, + Label* false_target) + : owner_(owner), + access_(access), + ref_(NULL), + true_target_(true_target), + false_target_(false_target), + previous_(owner->state()) { + owner_->set_state(this); +} + + +CodeGenState::CodeGenState(ArmCodeGenerator* owner, Reference* ref) + : owner_(owner), + access_(LOAD), + ref_(ref), + true_target_(owner->state()->true_target_), + false_target_(owner->state()->false_target_), + previous_(owner->state()) { + owner_->set_state(this); +} + + +CodeGenState::~CodeGenState() { + ASSERT(owner_->state() == this); + owner_->set_state(previous_); +} + + // ----------------------------------------------------------------------------- // ArmCodeGenerator implementation @@ -456,8 +524,7 @@ void ArmCodeGenerator::GenCode(FunctionLiteral* fun) { ZoneList* body = fun->body(); // Initialize state. - { CodeGenState state; - state_ = &state; + { CodeGenState state(this); scope_ = scope; cc_reg_ = al; @@ -609,8 +676,6 @@ void ArmCodeGenerator::GenCode(FunctionLiteral* fun) { #endif VisitStatements(body); } - - state_ = NULL; } // exit @@ -717,11 +782,9 @@ void ArmCodeGenerator::LoadCondition(Expression* x, access == CodeGenState::LOAD_TYPEOF_EXPR); ASSERT(!has_cc() && !is_referenced()); - CodeGenState* old_state = state_; - CodeGenState new_state(access, NULL, true_target, false_target); - state_ = &new_state; - Visit(x); - state_ = old_state; + { CodeGenState new_state(this, access, true_target, false_target); + Visit(x); + } if (force_cc && !has_cc()) { // Convert the TOS value to a boolean in the condition code register. ToBoolean(true_target, false_target); @@ -869,18 +932,6 @@ void ArmCodeGenerator::UnloadReference(Reference* ref) { } -void ArmCodeGenerator::GetValue(Reference* ref) { - ASSERT(!has_cc()); - ASSERT(!ref->is_illegal()); - CodeGenState* old_state = state_; - CodeGenState new_state(CodeGenState::LOAD, ref, true_target(), - false_target()); - state_ = &new_state; - Visit(ref->expression()); - state_ = old_state; -} - - void Property::GenerateStoreCode(MacroAssembler* masm, Scope* scope, Reference* ref, @@ -953,7 +1004,6 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, ASSERT(var()->mode() != Variable::DYNAMIC); Label exit; - bool may_skip_write = false; if (init_state == CONST_INIT) { ASSERT(var()->mode() == Variable::CONST); // Only the first const initialization must be executed (the slot @@ -963,7 +1013,6 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, masm->ldr(r2, ArmCodeGenerator::SlotOperand(masm, scope, this, r2)); masm->cmp(r2, Operand(Factory::the_hole_value())); masm->b(ne, &exit); - may_skip_write = true; } // We must execute the store. @@ -983,7 +1032,6 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, // Skip write barrier if the written value is a smi. masm->tst(r0, Operand(kSmiTagMask)); masm->b(eq, &exit); - may_skip_write = true; // r2 is loaded with context when calling SlotOperand above. int offset = FixedArray::kHeaderSize + index() * kPointerSize; masm->mov(r3, Operand(offset)); @@ -991,7 +1039,9 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, } // If we definitely did not jump over the assignment, we do not need to // bind the exit label. Doing so can defeat peephole optimization. - if (may_skip_write) masm->bind(&exit); + if (init_state == CONST_INIT || type() == Slot::CONTEXT) { + masm->bind(&exit); + } } } diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc index 42fe7ca..8d39d7e 100644 --- a/src/codegen-ia32.cc +++ b/src/codegen-ia32.cc @@ -107,9 +107,18 @@ class Reference BASE_EMBEDDED { }; -// ----------------------------------------------------------------------------- +// ------------------------------------------------------------------------- // Code generation state +// The state is passed down the AST by the code generator. It is passed +// implicitly (in a member variable) to the non-static code generator member +// functions, and explicitly (as an argument) to the static member functions +// and the AST node member functions. +// +// The state is threaded through the call stack. Constructing a state +// implicitly pushes it on the owning code generator's stack of states, and +// destroying one implicitly pops it. + class CodeGenState BASE_EMBEDDED { public: enum AccessType { @@ -118,22 +127,26 @@ class CodeGenState BASE_EMBEDDED { LOAD_TYPEOF_EXPR }; - CodeGenState() - : access_(UNDEFINED), - ref_(NULL), - true_target_(NULL), - false_target_(NULL) { - } + // Create an initial code generator state. Destroying the initial state + // leaves the code generator with a NULL state. + CodeGenState(Ia32CodeGenerator* owner); - CodeGenState(AccessType access, - Reference* ref, + // Create a code generator state based on a code generator's current + // state. The new state has its own access type and pair of branch + // labels, and no reference. + CodeGenState(Ia32CodeGenerator* owner, + AccessType access, Label* true_target, - Label* false_target) - : access_(access), - ref_(ref), - true_target_(true_target), - false_target_(false_target) { - } + Label* false_target); + + // Create a code generator state based on a code generator's current + // state. The new state has an access type of LOAD, its own reference, + // and inherits the pair of branch labels of the current state. + CodeGenState(Ia32CodeGenerator* owner, Reference* ref); + + // Destroy a code generator state and restore the owning code generator's + // previous state. + ~CodeGenState(); AccessType access() const { return access_; } Reference* ref() const { return ref_; } @@ -141,10 +154,12 @@ class CodeGenState BASE_EMBEDDED { Label* false_target() const { return false_target_; } private: + Ia32CodeGenerator* owner_; AccessType access_; Reference* ref_; Label* true_target_; Label* false_target_; + CodeGenState* previous_; }; @@ -159,6 +174,9 @@ class Ia32CodeGenerator: public CodeGenerator { MacroAssembler* masm() { return masm_; } + CodeGenState* state() { return state_; } + void set_state(CodeGenState* state) { state_ = state; } + private: // Assembler MacroAssembler* masm_; // to generate code @@ -251,7 +269,12 @@ class Ia32CodeGenerator: public CodeGenerator { // Generate code to fetch the value of a reference. The reference is // expected to be on top of the expression stack. It is left in place and // its value is pushed on top of it. - void GetValue(Reference* ref); + void GetValue(Reference* ref) { + ASSERT(!has_cc()); + ASSERT(!ref->is_illegal()); + CodeGenState new_state(this, ref); + Visit(ref->expression()); + } // Generate code to store a value in a reference. The stored value is // expected on top of the expression stack, with the reference immediately @@ -356,6 +379,51 @@ class Ia32CodeGenerator: public CodeGenerator { }; +// ------------------------------------------------------------------------- +// CodeGenState implementation. + +CodeGenState::CodeGenState(Ia32CodeGenerator* owner) + : owner_(owner), + access_(UNDEFINED), + ref_(NULL), + true_target_(NULL), + false_target_(NULL), + previous_(NULL) { + owner_->set_state(this); +} + + +CodeGenState::CodeGenState(Ia32CodeGenerator* owner, + AccessType access, + Label* true_target, + Label* false_target) + : owner_(owner), + access_(access), + ref_(NULL), + true_target_(true_target), + false_target_(false_target), + previous_(owner->state()) { + owner_->set_state(this); +} + + +CodeGenState::CodeGenState(Ia32CodeGenerator* owner, Reference* ref) + : owner_(owner), + access_(LOAD), + ref_(ref), + true_target_(owner->state()->true_target_), + false_target_(owner->state()->false_target_), + previous_(owner->state()) { + owner_->set_state(this); +} + + +CodeGenState::~CodeGenState() { + ASSERT(owner_->state() == this); + owner_->set_state(previous_); +} + + // ----------------------------------------------------------------------------- // Ia32CodeGenerator implementation @@ -474,8 +542,7 @@ void Ia32CodeGenerator::GenCode(FunctionLiteral* fun) { ZoneList* body = fun->body(); // Initialize state. - { CodeGenState state; - state_ = &state; + { CodeGenState state(this); scope_ = scope; cc_reg_ = no_condition; @@ -659,8 +726,6 @@ void Ia32CodeGenerator::GenCode(FunctionLiteral* fun) { VisitReturnStatement(&statement); } } - - state_ = NULL; } // Code generation state must be reset. @@ -740,11 +805,9 @@ void Ia32CodeGenerator::LoadCondition(Expression* x, access == CodeGenState::LOAD_TYPEOF_EXPR); ASSERT(!has_cc() && !is_referenced()); - CodeGenState* old_state = state_; - CodeGenState new_state(access, NULL, true_target, false_target); - state_ = &new_state; - Visit(x); - state_ = old_state; + { CodeGenState new_state(this, access, true_target, false_target); + Visit(x); + } if (force_cc && !has_cc()) { ToBoolean(true_target, false_target); } @@ -892,18 +955,6 @@ void Ia32CodeGenerator::UnloadReference(Reference* ref) { } -void Ia32CodeGenerator::GetValue(Reference* ref) { - ASSERT(!has_cc()); - ASSERT(ref->type() != Reference::ILLEGAL); - CodeGenState* old_state = state_; - CodeGenState new_state(CodeGenState::LOAD, ref, true_target(), - false_target()); - state_ = &new_state; - Visit(ref->expression()); - state_ = old_state; -} - - void Property::GenerateStoreCode(MacroAssembler* masm, Scope* scope, Reference* ref, @@ -975,7 +1026,6 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, ASSERT(var()->mode() != Variable::DYNAMIC); Label exit; - bool may_skip_write = false; if (init_state == CONST_INIT) { ASSERT(var()->mode() == Variable::CONST); // Only the first const initialization must be executed (the slot @@ -985,7 +1035,6 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, masm->mov(eax, Ia32CodeGenerator::SlotOperand(masm, scope, this, ecx)); masm->cmp(eax, Factory::the_hole_value()); masm->j(not_equal, &exit); - may_skip_write = true; } // We must execute the store. @@ -1007,7 +1056,7 @@ void Slot::GenerateStoreCode(MacroAssembler* masm, } // If we definitely did not jump over the assignment, we do not need to // bind the exit label. Doing so can defeat peephole optimization. - if (may_skip_write) masm->bind(&exit); + if (init_state == CONST_INIT) masm->bind(&exit); } } -- 2.7.4