Revision: 7240
Author: [email protected]
Date: Thu Mar 17 07:36:40 2011
Log: [Arguments] Remove arguments shadow from scopes, stop rewriting
parameters.
Do not rewrite parameters in functions using the arguments object, access
them directly. They are allocated to the context in preparation for
capturing by the arguments object.
The arguments shadow object is removed from scopes, because there is no
longer any reason to preserve the original arguments object against
assignments to "arguments".
This change breaks the aliasing between parameters and arguments, and
temporarily makes the arguments object mirror behave the same in strict and
non-strict mode.
Review URL: http://codereview.chromium.org/6667076
http://code.google.com/p/v8/source/detail?r=7240
Modified:
/branches/experimental/arguments/src/arm/codegen-arm.cc
/branches/experimental/arguments/src/arm/full-codegen-arm.cc
/branches/experimental/arguments/src/ast.h
/branches/experimental/arguments/src/hydrogen.cc
/branches/experimental/arguments/src/ia32/codegen-ia32.cc
/branches/experimental/arguments/src/ia32/full-codegen-ia32.cc
/branches/experimental/arguments/src/scopes.cc
/branches/experimental/arguments/src/scopes.h
/branches/experimental/arguments/src/variables.h
/branches/experimental/arguments/src/x64/codegen-x64.cc
/branches/experimental/arguments/src/x64/full-codegen-x64.cc
=======================================
--- /branches/experimental/arguments/src/arm/codegen-arm.cc Tue Mar 15
14:56:12 2011
+++ /branches/experimental/arguments/src/arm/codegen-arm.cc Thu Mar 17
07:36:40 2011
@@ -578,8 +578,6 @@
ArgumentsAllocationMode CodeGenerator::ArgumentsMode() {
if (scope()->arguments() == NULL) return NO_ARGUMENTS_ALLOCATION;
- // In strict mode there is no need for shadow arguments.
- ASSERT(scope()->arguments_shadow() != NULL || scope()->is_strict_mode());
// We don't want to do lazy arguments allocation for functions that
// have heap-allocated contexts, because it interfers with the
// uninitialized const tracking in the context objects.
@@ -615,10 +613,7 @@
}
Variable* arguments = scope()->arguments();
- Variable* shadow = scope()->arguments_shadow();
ASSERT(arguments != NULL && arguments->AsSlot() != NULL);
- ASSERT((shadow != NULL && shadow->AsSlot() != NULL) ||
- scope()->is_strict_mode());
JumpTarget done;
if (mode == LAZY_ARGUMENTS_ALLOCATION && !initial) {
@@ -633,9 +628,6 @@
}
StoreToSlot(arguments->AsSlot(), NOT_CONST_INIT);
if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind();
- if (shadow != NULL) {
- StoreToSlot(shadow->AsSlot(), NOT_CONST_INIT);
- }
}
=======================================
--- /branches/experimental/arguments/src/arm/full-codegen-arm.cc Thu Mar 17
01:16:12 2011
+++ /branches/experimental/arguments/src/arm/full-codegen-arm.cc Thu Mar 17
07:36:40 2011
@@ -213,12 +213,6 @@
ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
__ CallStub(&stub);
- Variable* arguments_shadow = scope()->arguments_shadow();
- if (arguments_shadow != NULL) {
- // Duplicate the value; move-to-slot operation might clobber
registers.
- __ mov(r3, r0);
- Move(arguments_shadow->AsSlot(), r3, r1, r2);
- }
Move(arguments->AsSlot(), r0, r1, r2);
}
=======================================
--- /branches/experimental/arguments/src/ast.h Thu Mar 17 01:16:12 2011
+++ /branches/experimental/arguments/src/ast.h Thu Mar 17 07:36:40 2011
@@ -1079,16 +1079,7 @@
DECLARE_NODE_TYPE(VariableProxy)
// Type testing & conversion
- virtual Property* AsProperty() {
- return var_ == NULL ? NULL : var_->AsProperty();
- }
-
- Variable* AsVariable() {
- if (this == NULL || var_ == NULL) return NULL;
- Expression* rewrite = var_->rewrite();
- if (rewrite == NULL || rewrite->AsSlot() != NULL) return var_;
- return NULL;
- }
+ Variable* AsVariable() { return (this == NULL) ? NULL : var_; }
virtual bool IsValidLeftHandSide() {
return var_ == NULL ? true : var_->IsValidLeftHandSide();
=======================================
--- /branches/experimental/arguments/src/hydrogen.cc Thu Mar 17 02:55:57
2011
+++ /branches/experimental/arguments/src/hydrogen.cc Thu Mar 17 07:36:40
2011
@@ -2294,18 +2294,13 @@
// Handle the arguments and arguments shadow variables specially (they do
// not have declarations).
if (scope->arguments() != NULL) {
- if (!scope->arguments()->IsStackAllocated() ||
- (scope->arguments_shadow() != NULL &&
- !scope->arguments_shadow()->IsStackAllocated())) {
+ if (!scope->arguments()->IsStackAllocated()) {
BAILOUT("context-allocated arguments");
}
HArgumentsObject* object = new HArgumentsObject;
AddInstruction(object);
graph()->SetArgumentsObject(object);
environment()->Bind(scope->arguments(), object);
- if (scope->arguments_shadow() != NULL) {
- environment()->Bind(scope->arguments_shadow(), object);
- }
}
}
=======================================
--- /branches/experimental/arguments/src/ia32/codegen-ia32.cc Tue Mar 15
04:01:21 2011
+++ /branches/experimental/arguments/src/ia32/codegen-ia32.cc Thu Mar 17
07:36:40 2011
@@ -730,9 +730,6 @@
ArgumentsAllocationMode CodeGenerator::ArgumentsMode() {
if (scope()->arguments() == NULL) return NO_ARGUMENTS_ALLOCATION;
- // In strict mode there is no need for shadow arguments.
- ASSERT(scope()->arguments_shadow() != NULL || scope()->is_strict_mode());
-
// We don't want to do lazy arguments allocation for functions that
// have heap-allocated contexts, because it interfers with the
// uninitialized const tracking in the context objects.
@@ -762,11 +759,7 @@
}
Variable* arguments = scope()->arguments();
- Variable* shadow = scope()->arguments_shadow();
-
ASSERT(arguments != NULL && arguments->AsSlot() != NULL);
- ASSERT((shadow != NULL && shadow->AsSlot() != NULL) ||
- scope()->is_strict_mode());
JumpTarget done;
bool skip_arguments = false;
@@ -790,9 +783,6 @@
StoreToSlot(arguments->AsSlot(), NOT_CONST_INIT);
if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind();
}
- if (shadow != NULL) {
- StoreToSlot(shadow->AsSlot(), NOT_CONST_INIT);
- }
return frame_->Pop();
}
=======================================
--- /branches/experimental/arguments/src/ia32/full-codegen-ia32.cc Thu Mar
17 01:16:12 2011
+++ /branches/experimental/arguments/src/ia32/full-codegen-ia32.cc Thu Mar
17 07:36:40 2011
@@ -200,11 +200,6 @@
ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
__ CallStub(&stub);
- Variable* arguments_shadow = scope()->arguments_shadow();
- if (arguments_shadow != NULL) {
- __ mov(ecx, eax); // Duplicate result.
- Move(arguments_shadow->AsSlot(), ecx, ebx, edx);
- }
Move(arguments->AsSlot(), eax, ebx, edx);
}
=======================================
--- /branches/experimental/arguments/src/scopes.cc Wed Mar 9 08:57:03 2011
+++ /branches/experimental/arguments/src/scopes.cc Thu Mar 17 07:36:40 2011
@@ -155,24 +155,6 @@
}
AddInnerScope(inner_scope);
-
- // This scope's arguments shadow (if present) is context-allocated if an
inner
- // scope accesses this one's parameters. Allocate the arguments_shadow_
- // variable if necessary.
- Variable::Mode mode;
- int arguments_shadow_index =
- scope_info_->ContextSlotIndex(Heap::arguments_shadow_symbol(),
&mode);
- if (arguments_shadow_index >= 0) {
- ASSERT(mode == Variable::INTERNAL);
- arguments_shadow_ = new Variable(this,
- Factory::arguments_shadow_symbol(),
- Variable::INTERNAL,
- true,
- Variable::ARGUMENTS);
- arguments_shadow_->set_rewrite(
- new Slot(arguments_shadow_, Slot::CONTEXT,
arguments_shadow_index));
- arguments_shadow_->set_is_used(true);
- }
}
@@ -265,52 +247,33 @@
if (result != NULL || !resolved()) {
return result;
}
- // If the scope is resolved, we can find a variable in serialized scope
info.
-
- // We should never lookup 'arguments' in this scope
- // as it is implicitly present in any scope.
+ // If the scope is resolved, we can find a variable in serialized scope
+ // info.
+ //
+ // We should never lookup 'arguments' in this scope as it is implicitly
+ // present in every scope.
ASSERT(*name != *Factory::arguments_symbol());
-
- // Assert that there is no local slot with the given name.
+ // There should be no local slot with the given name.
ASSERT(scope_info_->StackSlotIndex(*name) < 0);
// Check context slot lookup.
Variable::Mode mode;
int index = scope_info_->ContextSlotIndex(*name, &mode);
- if (index >= 0) {
- Variable* var =
- variables_.Declare(this, name, mode, true, Variable::NORMAL);
- var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
- return var;
+ if (index < 0) {
+ // Check parameters.
+ mode = Variable::VAR;
+ index = scope_info_->ParameterIndex(*name);
+ if (index < 0) {
+ // Check the function name.
+ index = scope_info_->FunctionContextSlotIndex(*name);
+ if (index < 0) return NULL;
+ }
}
- index = scope_info_->ParameterIndex(*name);
- if (index >= 0) {
- // ".arguments" must be present in context slots.
- ASSERT(arguments_shadow_ != NULL);
- Variable* var =
- variables_.Declare(this, name, Variable::VAR, true,
Variable::NORMAL);
- Property* rewrite =
- new Property(new VariableProxy(arguments_shadow_),
- new Literal(Handle<Object>(Smi::FromInt(index))),
- RelocInfo::kNoPosition,
- Property::SYNTHETIC);
- rewrite->set_is_arguments_access(true);
- var->set_rewrite(rewrite);
- return var;
- }
-
- index = scope_info_->FunctionContextSlotIndex(*name);
- if (index >= 0) {
- // Check that there is no local slot with the given name.
- ASSERT(scope_info_->StackSlotIndex(*name) < 0);
- Variable* var =
- variables_.Declare(this, name, Variable::VAR, true,
Variable::NORMAL);
- var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
- return var;
- }
-
- return NULL;
+ Variable* var =
+ variables_.Declare(this, name, mode, true, Variable::NORMAL);
+ var->set_rewrite(new Slot(var, Slot::CONTEXT, index));
+ return var;
}
@@ -890,36 +853,17 @@
Variable* arguments = LocalLookup(Factory::arguments_symbol());
ASSERT(arguments != NULL); // functions have 'arguments' declared
implicitly
- // Parameters are rewritten to arguments[i] if 'arguments' is used in
- // a non-strict mode function. Strict mode code doesn't alias arguments.
- bool rewrite_parameters = false;
+ bool uses_nonstrict_arguments = false;
if (MustAllocate(arguments) && !HasArgumentsParameter()) {
// 'arguments' is used. Unless there is also a parameter called
- // 'arguments', we must be conservative and access all parameters via
- // the arguments object: The i'th parameter is rewritten into
- // '.arguments[i]' (*). If we have a parameter named 'arguments', a
- // (new) value is always assigned to it via the function
- // invocation. Then 'arguments' denotes that specific parameter value
- // and cannot be used to access the parameters, which is why we don't
- // need to rewrite in that case.
- //
- // (*) Instead of having a parameter called 'arguments', we may have an
- // assignment to 'arguments' in the function body, at some arbitrary
- // point in time (possibly through an 'eval()' call!). After that
- // assignment any re-write of parameters would be invalid (was bug
- // 881452). Thus, we introduce a shadow '.arguments'
- // variable which also points to the arguments object. For rewrites we
- // use '.arguments' which remains valid even if we assign to
- // 'arguments'. To summarize: If we need to rewrite, we allocate an
- // 'arguments' object dynamically upon function invocation. The
compiler
- // introduces 2 local variables 'arguments' and '.arguments', both of
- // which originally point to the arguments object that was
- // allocated. All parameters are rewritten into property accesses via
- // the '.arguments' variable. Thus, any changes to properties of
- // 'arguments' are reflected in the variables and vice versa. If the
- // 'arguments' variable is changed, '.arguments' still points to the
- // correct arguments object and the rewrites still work.
+ // 'arguments', we must be conservative and allocate all parameters to
+ // the context assuming they will be captured by the arguments object.
+ // If we have a parameter named 'arguments', a (new) value is always
+ // assigned to it via the function invocation. Then 'arguments' denotes
+ // that specific parameter value and cannot be used to access the
+ // parameters, which is why we don't need to allocate an arguments
+ // object in that case.
// We are using 'arguments'. Tell the code generator that is needs to
// allocate the arguments object by setting 'arguments_'.
@@ -928,75 +872,31 @@
// In strict mode 'arguments' does not alias formal parameters.
// Therefore in strict mode we allocate parameters as if 'arguments'
// were not used.
- rewrite_parameters = !is_strict_mode();
+ uses_nonstrict_arguments = !is_strict_mode();
}
- if (rewrite_parameters) {
- // We also need the '.arguments' shadow variable. Declare it and create
- // and bind the corresponding proxy. It's ok to declare it only now
- // because it's a local variable that is allocated after the parameters
- // have been allocated.
- //
- // Note: This is "almost" at temporary variable but we cannot use
- // NewTemporary() because the mode needs to be INTERNAL since this
- // variable may be allocated in the heap-allocated context (temporaries
- // are never allocated in the context).
- arguments_shadow_ = new Variable(this,
- Factory::arguments_shadow_symbol(),
- Variable::INTERNAL,
- true,
- Variable::ARGUMENTS);
- arguments_shadow_->set_is_used(true);
- temps_.Add(arguments_shadow_);
-
- // Allocate the parameters by rewriting them into '.arguments[i]'
accesses.
- for (int i = 0; i < params_.length(); i++) {
- Variable* var = params_[i];
- ASSERT(var->scope() == this);
- if (MustAllocate(var)) {
- if (MustAllocateInContext(var)) {
- // It is ok to set this only now, because arguments is a local
- // variable that is allocated after the parameters have been
- // allocated.
- arguments_shadow_->MarkAsAccessedFromInnerScope();
- }
- Property* rewrite =
- new Property(new VariableProxy(arguments_shadow_),
- new Literal(Handle<Object>(Smi::FromInt(i))),
- RelocInfo::kNoPosition,
- Property::SYNTHETIC);
- rewrite->set_is_arguments_access(true);
- var->set_rewrite(rewrite);
- }
+ // The same parameter may occur multiple times in the parameters_ list.
+ // If it does, and if it is not copied into the context object, it must
+ // receive the highest parameter index for that parameter; thus iteration
+ // order is relevant!
+ for (int i = params_.length() - 1; i >= 0; --i) {
+ Variable* var = params_[i];
+ ASSERT(var->scope() == this);
+ if (uses_nonstrict_arguments) {
+ // Give the parameter a use from an inner scope, to force allocation
+ // to the context.
+ var->MarkAsAccessedFromInnerScope();
}
- } else {
- // The arguments object is not used, so we can access parameters
directly.
- // The same parameter may occur multiple times in the parameters_ list.
- // If it does, and if it is not copied into the context object, it must
- // receive the highest parameter index for that parameter; thus
iteration
- // order is relevant!
- for (int i = 0; i < params_.length(); i++) {
- Variable* var = params_[i];
- ASSERT(var->scope() == this);
- if (MustAllocate(var)) {
- if (MustAllocateInContext(var)) {
- ASSERT(var->rewrite() == NULL ||
- (var->AsSlot() != NULL &&
- var->AsSlot()->type() == Slot::CONTEXT));
- if (var->rewrite() == NULL) {
- // Only set the heap allocation if the parameter has not
- // been allocated yet.
- AllocateHeapSlot(var);
- }
- } else {
- ASSERT(var->rewrite() == NULL ||
- (var->AsSlot() != NULL &&
- var->AsSlot()->type() == Slot::PARAMETER));
- // Set the parameter index always, even if the parameter
- // was seen before! (We need to access the actual parameter
- // supplied for the last occurrence of a multiply declared
- // parameter.)
+ if (MustAllocate(var)) {
+ if (MustAllocateInContext(var)) {
+ ASSERT(var->rewrite() == NULL || var->IsContextSlot());
+ if (var->rewrite() == NULL) {
+ AllocateHeapSlot(var);
+ }
+ } else {
+ ASSERT(var->rewrite() == NULL || var->IsParameter());
+ if (var->rewrite() == NULL) {
var->set_rewrite(new Slot(var, Slot::PARAMETER, i));
}
}
=======================================
--- /branches/experimental/arguments/src/scopes.h Wed Mar 9 08:57:03 2011
+++ /branches/experimental/arguments/src/scopes.h Thu Mar 17 07:36:40 2011
@@ -253,10 +253,6 @@
// The local variable 'arguments' if we need to allocate it; NULL
otherwise.
// If arguments() exist, arguments_shadow() exists, too.
Variable* arguments() const { return arguments_; }
-
- // The '.arguments' shadow variable if we need to allocate it; NULL
otherwise.
- // If arguments_shadow() exist, arguments() exists, too.
- Variable* arguments_shadow() const { return arguments_shadow_; }
// Declarations list.
ZoneList<Declaration*>* declarations() { return &decls_; }
@@ -353,8 +349,6 @@
Variable* function_;
// Convenience variable; function scopes only.
Variable* arguments_;
- // Convenience variable; function scopes only.
- Variable* arguments_shadow_;
// Illegal redeclaration.
Expression* illegal_redecl_;
@@ -431,7 +425,6 @@
receiver_ = NULL;
function_ = NULL;
arguments_ = NULL;
- arguments_shadow_ = NULL;
illegal_redecl_ = NULL;
scope_inside_with_ = false;
scope_contains_with_ = false;
=======================================
--- /branches/experimental/arguments/src/variables.h Wed Jan 26 10:15:43
2011
+++ /branches/experimental/arguments/src/variables.h Thu Mar 17 07:36:40
2011
@@ -178,8 +178,8 @@
local_if_not_shadowed_ = local;
}
- Expression* rewrite() const { return rewrite_; }
- void set_rewrite(Expression* expr) { rewrite_ = expr; }
+ Slot* rewrite() const { return rewrite_; }
+ void set_rewrite(Slot* expr) { rewrite_ = expr; }
StaticType* type() { return &type_; }
@@ -195,8 +195,7 @@
StaticType type_;
// Code generation.
- // rewrite_ is usually a Slot or a Property, but may be any expression.
- Expression* rewrite_;
+ Slot* rewrite_;
// Valid as a LHS? (const and this are not valid LHS, for example)
bool is_valid_LHS_;
=======================================
--- /branches/experimental/arguments/src/x64/codegen-x64.cc Tue Mar 15
04:01:21 2011
+++ /branches/experimental/arguments/src/x64/codegen-x64.cc Thu Mar 17
07:36:40 2011
@@ -612,8 +612,6 @@
ArgumentsAllocationMode CodeGenerator::ArgumentsMode() {
if (scope()->arguments() == NULL) return NO_ARGUMENTS_ALLOCATION;
- // In strict mode there is no need for shadow arguments.
- ASSERT(scope()->arguments_shadow() != NULL || scope()->is_strict_mode());
// We don't want to do lazy arguments allocation for functions that
// have heap-allocated contexts, because it interfers with the
// uninitialized const tracking in the context objects.
@@ -643,10 +641,7 @@
}
Variable* arguments = scope()->arguments();
- Variable* shadow = scope()->arguments_shadow();
ASSERT(arguments != NULL && arguments->AsSlot() != NULL);
- ASSERT((shadow != NULL && shadow->AsSlot() != NULL) ||
- scope()->is_strict_mode());
JumpTarget done;
bool skip_arguments = false;
@@ -670,9 +665,6 @@
StoreToSlot(arguments->AsSlot(), NOT_CONST_INIT);
if (mode == LAZY_ARGUMENTS_ALLOCATION) done.Bind();
}
- if (shadow != NULL) {
- StoreToSlot(shadow->AsSlot(), NOT_CONST_INIT);
- }
return frame_->Pop();
}
=======================================
--- /branches/experimental/arguments/src/x64/full-codegen-x64.cc Thu Mar 17
01:16:12 2011
+++ /branches/experimental/arguments/src/x64/full-codegen-x64.cc Thu Mar 17
07:36:40 2011
@@ -201,12 +201,6 @@
ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
__ CallStub(&stub);
- Variable* arguments_shadow = scope()->arguments_shadow();
- if (arguments_shadow != NULL) {
- // Store new arguments object in both "arguments" and ".arguments"
slots.
- __ movq(rcx, rax);
- Move(arguments_shadow->AsSlot(), rcx, rbx, rdx);
- }
Move(arguments->AsSlot(), rax, rbx, rdx);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
