[Interpreter] Removes assignment hazard scope.

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/ast/scopes.h"
 #include "src/compiler.h"
 #include "src/interpreter/control-flow-builders.h"
 #include "src/objects.h"
@@ skipping 196 matching lines @@
 
   Register NewRegister() {
     ExpressionResultScope* current_scope = generator()->execution_result();
     if ((current_scope == this) ||
         (current_scope->outer() == this &&
          !current_scope->allocator_.hasConsecutiveAllocations())) {
       // Regular case - Allocating registers in current or outer context.
       // VisitForRegisterValue allocates register in outer context.
       return allocator_.NewRegister();
     } else {
-      // We need this when allocating registers due to an Assignment hazard.
+      // Allocating a register other than the current or outer scope:
       // It might be expensive to walk the full context chain and compute the
       // list of consecutive reservations in the innerscopes. So allocates a
-      // new unallocated temporary register.
+      // new unallocated temporary register. We needed this in an implementation
+      // of Assignment hazard.
+      // TODO(mythria): Reevaluate this after we have the new assignment hazard
+      // implementation. Remove this if not required.

[rmcilroy, 2016/01/12 14:43:17]

I'd rather just remove this now if it is no longer needed (perhaps adding a TODO
that it might be necessary to allocated a new register here for outer scopes. If
you remove this call, also remove AllocateNewRegister since this is the only
place it's called.

[mythria, 2016/01/12 16:53:21]

Done.

       return allocator_.AllocateNewRegister();
     }
   }
 
   void PrepareForConsecutiveAllocations(size_t count) {
     allocator_.PrepareForConsecutiveAllocations(count);
   }
 
   Register NextConsecutiveRegister() {
     return allocator_.NextConsecutiveRegister();
@@ skipping 74 matching lines @@
     set_result_identified();
   }
 
   Register ResultRegister() const { return result_register_; }
 
  private:
   Register result_register_;
 };
 
 
