Revision: 6635
Author: [email protected]
Date: Fri Feb 4 04:06:41 2011
Log: Remove the redundant load on every context lookup.
There was an unnecessary load on every statically-resolved context lookup.
Remove it.
This revealed a hidden bug in const initializers inside 'with'. They claim
to be statically resolved (having slot type CONTEXT) but they occur in a
spot where the runtime context chain and the static scope chain do not
agree. This is fixed by special casing const initializers in the backend.
Review URL: http://codereview.chromium.org/6384020
http://code.google.com/p/v8/source/detail?r=6635
Modified:
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
/branches/bleeding_edge/src/ia32/lithium-ia32.cc
/branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
/branches/bleeding_edge/src/prettyprinter.cc
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Jan 28
06:18:26 2011
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Feb 4
04:06:41 2011
@@ -610,7 +610,7 @@
__ mov(location, src);
// Emit the write barrier code if the location is in the heap.
if (dst->type() == Slot::CONTEXT) {
- int offset = FixedArray::kHeaderSize + dst->index() * kPointerSize;
+ int offset = Context::SlotOffset(dst->index());
__ RecordWrite(scratch1, offset, src, scratch2);
}
}
@@ -666,10 +666,11 @@
// 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.
+ // 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.");
@@ -1124,8 +1125,11 @@
// Check that last extension is NULL.
__ cmp(ContextOperand(context, Context::EXTENSION_INDEX), Immediate(0));
__ j(not_equal, slow);
- __ mov(temp, ContextOperand(context, Context::FCONTEXT_INDEX));
- return ContextOperand(temp, slot->index());
+
+ // This function is used only for loads, not stores, so it's safe to
+ // return an esi-based operand (the write barrier cannot be allowed to
+ // destroy the esi register).
+ return ContextOperand(context, slot->index());
}
@@ -2000,57 +2004,75 @@
Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
EmitCallIC(ic, RelocInfo::CODE_TARGET);
- } else if (var->mode() != Variable::CONST || op == Token::INIT_CONST) {
- // Perform the assignment for non-const variables and for
initialization
- // of const variables. Const assignments are simply skipped.
- Label done;
+ } else if (op == Token::INIT_CONST) {
+ // Like var declarations, const declarations are hoisted to function
+ // scope. However, unlike var initializers, const initializers are
able
+ // to drill a hole to that function context, even from inside a 'with'
+ // context. We thus bypass the normal static scope lookup.
Slot* slot = var->AsSlot();
+ Label skip;
switch (slot->type()) {
case Slot::PARAMETER:
+ // No const parameters.
+ UNREACHABLE();
+ break;
case Slot::LOCAL:
- if (op == Token::INIT_CONST) {
- // Detect const reinitialization by checking for the hole value.
- __ mov(edx, Operand(ebp, SlotOffset(slot)));
- __ cmp(edx, Factory::the_hole_value());
- __ j(not_equal, &done);
- }
+ __ mov(edx, Operand(ebp, SlotOffset(slot)));
+ __ cmp(edx, Factory::the_hole_value());
+ __ 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, 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::LOOKUP:
+ __ push(eax);
+ __ push(esi);
+ __ push(Immediate(var->name()));
+ __ CallRuntime(Runtime::kInitializeConstContextSlot, 3);
+ break;
+ }
+ __ bind(&skip);
+
+ } else if (var->mode() != Variable::CONST) {
+ // Perform the assignment for non-const variables. Const assignments
+ // are simply skipped.
+ Slot* slot = var->AsSlot();
+ switch (slot->type()) {
+ case Slot::PARAMETER:
+ case Slot::LOCAL:
// Perform the assignment.
__ mov(Operand(ebp, SlotOffset(slot)), eax);
break;
case Slot::CONTEXT: {
MemOperand target = EmitSlotSearch(slot, ecx);
- if (op == Token::INIT_CONST) {
- // Detect const reinitialization by checking for the hole value.
- __ mov(edx, target);
- __ cmp(edx, Factory::the_hole_value());
- __ j(not_equal, &done);
- }
// Perform the assignment and issue the write barrier.
__ mov(target, eax);
// The value of the assignment is in eax. RecordWrite clobbers its
// register arguments.
__ mov(edx, eax);
- int offset = FixedArray::kHeaderSize + slot->index() *
kPointerSize;
+ int offset = Context::SlotOffset(slot->index());
__ RecordWrite(ecx, offset, edx, ebx);
break;
}
case Slot::LOOKUP:
- // Call the runtime for the assignment. The runtime will ignore
- // const reinitialization.
+ // Call the runtime for the assignment.
__ push(eax); // Value.
__ push(esi); // Context.
__ push(Immediate(var->name()));
- if (op == Token::INIT_CONST) {
- // The runtime will ignore const redeclaration.
- __ CallRuntime(Runtime::kInitializeConstContextSlot, 3);
- } else {
- __ CallRuntime(Runtime::kStoreContextSlot, 3);
- }
+ __ CallRuntime(Runtime::kStoreContextSlot, 3);
break;
}
- __ bind(&done);
}
}
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Thu Feb 3
05:10:28 2011
+++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Fri Feb 4
04:06:41 2011
@@ -1914,19 +1914,15 @@
void LCodeGen::DoLoadContextSlot(LLoadContextSlot* instr) {
- Register context = ToRegister(instr->InputAt(0));
+ Register context = ToRegister(instr->context());
Register result = ToRegister(instr->result());
- __ mov(result,
- Operand(context, Context::SlotOffset(Context::FCONTEXT_INDEX)));
- __ mov(result, ContextOperand(result, instr->slot_index()));
+ __ mov(result, ContextOperand(context, instr->slot_index()));
}
void LCodeGen::DoStoreContextSlot(LStoreContextSlot* instr) {
Register context = ToRegister(instr->context());
Register value = ToRegister(instr->value());
- __ mov(context,
- Operand(context, Context::SlotOffset(Context::FCONTEXT_INDEX)));
__ mov(ContextOperand(context, instr->slot_index()), value);
if (instr->needs_write_barrier()) {
Register temp = ToRegister(instr->TempAt(0));
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Thu Feb 3 05:10:28
2011
+++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Fri Feb 4 04:06:41
2011
@@ -1685,13 +1685,15 @@
LInstruction* LChunkBuilder::DoStoreContextSlot(HStoreContextSlot* instr) {
- LOperand* context = UseTempRegister(instr->context());
+ LOperand* context;
LOperand* value;
LOperand* temp;
if (instr->NeedsWriteBarrier()) {
+ context = UseTempRegister(instr->context());
value = UseTempRegister(instr->value());
temp = TempRegister();
} else {
+ context = UseRegister(instr->context());
value = UseRegister(instr->value());
temp = NULL;
}
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Jan 26
07:02:02 2011
+++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Fri Feb 4
04:06:41 2011
@@ -1523,11 +1523,21 @@
mov(dst, Operand(dst, Context::SlotOffset(Context::CLOSURE_INDEX)));
mov(dst, FieldOperand(dst, JSFunction::kContextOffset));
}
- // The context may be an intermediate context, not a function context.
- mov(dst, Operand(dst, Context::SlotOffset(Context::FCONTEXT_INDEX)));
- } else { // Slot is in the current function context.
- // The context may be an intermediate context, not a function context.
- mov(dst, Operand(esi, Context::SlotOffset(Context::FCONTEXT_INDEX)));
+ } else {
+ // Slot is in the current function context. Move it into the
+ // destination register in case we store into it (the write barrier
+ // cannot be allowed to destroy the context in esi).
+ 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.
+ if (FLAG_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");
}
}
=======================================
--- /branches/bleeding_edge/src/prettyprinter.cc Tue Dec 7 03:01:02 2010
+++ /branches/bleeding_edge/src/prettyprinter.cc Fri Feb 4 04:06:41 2011
@@ -297,13 +297,13 @@
Print("parameter[%d]", node->index());
break;
case Slot::LOCAL:
- Print("frame[%d]", node->index());
+ Print("local[%d]", node->index());
break;
case Slot::CONTEXT:
- Print(".context[%d]", node->index());
+ Print("context[%d]", node->index());
break;
case Slot::LOOKUP:
- Print(".context[");
+ Print("lookup[");
PrintLiteral(node->var()->name(), false);
Print("]");
break;
@@ -999,24 +999,7 @@
void AstPrinter::VisitSlot(Slot* node) {
PrintIndented("SLOT ");
- switch (node->type()) {
- case Slot::PARAMETER:
- Print("parameter[%d]", node->index());
- break;
- case Slot::LOCAL:
- Print("frame[%d]", node->index());
- break;
- case Slot::CONTEXT:
- Print(".context[%d]", node->index());
- break;
- case Slot::LOOKUP:
- Print(".context[");
- PrintLiteral(node->var()->name(), false);
- Print("]");
- break;
- default:
- UNREACHABLE();
- }
+ PrettyPrinter::VisitSlot(node);
Print("\n");
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
