From ede65c19a1a5d69957ab7ac996ac7fa055acff68 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Wed, 13 Jan 2010 16:01:15 +0000 Subject: [PATCH] Remove a pair of problematic uses of the Reference utility class from the code generators. These uses broke the rules of the class because it was safe to do so, but there was no real reason to do it that way. Review URL: http://codereview.chromium.org/543041 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3598 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 46 ++++++++++++++++------------------ src/ast.h | 6 ++--- src/ia32/codegen-ia32.cc | 63 +++++++++++++++++++++-------------------------- src/x64/codegen-x64.cc | 64 ++++++++++++++++++++++-------------------------- 4 files changed, 81 insertions(+), 98 deletions(-) diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 2aa5a2d..70d8ab4 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -247,25 +247,25 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) { // initialization because the arguments object may be stored in the // context. if (scope_->arguments() != NULL) { - ASSERT(scope_->arguments_shadow() != NULL); Comment cmnt(masm_, "[ allocate arguments object"); - { Reference shadow_ref(this, scope_->arguments_shadow()); - { Reference arguments_ref(this, scope_->arguments()); - ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); - __ ldr(r2, frame_->Function()); - // The receiver is below the arguments, the return address, - // and the frame pointer on the stack. - const int kReceiverDisplacement = 2 + scope_->num_parameters(); - __ add(r1, fp, Operand(kReceiverDisplacement * kPointerSize)); - __ mov(r0, Operand(Smi::FromInt(scope_->num_parameters()))); - frame_->Adjust(3); - __ stm(db_w, sp, r0.bit() | r1.bit() | r2.bit()); - frame_->CallStub(&stub, 3); - frame_->EmitPush(r0); - arguments_ref.SetValue(NOT_CONST_INIT); - } - shadow_ref.SetValue(NOT_CONST_INIT); - } + ASSERT(scope_->arguments_shadow() != NULL); + Variable* arguments = scope_->arguments()->var(); + Variable* shadow = scope_->arguments_shadow()->var(); + ASSERT(arguments != NULL && arguments->slot() != NULL); + ASSERT(shadow != NULL && shadow->slot() != NULL); + ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT); + __ ldr(r2, frame_->Function()); + // The receiver is below the arguments, the return address, and the + // frame pointer on the stack. + const int kReceiverDisplacement = 2 + scope_->num_parameters(); + __ add(r1, fp, Operand(kReceiverDisplacement * kPointerSize)); + __ mov(r0, Operand(Smi::FromInt(scope_->num_parameters()))); + frame_->Adjust(3); + __ stm(db_w, sp, r0.bit() | r1.bit() | r2.bit()); + frame_->CallStub(&stub, 3); + frame_->EmitPush(r0); + StoreToSlot(arguments->slot(), NOT_CONST_INIT); + StoreToSlot(shadow->slot(), NOT_CONST_INIT); frame_->Drop(); // Value is no longer needed. } @@ -1992,13 +1992,9 @@ void CodeGenerator::VisitTryCatchStatement(TryCatchStatement* node) { frame_->EmitPush(r0); // Store the caught exception in the catch variable. - { Reference ref(this, node->catch_var()); - ASSERT(ref.is_slot()); - // Here we make use of the convenient property that it doesn't matter - // whether a value is immediately on top of or underneath a zero-sized - // reference. - ref.SetValue(NOT_CONST_INIT); - } + Variable* catch_var = node->catch_var()->var(); + ASSERT(catch_var != NULL && catch_var->slot() != NULL); + StoreToSlot(catch_var->slot(), NOT_CONST_INIT); // Remove the exception from the stack. frame_->Drop(); diff --git a/src/ast.h b/src/ast.h index ec32318..2498970 100644 --- a/src/ast.h +++ b/src/ast.h @@ -647,7 +647,7 @@ class TryStatement: public Statement { class TryCatchStatement: public TryStatement { public: TryCatchStatement(Block* try_block, - Expression* catch_var, + VariableProxy* catch_var, Block* catch_block) : TryStatement(try_block), catch_var_(catch_var), @@ -657,11 +657,11 @@ class TryCatchStatement: public TryStatement { virtual void Accept(AstVisitor* v); - Expression* catch_var() const { return catch_var_; } + VariableProxy* catch_var() const { return catch_var_; } Block* catch_block() const { return catch_block_; } private: - Expression* catch_var_; + VariableProxy* catch_var_; Block* catch_block_; }; diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 49b32c2..993675b 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -609,36 +609,33 @@ Result CodeGenerator::StoreArgumentsObject(bool initial) { frame_->Push(&result); } - { Reference shadow_ref(this, scope_->arguments_shadow()); - Reference arguments_ref(this, scope_->arguments()); - ASSERT(shadow_ref.is_slot() && arguments_ref.is_slot()); - // Here we rely on the convenient property that references to slot - // take up zero space in the frame (ie, it doesn't matter that the - // stored value is actually below the reference on the frame). - JumpTarget done; - bool skip_arguments = false; - if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) { - // We have to skip storing into the arguments slot if it has - // already been written to. This can happen if the a function - // has a local variable named 'arguments'. - LoadFromSlot(scope_->arguments()->var()->slot(), NOT_INSIDE_TYPEOF); - Result arguments = frame_->Pop(); - if (arguments.is_constant()) { - // We have to skip updating the arguments object if it has - // been assigned a proper value. - skip_arguments = !arguments.handle()->IsTheHole(); - } else { - __ cmp(Operand(arguments.reg()), Immediate(Factory::the_hole_value())); - arguments.Unuse(); - done.Branch(not_equal); - } - } - if (!skip_arguments) { - arguments_ref.SetValue(NOT_CONST_INIT); - if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind(); + Variable* arguments = scope_->arguments()->var(); + Variable* shadow = scope_->arguments_shadow()->var(); + ASSERT(arguments != NULL && arguments->slot() != NULL); + ASSERT(shadow != NULL && shadow->slot() != NULL); + JumpTarget done; + bool skip_arguments = false; + if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) { + // We have to skip storing into the arguments slot if it has already + // been written to. This can happen if the a function has a local + // variable named 'arguments'. + LoadFromSlot(arguments->slot(), NOT_INSIDE_TYPEOF); + Result probe = frame_->Pop(); + if (probe.is_constant()) { + // We have to skip updating the arguments object if it has + // been assigned a proper value. + skip_arguments = !probe.handle()->IsTheHole(); + } else { + __ cmp(Operand(probe.reg()), Immediate(Factory::the_hole_value())); + probe.Unuse(); + done.Branch(not_equal); } - shadow_ref.SetValue(NOT_CONST_INIT); } + if (!skip_arguments) { + StoreToSlot(arguments->slot(), NOT_CONST_INIT); + if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind(); + } + StoreToSlot(shadow->slot(), NOT_CONST_INIT); return frame_->Pop(); } @@ -3581,13 +3578,9 @@ void CodeGenerator::VisitTryCatchStatement(TryCatchStatement* node) { frame_->EmitPush(eax); // Store the caught exception in the catch variable. - { Reference ref(this, node->catch_var()); - ASSERT(ref.is_slot()); - // Load the exception to the top of the stack. Here we make use of the - // convenient property that it doesn't matter whether a value is - // immediately on top of or underneath a zero-sized reference. - ref.SetValue(NOT_CONST_INIT); - } + Variable* catch_var = node->catch_var()->var(); + ASSERT(catch_var != NULL && catch_var->slot() != NULL); + StoreToSlot(catch_var->slot(), NOT_CONST_INIT); // Remove the exception from the stack. frame_->Drop(); diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 13c782d..e912bbc 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -1878,13 +1878,9 @@ void CodeGenerator::VisitTryCatchStatement(TryCatchStatement* node) { frame_->EmitPush(rax); // Store the caught exception in the catch variable. - { Reference ref(this, node->catch_var()); - ASSERT(ref.is_slot()); - // Load the exception to the top of the stack. Here we make use of the - // convenient property that it doesn't matter whether a value is - // immediately on top of or underneath a zero-sized reference. - ref.SetValue(NOT_CONST_INIT); - } + Variable* catch_var = node->catch_var()->var(); + ASSERT(catch_var != NULL && catch_var->slot() != NULL); + StoreToSlot(catch_var->slot(), NOT_CONST_INIT); // Remove the exception from the stack. frame_->Drop(); @@ -4745,36 +4741,34 @@ Result CodeGenerator::StoreArgumentsObject(bool initial) { frame_->Push(&result); } - { Reference shadow_ref(this, scope_->arguments_shadow()); - Reference arguments_ref(this, scope_->arguments()); - ASSERT(shadow_ref.is_slot() && arguments_ref.is_slot()); - // Here we rely on the convenient property that references to slot - // take up zero space in the frame (ie, it doesn't matter that the - // stored value is actually below the reference on the frame). - JumpTarget done; - bool skip_arguments = false; - if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) { - // We have to skip storing into the arguments slot if it has - // already been written to. This can happen if the a function - // has a local variable named 'arguments'. - LoadFromSlot(scope_->arguments()->var()->slot(), NOT_INSIDE_TYPEOF); - Result arguments = frame_->Pop(); - if (arguments.is_constant()) { - // We have to skip updating the arguments object if it has - // been assigned a proper value. - skip_arguments = !arguments.handle()->IsTheHole(); - } else { - __ CompareRoot(arguments.reg(), Heap::kTheHoleValueRootIndex); - arguments.Unuse(); - done.Branch(not_equal); - } - } - if (!skip_arguments) { - arguments_ref.SetValue(NOT_CONST_INIT); - if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind(); + + Variable* arguments = scope_->arguments()->var(); + Variable* shadow = scope_->arguments_shadow()->var(); + ASSERT(arguments != NULL && arguments->slot() != NULL); + ASSERT(shadow != NULL && shadow->slot() != NULL); + JumpTarget done; + bool skip_arguments = false; + if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) { + // We have to skip storing into the arguments slot if it has + // already been written to. This can happen if the a function + // has a local variable named 'arguments'. + LoadFromSlot(scope_->arguments()->var()->slot(), NOT_INSIDE_TYPEOF); + Result probe = frame_->Pop(); + if (probe.is_constant()) { + // We have to skip updating the arguments object if it has been + // assigned a proper value. + skip_arguments = !probe.handle()->IsTheHole(); + } else { + __ CompareRoot(probe.reg(), Heap::kTheHoleValueRootIndex); + probe.Unuse(); + done.Branch(not_equal); } - shadow_ref.SetValue(NOT_CONST_INIT); } + if (!skip_arguments) { + StoreToSlot(arguments->slot(), NOT_CONST_INIT); + if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind(); + } + StoreToSlot(shadow->slot(), NOT_CONST_INIT); return frame_->Pop(); } -- 2.7.4