-BytecodeGenerator::AssignmentHazardHelper::AssignmentHazardHelper(
-    BytecodeGenerator* generator)
-    : generator_(generator),
-      alias_mappings_(generator->zone()),
-      aliased_locals_and_parameters_(generator->zone()),
-      execution_result_(nullptr),
-      scope_depth_(0) {}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::EnterScope() {
-  DCHECK_GE(scope_depth_, 0);
-  if (scope_depth_++ == 0) {
-    execution_result_ = generator_->execution_result();
-  }
-}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::LeaveScope() {
-  DCHECK_GT(scope_depth_, 0);
-  if (--scope_depth_ == 0) {
-    DCHECK_EQ(execution_result_, generator_->execution_result());
-    RestoreAliasedLocalsAndParameters();
-  }
-}
-
-
-// Returns a register that a load instruction should use when
-// loading from |reg|. This allows an alias for a modified version
-// of |reg| to be used within a hazard regions.
-MUST_USE_RESULT Register
-BytecodeGenerator::AssignmentHazardHelper::GetRegisterForLoad(Register reg) {
-  if (scope_depth_ == 0) {
-    return reg;
-  } else {
-    // A load from |reg| is to be issued. The register is placed in
-    // the mappings table initially mapping to itself. Future stores
-    // will update the mapping with temporaries thus preserving the
-    // original register's value.
-    //
-    // NB This insert only updates the table if no mapping exists
-    // already (std::map::insert semantics).
-    auto insert_result =
-        alias_mappings_.insert(std::make_pair(reg.index(), reg.index()));
-    auto mapping = insert_result.first;
-    // Return the current alias for reg.
-    return Register(mapping->second);
-  }
-}
-
-
-// Returns a register that a store instruction should use when
-// loading from |reg|. This allows an alias for a modified version
-// of |reg| to be used within hazard regions.
-MUST_USE_RESULT Register
-BytecodeGenerator::AssignmentHazardHelper::GetRegisterForStore(Register reg) {
-  if (scope_depth_ == 0 ||
-      alias_mappings_.find(reg.index()) == alias_mappings_.end()) {
-    // If not in a hazard region or a load for this register has not
-    // occurred no mapping is necessary.
-    return reg;
-  } else {
-    // Storing to a register with 1 or more loads issued. The
-    // register is mapped to a temporary alias so we don't overwrite
-    // the lhs value, e.g.  y = x + (x = 1); has a register for x on
-    // the lhs and needs a new register x' for the upcoming store on
-    // the rhs as the original x is an input to the add operation.
-    Register alias = execution_result_->NewRegister();
-    alias_mappings_[reg.index()] = alias.index();
-    if (generator_->builder()->RegisterIsParameterOrLocal(reg)) {
-      // Keep track of registers that need to be restored on exit
-      // from the assignment hazard region.
-      aliased_locals_and_parameters_.insert(reg.index());
-    }
-    return alias;
-  }
-}
-
-
-void BytecodeGenerator::AssignmentHazardHelper::
-    RestoreAliasedLocalsAndParameters() {
-  DCHECK(scope_depth_ == 0);
-  // Move temporary registers holding values for locals and
-  // parameters back into their local and parameter registers.
-  for (auto reg = aliased_locals_and_parameters_.begin();
-       reg != aliased_locals_and_parameters_.end(); reg++) {
-    auto mapping = alias_mappings_.find(*reg);
-    if (mapping != alias_mappings_.end()) {
-      generator_->builder()->MoveRegister(Register(mapping->second),
-                                          Register(*reg));
-    }
-  }
-  alias_mappings_.clear();
-  aliased_locals_and_parameters_.clear();
-}
-
-
-class BytecodeGenerator::AssignmentHazardScope final {
- public:
-  explicit AssignmentHazardScope(BytecodeGenerator* generator)
-      : generator_(generator) {
-    generator_->assignment_hazard_helper()->EnterScope();
-  }
-
-  ~AssignmentHazardScope() {
-    generator_->assignment_hazard_helper()->LeaveScope();
-  }
-
- private:
-  BytecodeGenerator* generator_;
-
-  DISALLOW_COPY_AND_ASSIGN(AssignmentHazardScope);
-};
-
-
 BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone)
     : isolate_(isolate),
       zone_(zone),
       builder_(isolate, zone),
       info_(nullptr),
       scope_(nullptr),
       globals_(0, zone),
       execution_control_(nullptr),
       execution_context_(nullptr),
