Change interface to CodeGenerator::LoadFromSlot.

src/ia32/codegen-ia32.cc

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 556 matching lines @@
     // <global>.<variable> and perform a (regular non-contextual) property
     // load to make sure we do not get reference errors.
     Slot global(variable, Slot::CONTEXT, Context::GLOBAL_INDEX);
     Literal key(variable->name());
     Property property(&global, &key, RelocInfo::kNoPosition);
     Reference ref(this, &property);
     ref.GetValue();
   } else if (variable != NULL && variable->slot() != NULL) {
     // For a variable that rewrites to a slot, we signal it is the immediate
     // subexpression of a typeof.
-    LoadFromSlotCheckForArguments(variable->slot(), INSIDE_TYPEOF);
+    Result result =
+        LoadFromSlotCheckForArguments(variable->slot(), INSIDE_TYPEOF);
+    frame()->Push(&result);
   } else {
     // Anything else can be handled normally.
     Load(expr);
   }
 }
 
 
 ArgumentsAllocationMode CodeGenerator::ArgumentsMode() {
   if (scope()->arguments() == NULL) return NO_ARGUMENTS_ALLOCATION;
   ASSERT(scope()->arguments_shadow() != NULL);
@@ skipping 28 matching lines @@
   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();
+    Result probe = LoadFromSlot(arguments->slot(), NOT_INSIDE_TYPEOF);
     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);
     }
   }
@@ skipping 1692 matching lines @@
   frame()->Dup();
   Handle<String> name = Factory::LookupAsciiSymbol("apply");
   frame()->Push(name);
   Result answer = frame()->CallLoadIC(RelocInfo::CODE_TARGET);
   __ nop();
   frame()->Push(&answer);
 
   // Load the receiver and the existing arguments object onto the
   // expression stack. Avoid allocating the arguments object here.
   Load(receiver);
-  LoadFromSlot(scope()->arguments()->var()->slot(), NOT_INSIDE_TYPEOF);
+  Result existing_args =
+      LoadFromSlot(scope()->arguments()->var()->slot(), NOT_INSIDE_TYPEOF);
+  frame()->Push(&existing_args);
 
   // Emit the source position information after having loaded the
   // receiver and the arguments.
   CodeForSourcePosition(position);
   // Contents of frame at this point:
   // Frame[0]: arguments object of the current function or the hole.
   // Frame[1]: receiver
   // Frame[2]: applicand.apply
   // Frame[3]: applicand.
 
@@ skipping 1632 matching lines @@
       exit.Jump();
       else_.Bind();
       Load(node->else_expression());
     }
   }
 
   exit.Bind();
 }
 
 
