[Interpreter] Unify global and unallocated variable access.

src/interpreter/bytecode-generator.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/interpreter/bytecode-generator.h"
 
 #include "src/compiler.h"
 #include "src/interpreter/control-flow-builders.h"
 #include "src/objects.h"
 #include "src/parser.h"
@@ skipping 695 matching lines @@
       case ObjectLiteral::Property::CONSTANT:
         UNREACHABLE();
       case ObjectLiteral::Property::MATERIALIZED_LITERAL:
         DCHECK(!CompileTimeValue::IsCompileTimeValue(property->value()));
       // Fall through.
       case ObjectLiteral::Property::COMPUTED: {
         // It is safe to use [[Put]] here because the boilerplate already
         // contains computed properties with an uninitialized value.
         if (literal_key->value()->IsInternalizedString()) {
           if (property->emit_store()) {
-            Register name = inner_temporary_register_scope.NewRegister();
-            builder()
-                ->LoadLiteral(literal_key->AsPropertyName())
-                .StoreAccumulatorInRegister(name);
+            size_t name_index =
+                builder()->GetConstantPoolEntry(literal_key->AsPropertyName());
             VisitForAccumulatorValue(property->value());
-            builder()->StoreNamedProperty(literal, name,
+            builder()->StoreNamedProperty(literal, name_index,
                                           feedback_index(property->GetSlot(0)),
                                           language_mode());
           } else {
             VisitForEffect(property->value());
           }
         } else {
           inner_temporary_register_scope.PrepareForConsecutiveAllocations(3);
           Register key =
               inner_temporary_register_scope.NextConsecutiveRegister();
           Register value =
@@ skipping 200 matching lines @@
     }
     case VariableLocation::PARAMETER: {
       // The parameter indices are shifted by 1 (receiver is variable
       // index -1 but is parameter index 0 in BytecodeArrayBuilder).
       Register source = builder()->Parameter(variable->index() + 1);
       execution_result()->SetResultInRegister(source);
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
-      TemporaryRegisterScope temporary_register_scope(builder());
-      Register obj = temporary_register_scope.NewRegister();
-      builder()->LoadContextSlot(execution_context()->reg(),
-                                 Context::GLOBAL_OBJECT_INDEX);
-      builder()->StoreAccumulatorInRegister(obj);
-      builder()->LoadLiteral(variable->name());
-      builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode());
+      size_t name_index = builder()->GetConstantPoolEntry(variable->name());
+      builder()->LoadGlobal(name_index, feedback_index(slot), language_mode());
       execution_result()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::CONTEXT: {
       ContextScope* context = execution_context()->Previous(variable->scope());
       if (context) {
         builder()->LoadContextSlot(context->reg(), variable->index());
         execution_result()->SetResultInAccumulator();
       } else {
         UNIMPLEMENTED();
@@ skipping 21 matching lines @@
     case VariableLocation::PARAMETER: {
       // The parameter indices are shifted by 1 (receiver is variable
       // index -1 but is parameter index 0 in BytecodeArrayBuilder).
       Register destination(builder()->Parameter(variable->index() + 1));
       builder()->StoreAccumulatorInRegister(destination);
       RecordStoreToRegister(destination);
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
-      Register value = execution_result()->NewRegister();
-      Register obj = execution_result()->NewRegister();
-      Register name = execution_result()->NewRegister();
-
-      // TODO(rmcilroy): Investigate whether we can avoid having to stash the
-      // value in a register.
-      builder()->StoreAccumulatorInRegister(value);
-      builder()->LoadContextSlot(execution_context()->reg(),
-                                 Context::GLOBAL_OBJECT_INDEX);
-      builder()->StoreAccumulatorInRegister(obj);
-      builder()->LoadLiteral(variable->name());
-      builder()->StoreAccumulatorInRegister(name);
-      builder()->LoadAccumulatorWithRegister(value);
-      builder()->StoreNamedProperty(obj, name, feedback_index(slot),
-                                    language_mode());
+      size_t name_index = builder()->GetConstantPoolEntry(variable->name());
+      builder()->StoreGlobal(name_index, feedback_index(slot), language_mode());
       break;
     }
     case VariableLocation::CONTEXT: {
       // TODO(rmcilroy): support const mode initialization.
       ContextScope* context = execution_context()->Previous(variable->scope());
       if (context) {
         builder()->StoreContextSlot(context->reg(), variable->index());
       } else {
         UNIMPLEMENTED();
       }
       break;
     }
     case VariableLocation::LOOKUP:
       UNIMPLEMENTED();
   }
 }
 
 
 void BytecodeGenerator::VisitAssignment(Assignment* expr) {
   DCHECK(expr->target()->IsValidReferenceExpression());
   Register object, key;
+  size_t name_index;
 
   // Left-hand side can only be a property, a global or a variable slot.
   Property* property = expr->target()->AsProperty();
   LhsKind assign_type = Property::GetAssignType(property);
 
   // Evaluate LHS expression.
   switch (assign_type) {
     case VARIABLE:
       // Nothing to do to evaluate variable assignment LHS.
       break;
     case NAMED_PROPERTY: {
       object = VisitForRegisterValue(property->obj());
-      key = execution_result()->NewRegister();
-      builder()->LoadLiteral(property->key()->AsLiteral()->AsPropertyName());
-      builder()->StoreAccumulatorInRegister(key);
+      name_index = builder()->GetConstantPoolEntry(
+          property->key()->AsLiteral()->AsPropertyName());
       break;
     }
     case KEYED_PROPERTY: {
       object = VisitForRegisterValue(property->obj());
       key = VisitForRegisterValue(property->key());
       break;
     }
     case NAMED_SUPER_PROPERTY:
     case KEYED_SUPER_PROPERTY:
       UNIMPLEMENTED();
@@ skipping 11 matching lines @@
   FeedbackVectorSlot slot = expr->AssignmentSlot();
   switch (assign_type) {
     case VARIABLE: {
       // TODO(oth): The VisitVariableAssignment() call is hard to reason about.
       // Is the value in the accumulator safe? Yes, but scary.
       Variable* variable = expr->target()->AsVariableProxy()->var();
       VisitVariableAssignment(variable, slot);
       break;
     }
     case NAMED_PROPERTY:
-      builder()->StoreNamedProperty(object, key, feedback_index(slot),
+      builder()->StoreNamedProperty(object, name_index, feedback_index(slot),
                                     language_mode());
       break;
     case KEYED_PROPERTY:
       builder()->StoreKeyedProperty(object, key, feedback_index(slot),
                                     language_mode());
       break;
     case NAMED_SUPER_PROPERTY:
     case KEYED_SUPER_PROPERTY:
       UNIMPLEMENTED();
   }
@@ skipping 10 matching lines @@
 }
 
 
 void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* expr) {
   LhsKind property_kind = Property::GetAssignType(expr);
   FeedbackVectorSlot slot = expr->PropertyFeedbackSlot();
   switch (property_kind) {
     case VARIABLE:
       UNREACHABLE();
     case NAMED_PROPERTY: {
-      builder()->LoadLiteral(expr->key()->AsLiteral()->AsPropertyName());
-      builder()->LoadNamedProperty(obj, feedback_index(slot), language_mode());
+      size_t name_index = builder()->GetConstantPoolEntry(
+          expr->key()->AsLiteral()->AsPropertyName());
+      builder()->LoadNamedProperty(obj, name_index, feedback_index(slot),
+                                   language_mode());
       break;
     }
     case KEYED_PROPERTY: {
       VisitForAccumulatorValue(expr->key());
       builder()->LoadKeyedProperty(obj, feedback_index(slot), language_mode());
       break;
     }
     case NAMED_SUPER_PROPERTY:
     case KEYED_SUPER_PROPERTY:
       UNIMPLEMENTED();
   }
-  execution_result()->SetResultInAccumulator();

[oth, 2015/10/22 10:54:30]

I find this function scary. It is setting the result in the accumulator, but no
longer signalling that and relying on the layer above to do so. Probably need a
comment here too or leave as-is so there's a consistent pattern.

[rmcilroy, 2015/10/22 13:27:48]

Done - added VisitPropertyLoadForAccumulator for this and added some
AccumulatorResultScopes (for documentation more than anything) to some of the
other visitors we have which store the result in the accumulator.

 }
 
 
 void BytecodeGenerator::VisitProperty(Property* expr) {
   Register obj = VisitForRegisterValue(expr->obj());
   VisitPropertyLoad(obj, expr);
+  execution_result()->SetResultInAccumulator();
 }
 
 
 Register BytecodeGenerator::VisitArguments(ZoneList<Expression*>* args) {
   // Visit arguments and place in a contiguous block of temporary
   // registers.  Return the first temporary register corresponding to
   // the first argument.
   //
   // NB the caller may have already called
   // PrepareForConsecutiveAllocations() with args->length() + N. The
@@ skipping 33 matching lines @@
   ZoneList<Expression*>* args = expr->arguments();
   execution_result()->PrepareForConsecutiveAllocations(args->length() + 1);
   Register receiver = execution_result()->NextConsecutiveRegister();
 
   switch (call_type) {
     case Call::PROPERTY_CALL: {
       Property* property = callee_expr->AsProperty();
       if (property->IsSuperAccess()) {
         UNIMPLEMENTED();
       }
+
       VisitForAccumulatorValue(property->obj());
       builder()->StoreAccumulatorInRegister(receiver);
-      // Need a result scope here to keep our consecutive
-      // temporaries.
-      AccumulatorResultScope accumulator_execution_result(this);
-      // Perform a property load of the callee.
       VisitPropertyLoad(receiver, property);
       builder()->StoreAccumulatorInRegister(callee);
       break;
     }
     case Call::GLOBAL_CALL: {
       // Receiver is undefined for global calls.
       builder()->LoadUndefined().StoreAccumulatorInRegister(receiver);
       // Load callee as a global variable.
       VariableProxy* proxy = callee_expr->AsVariableProxy();
       // Result scope for VisitVariableLoad to avoid using our temporaries
@@ skipping 317 matching lines @@
   }
 }
 
 
 void BytecodeGenerator::VisitSetHomeObject(Register value, Register home_object,
                                            ObjectLiteralProperty* property,
                                            int slot_number) {
   Expression* expr = property->value();
   if (!FunctionLiteral::NeedsHomeObject(expr)) return;
 
-  // TODO(rmcilroy): Remove UNIMPLEMENTED once we have tests for setting the
-  // home object.
   UNIMPLEMENTED();
-
-  TemporaryRegisterScope temporary_register_scope(builder());
-  Register name = temporary_register_scope.NewRegister();
-  isolate()->factory()->home_object_symbol();
-  builder()
-      ->LoadLiteral(isolate()->factory()->home_object_symbol())
-      .StoreAccumulatorInRegister(name)
-      .StoreNamedProperty(home_object, name,
-                          feedback_index(property->GetSlot(slot_number)),
-                          language_mode());
 }
 
 
 void BytecodeGenerator::VisitFunctionClosureForContext() {
   Scope* closure_scope = execution_context()->scope()->ClosureScope();
   if (closure_scope->is_script_scope() ||
       closure_scope->is_module_scope()) {
     // Contexts nested in the native context have a canonical empty function as
     // their closure, not the anonymous closure containing the global code.
     // Pass a SMI sentinel and let the runtime look up the empty function.
@@ skipping 92 matching lines @@
 }
 
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return info()->feedback_vector()->GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8