-      execution_result_(nullptr),
-      assignment_hazard_helper_(this) {
+      execution_result_(nullptr) {
   InitializeAstVisitor(isolate);
 }
 
 
 Handle<BytecodeArray> BytecodeGenerator::MakeBytecode(CompilationInfo* info) {
   set_info(info);
   set_scope(info->scope());
 
   // Initialize the incoming context.
   ContextScope incoming_context(this, scope(), false);
@@ skipping 251 matching lines @@
   builder()->Return();
 }
 
 
 void BytecodeGenerator::VisitWithStatement(WithStatement* stmt) {
   UNIMPLEMENTED();
 }
 
 
 void BytecodeGenerator::VisitSwitchStatement(SwitchStatement* stmt) {
+  // We need this scope because we visit for register values. We have to
+  // maintain a execution result scope where registers can be allocated.
+  EffectResultScope switch_result_scope(this);

[rmcilroy, 2016/01/12 14:43:17]

nit - effect_results_scope

[mythria, 2016/01/12 16:53:21]

Done.

   ZoneList<CaseClause*>* clauses = stmt->cases();
   SwitchBuilder switch_builder(builder(), clauses->length());
   ControlScopeForBreakable scope(this, stmt, &switch_builder);
   int default_index = -1;
 
   // Keep the switch value in a register until a case matches.
   Register tag = VisitForRegisterValue(stmt->tag());
 
   // Iterate over all cases and create nodes for label comparison.
   BytecodeLabel done_label;
@@ skipping 553 matching lines @@
   VisitVariableLoad(proxy->var(), proxy->VariableFeedbackSlot());
 }
 
 
 void BytecodeGenerator::VisitVariableLoad(Variable* variable,
                                           FeedbackVectorSlot slot,
                                           TypeofMode typeof_mode) {
   switch (variable->location()) {
     case VariableLocation::LOCAL: {
       Register source(Register(variable->index()));
-      source = assignment_hazard_helper()->GetRegisterForLoad(source);
-      execution_result()->SetResultInRegister(source);
+      builder()->LoadAccumulatorWithRegister(source);
+      execution_result()->SetResultInAccumulator();
       break;
     }
     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);
-      source = assignment_hazard_helper()->GetRegisterForLoad(source);
-      execution_result()->SetResultInRegister(source);
+      builder()->LoadAccumulatorWithRegister(source);
+      execution_result()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
       builder()->LoadGlobal(variable->name(), feedback_index(slot),
                             language_mode(), typeof_mode);
       execution_result()->SetResultInAccumulator();
       break;
     }
     case VariableLocation::CONTEXT: {
@@ skipping 47 matching lines @@
   return register_scope.ResultRegister();
 }
 
 
 void BytecodeGenerator::VisitVariableAssignment(Variable* variable,
                                                 FeedbackVectorSlot slot) {
   switch (variable->location()) {
     case VariableLocation::LOCAL: {
       // TODO(rmcilroy): support const mode initialization.
       Register destination(variable->index());
-      destination =
-          assignment_hazard_helper()->GetRegisterForStore(destination);
       builder()->StoreAccumulatorInRegister(destination);
       break;
     }
     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));
-      destination =
-          assignment_hazard_helper()->GetRegisterForStore(destination);
-
       builder()->StoreAccumulatorInRegister(destination);
       break;
     }
     case VariableLocation::GLOBAL:
     case VariableLocation::UNALLOCATED: {
       builder()->StoreGlobal(variable->name(), feedback_index(slot),
                              language_mode());
       break;
     }
     case VariableLocation::CONTEXT: {
@@ skipping 580 matching lines @@
       VisitLogicalAndExpression(binop);
       break;
     default:
       VisitArithmeticExpression(binop);
       break;
   }
 }
 
 
 void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
-  // The evaluation of binary comparison expressions has an assignment
-  // hazard because the lhs may be a variable that evaluates to a
-  // local or parameter and the rhs may modify that, e.g. y = x + (x = 1)
-  // To get a correct result the generator treats the inner assigment
-  // as being made to a temporary x' that is spilled on exit of the
-  // assignment hazard.
-  AssignmentHazardScope assignment_hazard_scope(this);
-
   Register lhs = VisitForRegisterValue(expr->left());
   VisitForAccumulatorValue(expr->right());
   builder()->CompareOperation(expr->op(), lhs, language_mode_strength());
   execution_result()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitArithmeticExpression(BinaryOperation* expr) {
-  // The evaluation of binary arithmetic expressions has an assignment
-  // hazard because the lhs may be a variable that evaluates to a
-  // local or parameter and the rhs may modify that, e.g. y = x + (x = 1)
-  // To get a correct result the generator treats the inner assigment
-  // as being made to a temporary x' that is spilled on exit of the
-  // assignment hazard.
-  AssignmentHazardScope assignment_hazard_scope(this);
-
   Register lhs = VisitForRegisterValue(expr->left());
   VisitForAccumulatorValue(expr->right());
   builder()->BinaryOperation(expr->op(), lhs, language_mode_strength());
   execution_result()->SetResultInAccumulator();
 }
 
 
 void BytecodeGenerator::VisitSpread(Spread* expr) { UNREACHABLE(); }
 
 
@@ skipping 274 matching lines @@
 }
 
 
 int BytecodeGenerator::feedback_index(FeedbackVectorSlot slot) const {
   return info()->feedback_vector()->GetIndex(slot);
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8