Revision: 8452
Author: [email protected]
Date: Tue Jun 28 08:22:08 2011
Log: Remove the fcontext field from all contexts.
Before: every context cached the nearest enclosing function context. This
assumed that for nested contexts (i.e., with and catch contexts) the
enclosing function had a materialized link in the context chain.
Now: when necessary, we loop up the context chain to find such a context.
This enables catch contexts without forcing the enclosing function to
allocate its own context.
[email protected]
BUG=
TEST=
Review URL: http://codereview.chromium.org/7230047
http://code.google.com/p/v8/source/detail?r=8452
Modified:
/branches/bleeding_edge/src/arm/code-stubs-arm.cc
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/arm/macro-assembler-arm.cc
/branches/bleeding_edge/src/bootstrapper.cc
/branches/bleeding_edge/src/contexts.cc
/branches/bleeding_edge/src/contexts.h
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/ia32/code-stubs-ia32.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/x64/code-stubs-x64.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/src/x64/macro-assembler-x64.cc
=======================================
--- /branches/bleeding_edge/src/arm/code-stubs-arm.cc Thu Jun 23 05:03:16
2011
+++ /branches/bleeding_edge/src/arm/code-stubs-arm.cc Tue Jun 28 08:22:08
2011
@@ -165,7 +165,6 @@
// Setup the fixed slots.
__ mov(r1, Operand(Smi::FromInt(0)));
__ str(r3, MemOperand(r0, Context::SlotOffset(Context::CLOSURE_INDEX)));
- __ str(r0, MemOperand(r0, Context::SlotOffset(Context::FCONTEXT_INDEX)));
__ str(cp, MemOperand(r0, Context::SlotOffset(Context::PREVIOUS_INDEX)));
__ str(r1, MemOperand(r0,
Context::SlotOffset(Context::EXTENSION_INDEX)));
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Jun 24 07:30:10
2011
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Tue Jun 28 08:22:08
2011
@@ -713,10 +713,12 @@
// context.
ASSERT_EQ(0, scope()->ContextChainLength(variable->scope()));
if (FLAG_debug_code) {
- // Check that we're not inside a 'with'.
- __ ldr(r1, ContextOperand(cp, Context::FCONTEXT_INDEX));
- __ cmp(r1, cp);
- __ Check(eq, "Unexpected declaration in current context.");
+ // Check that we're not inside a with or catch context.
+ __ ldr(r1, FieldMemOperand(cp, HeapObject::kMapOffset));
+ __ CompareRoot(r1, Heap::kWithContextMapRootIndex);
+ __ Check(ne, "Declaration in with context.");
+ __ CompareRoot(r1, Heap::kCatchContextMapRootIndex);
+ __ Check(ne, "Declaration in catch context.");
}
if (mode == Variable::CONST) {
__ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
@@ -1867,18 +1869,7 @@
__ b(ne, &skip);
__ str(result_register(), MemOperand(fp, SlotOffset(slot)));
break;
- case Slot::CONTEXT: {
- __ ldr(r1, ContextOperand(cp, Context::FCONTEXT_INDEX));
- __ ldr(r2, ContextOperand(r1, slot->index()));
- __ LoadRoot(ip, Heap::kTheHoleValueRootIndex);
- __ cmp(r2, ip);
- __ b(ne, &skip);
- __ str(r0, ContextOperand(r1, slot->index()));
- int offset = Context::SlotOffset(slot->index());
- __ mov(r3, r0); // Preserve the stored value in r0.
- __ RecordWrite(r1, Operand(offset), r3, r2);
- break;
- }
+ case Slot::CONTEXT:
case Slot::LOOKUP:
__ push(r0);
__ mov(r0, Operand(slot->var()->name()));
=======================================
--- /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Thu Jun 23
05:03:16 2011
+++ /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Tue Jun 28
08:22:08 2011
@@ -2562,17 +2562,6 @@
// cannot be allowed to destroy the context in esi).
mov(dst, cp);
}
-
- // We should not have found a 'with' context by walking the context chain
- // (i.e., the static scope chain and runtime context chain do not agree).
- // A variable occurring in such a scope should have slot type LOOKUP and
- // not CONTEXT.
- if (emit_debug_code()) {
- ldr(ip, MemOperand(dst, Context::SlotOffset(Context::FCONTEXT_INDEX)));
- cmp(dst, ip);
- Check(eq, "Yo dawg, I heard you liked function contexts "
- "so I put function contexts in all your contexts");
- }
}
=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Thu Jun 16 07:12:58 2011
+++ /branches/bleeding_edge/src/bootstrapper.cc Tue Jun 28 08:22:08 2011
@@ -814,7 +814,6 @@
// --- G l o b a l C o n t e x t ---
// Use the empty function as closure (no scope info).
global_context()->set_closure(*empty_function);
- global_context()->set_fcontext(*global_context());
global_context()->set_previous(NULL);
// Set extension and global object.
global_context()->set_extension(*inner_global);
=======================================
--- /branches/bleeding_edge/src/contexts.cc Thu Jun 16 07:12:58 2011
+++ /branches/bleeding_edge/src/contexts.cc Tue Jun 28 08:22:08 2011
@@ -34,6 +34,16 @@
namespace v8 {
namespace internal {
+Context* Context::declaration_context() {
+ Context* current = this;
+ while (!current->IsFunctionContext() && !current->IsGlobalContext()) {
+ current = current->previous();
+ ASSERT(current->closure() == closure());
+ }
+ return current;
+}
+
+
JSBuiltinsObject* Context::builtins() {
GlobalObject* object = global();
if (object->IsJSGlobalObject()) {
@@ -246,20 +256,22 @@
bool*
outer_scope_calls_non_strict_eval) {
// Skip up the context chain checking all the function contexts to see
// whether they call eval.
- Context* context = fcontext();
+ Context* context = this;
while (!context->IsGlobalContext()) {
- Handle<SerializedScopeInfo> scope_info(
- context->closure()->shared()->scope_info());
- if (scope_info->CallsEval()) {
- *outer_scope_calls_eval = true;
- if (!scope_info->IsStrictMode()) {
- // No need to go further since the answers will not change
- // from here.
- *outer_scope_calls_non_strict_eval = true;
- return;
+ if (context->IsFunctionContext()) {
+ Handle<SerializedScopeInfo> scope_info(
+ context->closure()->shared()->scope_info());
+ if (scope_info->CallsEval()) {
+ *outer_scope_calls_eval = true;
+ if (!scope_info->IsStrictMode()) {
+ // No need to go further since the answers will not change from
+ // here.
+ *outer_scope_calls_non_strict_eval = true;
+ return;
+ }
}
}
- context = context->previous()->fcontext();
+ context = context->previous();
}
}
=======================================
--- /branches/bleeding_edge/src/contexts.h Thu Jun 16 07:12:58 2011
+++ /branches/bleeding_edge/src/contexts.h Tue Jun 28 08:22:08 2011
@@ -130,13 +130,6 @@
// statically allocated context slots. The names are needed
// for dynamic lookups in the presence of 'with' or 'eval'.
//
-// [ fcontext ] A pointer to the innermost enclosing function context.
-// It is the same for all contexts *allocated* inside a
-// function, and the function context's fcontext points
-// to itself. It is only needed for fast access of the
-// function context (used for declarations, and static
-// context slot access).
-//
// [ previous ] A pointer to the previous context. It is NULL for
// function contexts, and non-NULL for 'with' contexts.
// Used to implement the 'with' statement.
@@ -158,16 +151,6 @@
// (via static context addresses) or through 'eval' (dynamic context
lookups).
// Finally, the global context contains additional slots for fast access to
// global properties.
-//
-// We may be able to simplify the implementation:
-//
-// - We may be able to get rid of 'fcontext': We can always use the fact
that
-// previous == NULL for function contexts and so we can search for them.
They
-// are only needed when doing dynamic declarations, and the context
chains
-// tend to be very very short (depth of nesting of 'with' statements). At
-// the moment we also use it in generated code for context slot accesses
-
-// and there we don't want a loop because of code bloat - but we may not
-// need it there after all (see comment in codegen_*.cc).
class Context: public FixedArray {
public:
@@ -181,7 +164,6 @@
enum {
// These slots are in all contexts.
CLOSURE_INDEX,
- FCONTEXT_INDEX,
PREVIOUS_INDEX,
// The extension slot is used for either the global object (in global
// contexts), eval extension object (function contexts), subject of
with
@@ -263,9 +245,6 @@
// Direct slot access.
JSFunction* closure() { return JSFunction::cast(get(CLOSURE_INDEX)); }
void set_closure(JSFunction* closure) { set(CLOSURE_INDEX, closure); }
-
- Context* fcontext() { return Context::cast(get(FCONTEXT_INDEX)); }
- void set_fcontext(Context* context) { set(FCONTEXT_INDEX, context); }
Context* previous() {
Object* result = unchecked_previous();
@@ -277,6 +256,10 @@
bool has_extension() { return extension() != NULL; }
Object* extension() { return get(EXTENSION_INDEX); }
void set_extension(Object* object) { set(EXTENSION_INDEX, object); }
+
+ // Get the context where var declarations will be hoisted to, which
+ // may be the context itself.
+ Context* declaration_context();
GlobalObject* global() {
Object* result = get(GLOBAL_INDEX);
=======================================
--- /branches/bleeding_edge/src/heap.cc Fri Jun 24 06:44:27 2011
+++ /branches/bleeding_edge/src/heap.cc Tue Jun 28 08:22:08 2011
@@ -3932,7 +3932,6 @@
Context* context = reinterpret_cast<Context*>(result);
context->set_map(function_context_map());
context->set_closure(function);
- context->set_fcontext(context);
context->set_previous(function->context());
context->set_extension(NULL);
context->set_global(function->context()->global());
@@ -3952,7 +3951,6 @@
Context* context = reinterpret_cast<Context*>(result);
context->set_map(catch_context_map());
context->set_closure(previous->closure());
- context->set_fcontext(previous->fcontext());
context->set_previous(previous);
context->set_extension(name);
context->set_global(previous->global());
@@ -3970,7 +3968,6 @@
Context* context = reinterpret_cast<Context*>(result);
context->set_map(with_context_map());
context->set_closure(previous->closure());
- context->set_fcontext(previous->fcontext());
context->set_previous(previous);
context->set_extension(extension);
context->set_global(previous->global());
=======================================
--- /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Wed Jun 22 01:28:35
2011
+++ /branches/bleeding_edge/src/ia32/code-stubs-ia32.cc Tue Jun 28 08:22:08
2011
@@ -136,7 +136,6 @@
// Setup the fixed slots.
__ Set(ebx, Immediate(0)); // Set to NULL.
__ mov(Operand(eax, Context::SlotOffset(Context::CLOSURE_INDEX)), ecx);
- __ mov(Operand(eax, Context::SlotOffset(Context::FCONTEXT_INDEX)), eax);
__ mov(Operand(eax, Context::SlotOffset(Context::PREVIOUS_INDEX)), esi);
__ mov(Operand(eax, Context::SlotOffset(Context::EXTENSION_INDEX)), ebx);
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Jun 24
07:30:10 2011
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Tue Jun 28
08:22:08 2011
@@ -687,10 +687,12 @@
// context.
ASSERT_EQ(0, scope()->ContextChainLength(variable->scope()));
if (FLAG_debug_code) {
- // Check that we're not inside a 'with'.
- __ mov(ebx, ContextOperand(esi, Context::FCONTEXT_INDEX));
- __ cmp(ebx, Operand(esi));
- __ Check(equal, "Unexpected declaration in current context.");
+ // Check that we're not inside a with or catch context.
+ __ mov(ebx, FieldOperand(esi, HeapObject::kMapOffset));
+ __ cmp(ebx, isolate()->factory()->with_context_map());
+ __ Check(not_equal, "Declaration in with context.");
+ __ cmp(ebx, isolate()->factory()->catch_context_map());
+ __ Check(not_equal, "Declaration in catch context.");
}
if (mode == Variable::CONST) {
__ mov(ContextOperand(esi, slot->index()),
@@ -1818,17 +1820,7 @@
__ j(not_equal, &skip);
__ mov(Operand(ebp, SlotOffset(slot)), eax);
break;
- case Slot::CONTEXT: {
- __ mov(ecx, ContextOperand(esi, Context::FCONTEXT_INDEX));
- __ mov(edx, ContextOperand(ecx, slot->index()));
- __ cmp(edx, isolate()->factory()->the_hole_value());
- __ j(not_equal, &skip);
- __ mov(ContextOperand(ecx, slot->index()), eax);
- int offset = Context::SlotOffset(slot->index());
- __ mov(edx, eax); // Preserve the stored value in eax.
- __ RecordWrite(ecx, offset, edx, ebx);
- break;
- }
+ case Slot::CONTEXT:
case Slot::LOOKUP:
__ push(eax);
__ push(esi);
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Fri Jun 17
11:32:36 2011
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Tue Jun 28
08:22:08 2011
@@ -1766,14 +1766,17 @@
mov(dst, esi);
}
- // We should not have found a 'with' context by walking the context chain
- // (i.e., the static scope chain and runtime context chain do not agree).
- // A variable occurring in such a scope should have slot type LOOKUP and
- // not CONTEXT.
+ // We should not have found a with or catch context by walking the
context
+ // chain (i.e., the static scope chain and runtime context chain do not
+ // agree). A variable occurring in such a scope should have slot type
+ // LOOKUP and not CONTEXT.
if (emit_debug_code()) {
- cmp(dst, Operand(dst, Context::SlotOffset(Context::FCONTEXT_INDEX)));
- Check(equal, "Yo dawg, I heard you liked function contexts "
- "so I put function contexts in all your contexts");
+ cmp(FieldOperand(dst, HeapObject::kMapOffset),
+ isolate()->factory()->with_context_map());
+ Check(not_equal, "Variable resolved to with context.");
+ cmp(FieldOperand(dst, HeapObject::kMapOffset),
+ isolate()->factory()->with_context_map());
+ Check(not_equal, "Variable resolved to catch context.");
}
}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Mon Jun 27 02:02:34 2011
+++ /branches/bleeding_edge/src/runtime.cc Tue Jun 28 08:22:08 2011
@@ -1236,9 +1236,8 @@
RUNTIME_ASSERT(mode == READ_ONLY || mode == NONE);
Handle<Object> initial_value(args[3], isolate);
- // Declarations are always done in the function context.
- context = Handle<Context>(context->fcontext());
- ASSERT(context->IsFunctionContext() || context->IsGlobalContext());
+ // Declarations are always done in a function or global context.
+ context = Handle<Context>(context->declaration_context());
int index;
PropertyAttributes attributes;
@@ -1525,8 +1524,8 @@
CONVERT_ARG_CHECKED(Context, context, 1);
Handle<String> name(String::cast(args[2]));
- // Initializations are always done in the function context.
- context = Handle<Context>(context->fcontext());
+ // Initializations are always done in a function or global context.
+ context = Handle<Context>(context->declaration_context());
int index;
PropertyAttributes attributes;
@@ -1547,14 +1546,12 @@
// In that case, the initialization behaves like a normal assignment
// to property 'x'.
if (index >= 0) {
- // Property was found in a context.
if (holder->IsContext()) {
- // The holder cannot be the function context. If it is, there
- // should have been a const redeclaration error when declaring
- // the const property.
- ASSERT(!holder.is_identical_to(context));
- if ((attributes & READ_ONLY) == 0) {
- Handle<Context>::cast(holder)->set(index, *value);
+ // Property was found in a context. Perform the assignment if we
+ // found some non-constant or an uninitialized constant.
+ Handle<Context> context = Handle<Context>::cast(holder);
+ if ((attributes & READ_ONLY) == 0 ||
context->get(index)->IsTheHole()) {
+ context->set(index, *value);
}
} else {
// The holder is an arguments object.
@@ -9986,9 +9983,6 @@
Handle<SerializedScopeInfo> scope_info(function->shared()->scope_info());
ScopeInfo<> info(*scope_info);
- // Get the nearest enclosing function context.
- Handle<Context>
context(Context::cast(it.frame()->context())->fcontext());
-
// Get the locals names and values into a temporary array.
//
// TODO(1240907): Hide compiler-introduced stack variables
@@ -9996,11 +9990,6 @@
// confusing.
Handle<FixedArray> locals =
isolate->factory()->NewFixedArray(info.NumberOfLocals() * 2);
-
- // Fill in the names of the locals.
- for (int i = 0; i < info.NumberOfLocals(); i++) {
- locals->set(i * 2, *info.LocalName(i));
- }
// Fill in the values of the locals.
if (is_optimized_frame) {
@@ -10010,15 +9999,22 @@
// TODO(1140): We should be able to get the correct values
// for locals in optimized frames.
for (int i = 0; i < info.NumberOfLocals(); i++) {
+ locals->set(i * 2, *info.LocalName(i));
locals->set(i * 2 + 1, isolate->heap()->undefined_value());
}
} else {
- for (int i = 0; i < info.number_of_stack_slots(); i++) {
- // Get the value from the stack.
+ int i = 0;
+ for (; i < info.number_of_stack_slots(); ++i) {
+ // Use the value from the stack.
+ locals->set(i * 2, *info.LocalName(i));
locals->set(i * 2 + 1, it.frame()->GetExpression(i));
}
- for (int i = info.number_of_stack_slots(); i < info.NumberOfLocals();
i++) {
+ // Get the context containing declarations.
+ Handle<Context> context(
+ Context::cast(it.frame()->context())->declaration_context());
+ for (; i < info.NumberOfLocals(); ++i) {
Handle<String> name = info.LocalName(i);
+ locals->set(i * 2, *name);
locals->set(i * 2 + 1,
context->get(scope_info->ContextSlotIndex(*name, NULL)));
}
@@ -10239,7 +10235,7 @@
// Third fill all context locals.
Handle<Context> frame_context(Context::cast(frame->context()));
- Handle<Context> function_context(frame_context->fcontext());
+ Handle<Context> function_context(frame_context->declaration_context());
if (!CopyContextLocalsToScopeObject(isolate,
serialized_scope_info, scope_info,
function_context, local_scope)) {
@@ -11121,7 +11117,7 @@
context->set_extension(*local_scope);
// Copy any with contexts present and chain them in front of this
context.
Handle<Context> frame_context(Context::cast(frame->context()));
- Handle<Context> function_context(frame_context->fcontext());
+ Handle<Context> function_context(frame_context->declaration_context());
context = CopyWithContextChain(isolate, frame_context, context);
if (additional_context->IsJSObject()) {
=======================================
--- /branches/bleeding_edge/src/x64/code-stubs-x64.cc Wed Jun 22 01:28:35
2011
+++ /branches/bleeding_edge/src/x64/code-stubs-x64.cc Tue Jun 28 08:22:08
2011
@@ -132,7 +132,6 @@
// Setup the fixed slots.
__ Set(rbx, 0); // Set to NULL.
__ movq(Operand(rax, Context::SlotOffset(Context::CLOSURE_INDEX)), rcx);
- __ movq(Operand(rax, Context::SlotOffset(Context::FCONTEXT_INDEX)), rax);
__ movq(Operand(rax, Context::SlotOffset(Context::PREVIOUS_INDEX)), rsi);
__ movq(Operand(rax, Context::SlotOffset(Context::EXTENSION_INDEX)),
rbx);
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Jun 24 07:30:10
2011
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Tue Jun 28 08:22:08
2011
@@ -680,13 +680,16 @@
// We bypass the general EmitSlotSearch because we know more about
// this specific context.
- // The variable in the decl always resides in the current context.
+ // The variable in the decl always resides in the current function
+ // context.
ASSERT_EQ(0, scope()->ContextChainLength(variable->scope()));
if (FLAG_debug_code) {
- // Check if we have the correct context pointer.
- __ movq(rbx, ContextOperand(rsi, Context::FCONTEXT_INDEX));
- __ cmpq(rbx, rsi);
- __ Check(equal, "Unexpected declaration in current context.");
+ // Check that we're not inside a with or catch context.
+ __ movq(rbx, FieldOperand(rsi, HeapObject::kMapOffset));
+ __ CompareRoot(rbx, Heap::kWithContextMapRootIndex);
+ __ Check(not_equal, "Declaration in with context.");
+ __ CompareRoot(rbx, Heap::kCatchContextMapRootIndex);
+ __ Check(not_equal, "Declaration in catch context.");
}
if (mode == Variable::CONST) {
__ LoadRoot(kScratchRegister, Heap::kTheHoleValueRootIndex);
@@ -1778,17 +1781,7 @@
__ j(not_equal, &skip);
__ movq(Operand(rbp, SlotOffset(slot)), rax);
break;
- case Slot::CONTEXT: {
- __ movq(rcx, ContextOperand(rsi, Context::FCONTEXT_INDEX));
- __ movq(rdx, ContextOperand(rcx, slot->index()));
- __ CompareRoot(rdx, Heap::kTheHoleValueRootIndex);
- __ j(not_equal, &skip);
- __ movq(ContextOperand(rcx, slot->index()), rax);
- int offset = Context::SlotOffset(slot->index());
- __ movq(rdx, rax); // Preserve the stored value in eax.
- __ RecordWrite(rcx, offset, rdx, rbx);
- break;
- }
+ case Slot::CONTEXT:
case Slot::LOOKUP:
__ push(rax);
__ push(rsi);
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Thu Jun 9
05:45:26 2011
+++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Tue Jun 28
08:22:08 2011
@@ -3628,14 +3628,17 @@
movq(dst, rsi);
}
- // We should not have found a 'with' context by walking the context chain
- // (i.e., the static scope chain and runtime context chain do not agree).
- // A variable occurring in such a scope should have slot type LOOKUP and
- // not CONTEXT.
+ // We should not have found a with or catch context by walking the
context
+ // chain (i.e., the static scope chain and runtime context chain do not
+ // agree). A variable occurring in such a scope should have slot type
+ // LOOKUP and not CONTEXT.
if (emit_debug_code()) {
- cmpq(dst, Operand(dst, Context::SlotOffset(Context::FCONTEXT_INDEX)));
- Check(equal, "Yo dawg, I heard you liked function contexts "
- "so I put function contexts in all your contexts");
+ CompareRoot(FieldOperand(dst, HeapObject::kMapOffset),
+ Heap::kWithContextMapRootIndex);
+ Check(not_equal, "Variable resolved to with context.");
+ CompareRoot(FieldOperand(dst, HeapObject::kMapOffset),
+ Heap::kCatchContextMapRootIndex);
+ Check(not_equal, "Variable resolved to catch context.");
}
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
