Remove the redundant load on every context lookup.
authorkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 4 Feb 2011 12:06:41 +0000 (12:06 +0000)
committerkmillikin@chromium.org <kmillikin@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Fri, 4 Feb 2011 12:06:41 +0000 (12:06 +0000)
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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6635 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ia32/full-codegen-ia32.cc
src/ia32/lithium-codegen-ia32.cc
src/ia32/lithium-ia32.cc
src/ia32/macro-assembler-ia32.cc
src/prettyprinter.cc

index 3ef8159..e6f7416 100644 (file)
@@ -610,7 +610,7 @@ void FullCodeGenerator::Move(Slot* dst,
   __ 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 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable,
         // 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 @@ MemOperand FullCodeGenerator::ContextSlotOperandCheckExtensions(
   // 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 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var,
     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:
+        __ 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:
-        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);
-        }
         // 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);
   }
 }
 
index 851a111..6dc3c3c 100644 (file)
@@ -1914,19 +1914,15 @@ void LCodeGen::DoStoreGlobal(LStoreGlobal* instr) {
 
 
 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));
index 1d5cefc..092601d 100644 (file)
@@ -1685,13 +1685,15 @@ LInstruction* LChunkBuilder::DoLoadContextSlot(HLoadContextSlot* instr) {
 
 
 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;
   }
index a4c9b11..ee4e3d9 100644 (file)
@@ -1523,11 +1523,21 @@ void MacroAssembler::LoadContext(Register dst, int context_chain_length) {
       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");
   }
 }
 
index 211f3f6..dda7abb 100644 (file)
@@ -297,13 +297,13 @@ void PrettyPrinter::VisitSlot(Slot* node) {
       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::VisitCatchExtensionObject(CatchExtensionObject* node) {
 
 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");
 }