[Interpreter] Always store current context in the frames context slot.

Change the interpreter to always store the current context in the frame's
context slot instead of the function context. This makes it possible to
restore the correct context during deopt.

BUG=v8:4678,v8:4280
LOG=N

Review URL: https://codereview.chromium.org/1604923002

Cr-Commit-Position: refs/heads/master@{#33477}

src/arm/builtins-arm.cc

@@ -1069,8 +1069,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
          Operand(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ ldr(kContextRegister,
          MemOperand(kInterpreterRegisterFileRegister,
                     InterpreterFrameConstants::kContextFromRegisterPointer));

src/arm64/builtins-arm64.cc

@@ -1028,8 +1028,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
          Operand(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ Ldr(kContextRegister,
          MemOperand(kInterpreterRegisterFileRegister,
                     InterpreterFrameConstants::kContextFromRegisterPointer));

src/compiler/bytecode-graph-builder.cc

@@ -155,8 +155,8 @@ Node* BytecodeGraphBuilder::Environment::LookupAccumulator() const {
 
 Node* BytecodeGraphBuilder::Environment::LookupRegister(
     interpreter::Register the_register) const {
-  if (the_register.is_function_context()) {
-    return builder()->GetFunctionContext();
+  if (the_register.is_current_context()) {
+    return Context();
   } else if (the_register.is_function_closure()) {
     return builder()->GetFunctionClosure();
   } else if (the_register.is_new_target()) {
@@ -1034,9 +1034,10 @@ void BytecodeGraphBuilder::VisitKeyedStoreICStrictWide(
 
 void BytecodeGraphBuilder::VisitPushContext(
     const interpreter::BytecodeArrayIterator& iterator) {
-  Node* context = environment()->LookupAccumulator();
-  environment()->BindRegister(iterator.GetRegisterOperand(0), context);
-  environment()->SetContext(context);
+  Node* new_context = environment()->LookupAccumulator();
+  environment()->BindRegister(iterator.GetRegisterOperand(0),
+                              environment()->Context());
+  environment()->SetContext(new_context);
 }
 
 

src/compiler/interpreter-assembler.cc

@@ -80,7 +80,10 @@ void InterpreterAssembler::SetAccumulator(Node* value) { accumulator_ = value; }
 Node* InterpreterAssembler::GetContext() { return context_; }
 
 
-void InterpreterAssembler::SetContext(Node* value) { context_ = value; }
+void InterpreterAssembler::SetContext(Node* value) {
+  StoreRegister(value, interpreter::Register::current_context());
+  context_ = value;
+}
 
 
 Node* InterpreterAssembler::BytecodeOffset() { return bytecode_offset_; }

src/ia32/builtins-ia32.cc

@@ -800,8 +800,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
   __ mov(ebx, Operand(ebx, esi, times_pointer_size, 0));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ mov(kContextRegister,
          Operand(kInterpreterRegisterFileRegister,
                  InterpreterFrameConstants::kContextFromRegisterPointer));

src/interpreter/bytecode-array-builder.cc

@@ -1341,7 +1341,7 @@ bool BytecodeArrayBuilder::RegisterIsValid(Register reg,
       return false;
   }
 
-  if (reg.is_function_context() || reg.is_function_closure() ||
+  if (reg.is_current_context() || reg.is_function_closure() ||
       reg.is_new_target()) {
     return true;
   } else if (reg.is_parameter()) {

src/interpreter/bytecode-generator.cc

@@ -27,19 +27,26 @@ class BytecodeGenerator::ContextScope BASE_EMBEDDED {
       : generator_(generator),
         scope_(scope),
         outer_(generator_->execution_context()),
-        register_(generator_->NextContextRegister()),
+        register_(Register::current_context()),
         depth_(0),
         should_pop_context_(should_pop_context) {
     if (outer_) {
       depth_ = outer_->depth_ + 1;
-      generator_->builder()->PushContext(register_);
+
+      // Push the outer context into a new context register.
+      Register outer_context_reg(builder()->first_context_register().index() +
+                                 outer_->depth_);
+      outer_->set_register(outer_context_reg);
+      generator_->builder()->PushContext(outer_context_reg);
     }
     generator_->set_execution_context(this);
   }
 
   ~ContextScope() {
     if (outer_ && should_pop_context_) {
+      DCHECK_EQ(register_.index(), Register::current_context().index());
       generator_->builder()->PopContext(outer_->reg());
+      outer_->set_register(register_);
     }
     generator_->set_execution_context(outer_);
   }
@@ -67,6 +74,10 @@ class BytecodeGenerator::ContextScope BASE_EMBEDDED {
   Register reg() const { return register_; }
 
  private:
+  const BytecodeArrayBuilder* builder() const { return generator_->builder(); }
+
+  void set_register(Register reg) { register_ = reg; }
+
   BytecodeGenerator* generator_;
   Scope* scope_;
   ContextScope* outer_;
@@ -1866,7 +1877,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
         DCHECK(Register::AreContiguous(callee, receiver));
         Variable* variable = callee_expr->AsVariableProxy()->var();
         builder()
-            ->MoveRegister(execution_context()->reg(), context)
+            ->MoveRegister(Register::current_context(), context)
             .LoadLiteral(variable->name())
             .StoreAccumulatorInRegister(name)
             .CallRuntimeForPair(Runtime::kLoadLookupSlot, context, 2, callee);
@@ -2480,24 +2491,6 @@ void BytecodeGenerator::VisitInScope(Statement* stmt, Scope* scope) {
 }
 
 
-Register BytecodeGenerator::NextContextRegister() const {
-  if (execution_context() == nullptr) {
-    // Return the incoming function context for the outermost execution context.
-    return Register::function_context();
-  }
-  Register previous = execution_context()->reg();
-  if (previous == Register::function_context()) {
-    // If the previous context was the incoming function context, then the next
-    // context register is the first local context register.
-    return builder_.first_context_register();
-  } else {
-    // Otherwise use the next local context register.
-    DCHECK_LT(previous.index(), builder_.last_context_register().index());
-    return Register(previous.index() + 1);
-  }
-}
-
-
 LanguageMode BytecodeGenerator::language_mode() const {
   return info()->language_mode();
 }

src/interpreter/bytecode-generator.h

@@ -42,7 +42,6 @@ class BytecodeGenerator final : public AstVisitor {
   class RegisterAllocationScope;
 
   void MakeBytecodeBody();
-  Register NextContextRegister() const;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 

src/interpreter/bytecodes.cc

@@ -294,7 +294,7 @@ std::ostream& Bytecodes::Decode(std::ostream& os, const uint8_t* bytecode_start,
       case interpreter::OperandType::kReg8:
       case interpreter::OperandType::kReg16: {
         Register reg = DecodeRegister(operand_start, op_type);
-        if (reg.is_function_context()) {
+        if (reg.is_current_context()) {
           os << "<context>";
         } else if (reg.is_function_closure()) {
           os << "<closure>";
@@ -358,7 +358,7 @@ static const int kLastParamRegisterIndex =
     -InterpreterFrameConstants::kLastParamFromRegisterPointer / kPointerSize;
 static const int kFunctionClosureRegisterIndex =
     -InterpreterFrameConstants::kFunctionFromRegisterPointer / kPointerSize;
-static const int kFunctionContextRegisterIndex =
+static const int kCurrentContextRegisterIndex =
     -InterpreterFrameConstants::kContextFromRegisterPointer / kPointerSize;
 static const int kNewTargetRegisterIndex =
     -InterpreterFrameConstants::kNewTargetFromRegisterPointer / kPointerSize;
@@ -397,13 +397,13 @@ bool Register::is_function_closure() const {
 }
 
 
-Register Register::function_context() {
-  return Register(kFunctionContextRegisterIndex);
+Register Register::current_context() {
+  return Register(kCurrentContextRegisterIndex);
 }
 
 
-bool Register::is_function_context() const {
-  return index() == kFunctionContextRegisterIndex;
+bool Register::is_current_context() const {
+  return index() == kCurrentContextRegisterIndex;
 }
 
 

src/interpreter/bytecodes.h

@@ -308,9 +308,9 @@ class Register {
   static Register function_closure();
   bool is_function_closure() const;
 
-  // Returns the register for the function's outer context.
-  static Register function_context();
-  bool is_function_context() const;
+  // Returns the register which holds the current context object.
+  static Register current_context();
+  bool is_current_context() const;
 
   // Returns the register for the incoming new target value.
   static Register new_target();

src/interpreter/interpreter.cc

@@ -816,12 +816,14 @@ void Interpreter::DoKeyedStoreICStrictWide(
 
 // PushContext <context>
 //
-// Pushes the accumulator as the current context, and saves it in <context>
+// Saves the current context in <context>, and pushes the accumulator as the
+// new current context.
 void Interpreter::DoPushContext(compiler::InterpreterAssembler* assembler) {
   Node* reg_index = __ BytecodeOperandReg(0);
-  Node* context = __ GetAccumulator();
-  __ SetContext(context);
-  __ StoreRegister(context, reg_index);
+  Node* new_context = __ GetAccumulator();
+  Node* old_context = __ GetContext();
+  __ StoreRegister(old_context, reg_index);
+  __ SetContext(new_context);
   __ Dispatch();
 }
 

src/mips/builtins-mips.cc

@@ -1058,8 +1058,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
           Operand(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ lw(kContextRegister,
         MemOperand(kInterpreterRegisterFileRegister,
                    InterpreterFrameConstants::kContextFromRegisterPointer));

src/mips64/builtins-mips64.cc

@@ -1049,8 +1049,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
            Operand(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ ld(kContextRegister,
         MemOperand(kInterpreterRegisterFileRegister,
                    InterpreterFrameConstants::kContextFromRegisterPointer));

src/ppc/builtins-ppc.cc

@@ -1058,8 +1058,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
           Operand(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ LoadP(kContextRegister,
            MemOperand(kInterpreterRegisterFileRegister,
                       InterpreterFrameConstants::kContextFromRegisterPointer));

src/x64/builtins-x64.cc

@@ -821,8 +821,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
           Immediate(FixedArray::kHeaderSize - kHeapObjectTag));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ movp(kContextRegister,
           Operand(kInterpreterRegisterFileRegister,
                   InterpreterFrameConstants::kContextFromRegisterPointer));

src/x87/builtins-x87.cc

@@ -801,8 +801,6 @@ static void Generate_EnterBytecodeDispatch(MacroAssembler* masm) {
   __ mov(ebx, Operand(ebx, esi, times_pointer_size, 0));
 
   // Get the context from the frame.
-  // TODO(rmcilroy): Update interpreter frame to expect current context at the
-  // context slot instead of the function context.
   __ mov(kContextRegister,
          Operand(kInterpreterRegisterFileRegister,
                  InterpreterFrameConstants::kContextFromRegisterPointer));

test/mjsunit/deopt-with-outer-context.js

+// 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.
+
+// Flags: --allow-natives-syntax
+
+function outer(y) {
+  function inner() {
+    var x = 10;
+    (function() {
+       // Access x from inner function to force it to be context allocated.
+       x = 20;
+       %DeoptimizeFunction(inner);
+    })();
+    // Variable y should be read from the outer context.
+    return y;
+  };
+  %OptimizeFunctionOnNextCall(inner);
+  return inner();
+}
+
+assertEquals(30, outer(30));

test/unittests/compiler/interpreter-assembler-unittest.cc

@@ -409,6 +409,16 @@ TARGET_TEST_F(InterpreterAssemblerTest, GetSetAccumulator) {
 }
 
 
+TARGET_TEST_F(InterpreterAssemblerTest, GetSetContext) {
+  TRACED_FOREACH(interpreter::Bytecode, bytecode, kBytecodes) {
+    InterpreterAssemblerForTest m(this, bytecode);
+    Node* context_node = m.Int32Constant(100);
+    m.SetContext(context_node);
+    EXPECT_THAT(m.GetContext(), context_node);
+  }
+}
+
+
 TARGET_TEST_F(InterpreterAssemblerTest, RegisterLocation) {
   TRACED_FOREACH(interpreter::Bytecode, bytecode, kBytecodes) {
     InterpreterAssemblerForTest m(this, bytecode);