-void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) {
+Result CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) {
+  Result result;

[fschneider, 2010/02/12 12:23:06]

I'd move the declaration into the innermost necessary scope because it is not
used on some paths.

[Kevin Millikin (Chromium), 2010/02/12 12:39:21]

Ultimately it will be a field in the codegen state, so I really want one
declaration at the very top of the function (so it's easy to remove) called
result (so it's easy to search for).

   if (slot->type() == Slot::LOOKUP) {
     ASSERT(slot->var()->is_dynamic());

[fschneider, 2010/02/12 12:23:06]

Declare result here instead.

-
     JumpTarget slow;
     JumpTarget done;
-    Result value;
 
     // Generate fast-case code for variables that might be shadowed by
     // eval-introduced variables.  Eval is used a lot without
     // introducing variables.  In those cases, we do not want to
     // perform a runtime call for all variables in the scope
     // containing the eval.
     if (slot->var()->mode() == Variable::DYNAMIC_GLOBAL) {
-      value = LoadFromGlobalSlotCheckExtensions(slot, typeof_state, &slow);
+      result = LoadFromGlobalSlotCheckExtensions(slot, typeof_state, &slow);
       // If there was no control flow to slow, we can exit early.
-      if (!slow.is_linked()) {
-        frame_->Push(&value);
-        return;
-      }
-
-      done.Jump(&value);
+      if (!slow.is_linked()) return result;
+      done.Jump(&result);
 
     } else if (slot->var()->mode() == Variable::DYNAMIC_LOCAL) {
       Slot* potential_slot = slot->var()->local_if_not_shadowed()->slot();

[fschneider, 2010/02/12 12:23:06]

Declare result here.

       // Only generate the fast case for locals that rewrite to slots.
       // This rules out argument loads.
       if (potential_slot != NULL) {
         // Allocate a fresh register to use as a temp in
         // ContextSlotOperandCheckExtensions and to hold the result
         // value.
-        value = allocator_->Allocate();
-        ASSERT(value.is_valid());
-        __ mov(value.reg(),
+        result = allocator()->Allocate();
+        ASSERT(result.is_valid());
+        __ mov(result.reg(),
                ContextSlotOperandCheckExtensions(potential_slot,
-                                                 value,
+                                                 result,
                                                  &slow));
         if (potential_slot->var()->mode() == Variable::CONST) {
-          __ cmp(value.reg(), Factory::the_hole_value());
-          done.Branch(not_equal, &value);
-          __ mov(value.reg(), Factory::undefined_value());
+          __ cmp(result.reg(), Factory::the_hole_value());
+          done.Branch(not_equal, &result);
+          __ mov(result.reg(), Factory::undefined_value());
         }
         // There is always control flow to slow from
         // ContextSlotOperandCheckExtensions so we have to jump around
         // it.
-        done.Jump(&value);
+        done.Jump(&result);
       }
     }
 
     slow.Bind();
     // A runtime call is inevitable.  We eagerly sync frame elements
     // to memory so that we can push the arguments directly into place
     // on top of the frame.
-    frame_->SyncRange(0, frame_->element_count() - 1);
-    frame_->EmitPush(esi);
-    frame_->EmitPush(Immediate(slot->var()->name()));
+    frame()->SyncRange(0, frame()->element_count() - 1);
+    frame()->EmitPush(esi);
+    frame()->EmitPush(Immediate(slot->var()->name()));
     if (typeof_state == INSIDE_TYPEOF) {
-      value =
-          frame_->CallRuntime(Runtime::kLoadContextSlotNoReferenceError, 2);
+      result =
+          frame()->CallRuntime(Runtime::kLoadContextSlotNoReferenceError, 2);
     } else {
-      value = frame_->CallRuntime(Runtime::kLoadContextSlot, 2);
+      result = frame()->CallRuntime(Runtime::kLoadContextSlot, 2);
     }
 
-    done.Bind(&value);
-    frame_->Push(&value);
+    done.Bind(&result);
+    return result;
 
   } else if (slot->var()->mode() == Variable::CONST) {
     // Const slots may contain 'the hole' value (the constant hasn't been
     // initialized yet) which needs to be converted into the 'undefined'
     // value.
     //
     // We currently spill the virtual frame because constants use the
     // potentially unsafe direct-frame access of SlotOperand.
     VirtualFrame::SpilledScope spilled_scope;
     Comment cmnt(masm_, "[ Load const");
-    JumpTarget exit;
+    Label exit;
     __ mov(ecx, SlotOperand(slot, ecx));
     __ cmp(ecx, Factory::the_hole_value());
-    exit.Branch(not_equal);
+    __ j(not_equal, &exit);
     __ mov(ecx, Factory::undefined_value());
-    exit.Bind();
-    frame_->EmitPush(ecx);
+    __ bind(&exit);
+    return Result(ecx);
 
   } else if (slot->type() == Slot::PARAMETER) {
-    frame_->PushParameterAt(slot->index());
+    frame()->PushParameterAt(slot->index());
+    return frame()->Pop();

[fschneider, 2010/02/12 12:23:06]

Would it make sense to have a virtual frame function 
"Result getParameterAt(index)" or 
"Result getLocalAt(index)" instead of pushing/popping?

[Kevin Millikin (Chromium), 2010/02/12 12:39:21]

Yes, and PushParameterAt should go away.  That's a separate change I'll make
after it's no longer used.

 
   } else if (slot->type() == Slot::LOCAL) {
-    frame_->PushLocalAt(slot->index());
+    frame()->PushLocalAt(slot->index());
+    return frame()->Pop();
 
   } else {
     // The other remaining slot types (LOOKUP and GLOBAL) cannot reach

[fschneider, 2010/02/12 12:23:06]

Declare result here.

     // here.
     //
     // The use of SlotOperand below is safe for an unspilled frame
     // because it will always be a context slot.
     ASSERT(slot->type() == Slot::CONTEXT);
-    Result temp = allocator_->Allocate();
-    ASSERT(temp.is_valid());
-    __ mov(temp.reg(), SlotOperand(slot, temp.reg()));
-    frame_->Push(&temp);
+    result = allocator()->Allocate();
+    ASSERT(result.is_valid());
+    __ mov(result.reg(), SlotOperand(slot, result.reg()));
+    return result;
   }
 }
 
 
-void CodeGenerator::LoadFromSlotCheckForArguments(Slot* slot,
-                                                  TypeofState state) {
-  LoadFromSlot(slot, state);
+Result CodeGenerator::LoadFromSlotCheckForArguments(Slot* slot,
+                                                    TypeofState state) {
+  Result result = LoadFromSlot(slot, state);
 
   // Bail out quickly if we're not using lazy arguments allocation.
-  if (ArgumentsMode() != LAZY_ARGUMENTS_ALLOCATION) return;
+  if (ArgumentsMode() != LAZY_ARGUMENTS_ALLOCATION) return result;
 
   // ... or if the slot isn't a non-parameter arguments slot.
-  if (slot->type() == Slot::PARAMETER || !slot->is_arguments()) return;
-
-  // Pop the loaded value from the stack.
-  Result value = frame_->Pop();
+  if (slot->type() == Slot::PARAMETER || !slot->is_arguments()) return result;
 
   // If the loaded value is a constant, we know if the arguments
   // object has been lazily loaded yet.
-  if (value.is_constant()) {
-    if (value.handle()->IsTheHole()) {
-      Result arguments = StoreArgumentsObject(false);
-      frame_->Push(&arguments);
+  if (result.is_constant()) {
+    if (result.handle()->IsTheHole()) {
+      result.Unuse();
+      return StoreArgumentsObject(false);
     } else {
-      frame_->Push(&value);
+      return result;
     }
-    return;
   }
 
   // The loaded value is in a register. If it is the sentinel that
   // indicates that we haven't loaded the arguments object yet, we
   // need to do it now.
   JumpTarget exit;
-  __ cmp(Operand(value.reg()), Immediate(Factory::the_hole_value()));
-  frame_->Push(&value);
-  exit.Branch(not_equal);
-  Result arguments = StoreArgumentsObject(false);
-  frame_->SetElementAt(0, &arguments);
-  exit.Bind();
+  __ cmp(Operand(result.reg()), Immediate(Factory::the_hole_value()));
+  exit.Branch(not_equal, &result);
+
+  result.Unuse();
+  result = StoreArgumentsObject(false);
+  exit.Bind(&result);
+  return result;
 }
 
 
 Result CodeGenerator::LoadFromGlobalSlotCheckExtensions(
     Slot* slot,
     TypeofState typeof_state,
     JumpTarget* slow) {
   // Check that no extension objects have been created by calls to
   // eval from the current scope to the global scope.
   Register context = esi;
@@ skipping 153 matching lines @@
       // scope.
     }
 
     exit.Bind();
   }
 }
 
 
 void CodeGenerator::VisitSlot(Slot* node) {
   Comment cmnt(masm_, "[ Slot");
-  LoadFromSlotCheckForArguments(node, NOT_INSIDE_TYPEOF);
+  Result result = LoadFromSlotCheckForArguments(node, NOT_INSIDE_TYPEOF);
+  frame()->Push(&result);
 }
 
 
 void CodeGenerator::VisitVariableProxy(VariableProxy* node) {
   Comment cmnt(masm_, "[ VariableProxy");
   Variable* var = node->var();
   Expression* expr = var->rewrite();
   if (expr != NULL) {
     Visit(expr);
   } else {
@@ skipping 2246 matching lines @@
   Property* property = expression_->AsProperty();
   if (property != NULL) {
     cgen_->CodeForSourcePosition(property->position());
   }
 
   switch (type_) {
     case SLOT: {
       Comment cmnt(masm, "[ Load from Slot");
       Slot* slot = expression_->AsVariableProxy()->AsVariable()->slot();
       ASSERT(slot != NULL);
-      cgen_->LoadFromSlotCheckForArguments(slot, NOT_INSIDE_TYPEOF);
-      if (!persist_after_get_) {
-        cgen_->UnloadReference(this);
-      }
+      Result result =
+          cgen_->LoadFromSlotCheckForArguments(slot, NOT_INSIDE_TYPEOF);
+      if (!persist_after_get_) set_unloaded();
+      cgen_->frame()->Push(&result);
       break;
     }
 
     case NAMED: {
       Variable* var = expression_->AsVariableProxy()->AsVariable();
       bool is_global = var != NULL;
       ASSERT(!is_global || var->is_global());
 
       if (persist_after_get_) {
         cgen_->frame()->Dup();
@@ skipping 3530 matching lines @@
 
   // Call the runtime; it returns -1 (less), 0 (equal), or 1 (greater)
   // tagged as a small integer.
   __ bind(&runtime);
   __ TailCallRuntime(ExternalReference(Runtime::kStringCompare), 2, 1);
 }
 
 #undef __
 
 } }  // namespace v8::internal