[Interpreter] Fix deopt when accumulator needs to be materialized.

Moves the accumulator value on-heap to be restored in the
InterpreterNotifyDeopt handler rather than explicitly
setting the accumulator register. This allows it to be
materialized correctly if required.

BUG=v8:4678
LOG=N

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

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

src/arm/builtins-arm.cc

@@ -1203,19 +1203,18 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ mov(r1, Operand(Smi::FromInt(static_cast<int>(type))));
     __ push(r1);
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use this for interpreter deopts).
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register.
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
 
   // Enter the bytecode dispatch.
   Generate_EnterBytecodeDispatch(masm);

src/arm64/builtins-arm64.cc

@@ -1145,19 +1145,18 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ Push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ Mov(x1, Operand(Smi::FromInt(static_cast<int>(type))));
     __ Push(x1);
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ Pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use this for interpreter deopts).
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register.
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
 
   // Enter the bytecode dispatch.
   Generate_EnterBytecodeDispatch(masm);

src/compiler/code-generator.cc

@@ -591,7 +591,7 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
     case FrameStateType::kInterpretedFunction:
       translation->BeginInterpretedFrame(
           descriptor->bailout_id(), shared_info_id,
-          static_cast<unsigned int>(descriptor->locals_count()));
+          static_cast<unsigned int>(descriptor->locals_count() + 1));
       break;
     case FrameStateType::kArgumentsAdaptor:
       translation->BeginArgumentsAdaptorFrame(

src/deoptimizer.cc

@@ -1275,23 +1275,28 @@ void Deoptimizer::DoComputeInterpretedFrame(int frame_index,
                      "bytecode offset ");
 
   // Translate the rest of the interpreter registers in the frame.
-  for (unsigned i = 0; i < height; ++i) {
+  for (unsigned i = 0; i < height - 1; ++i) {
     output_offset -= kPointerSize;
     WriteTranslatedValueToOutput(&value_iterator, &input_index, frame_index,
                                  output_offset);
   }
-  CHECK_EQ(0u, output_offset);
 
-  // Set the accumulator register. If we are lazy deopting to a catch handler,
-  // we set the accumulator to the exception (which lives in the result
-  // register).
+  // Put the accumulator on the stack. It will be popped by the
+  // InterpreterNotifyDeopt builtin (possibly after materialization).
+  output_offset -= kPointerSize;
+  if (goto_catch_handler) {
+    // If we are lazy deopting to a catch handler, we set the accumulator to
+    // the exception (which lives in the result register).
     intptr_t accumulator_value =
-      goto_catch_handler
-          ? input_->GetRegister(FullCodeGenerator::result_register().code())
-          : reinterpret_cast<intptr_t>(value_iterator->GetRawValue());
-  output_frame->SetRegister(kInterpreterAccumulatorRegister.code(),
-                            accumulator_value);
+        input_->GetRegister(FullCodeGenerator::result_register().code());
+    WriteValueToOutput(reinterpret_cast<Object*>(accumulator_value), 0,
+                       frame_index, output_offset, "accumulator ");
     value_iterator++;
+  } else {
+    WriteTranslatedValueToOutput(&value_iterator, &input_index, frame_index,
+                                 output_offset);
+  }
+  CHECK_EQ(0u, output_offset);
 
   Builtins* builtins = isolate_->builtins();
   Code* dispatch_builtin =
@@ -2573,8 +2578,10 @@ DeoptimizedFrameInfo::DeoptimizedFrameInfo(TranslatedState* state,
 
   // Get the expression stack.
   int stack_height = frame_it->height();
-  if (frame_it->kind() == TranslatedFrame::kFunction) {
+  if (frame_it->kind() == TranslatedFrame::kFunction ||
+      frame_it->kind() == TranslatedFrame::kInterpretedFunction) {
     // For full-code frames, we should not count the context.
+    // For interpreter frames, we should not count the accumulator.
     // TODO(jarin): Clean up the indexing in translated frames.
     stack_height--;
   }
@@ -2586,7 +2593,7 @@ DeoptimizedFrameInfo::DeoptimizedFrameInfo(TranslatedState* state,
   }
 
   // For interpreter frame, skip the accumulator.
-  if (parameter_frame->kind() == TranslatedFrame::kInterpretedFunction) {
+  if (frame_it->kind() == TranslatedFrame::kInterpretedFunction) {
     stack_it++;
   }
   CHECK(stack_it == frame_it->end());
@@ -2953,8 +2960,8 @@ int TranslatedFrame::GetValueCount() {
     case kInterpretedFunction: {
       int parameter_count =
           raw_shared_info_->internal_formal_parameter_count() + 1;
-      // + 3 for function, context and accumulator.
-      return height_ + parameter_count + 3;
+      // + 2 for function and context.
+      return height_ + parameter_count + 2;
     }
 
     case kGetter:

src/ia32/builtins-ia32.cc

@@ -803,22 +803,20 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ Push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ Push(Smi::FromInt(static_cast<int>(type)));
-
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ Pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use these for interpreter deopts) and push PC at top
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register and push PC at top
   // of stack (to simulate initial call to bytecode handler in interpreter entry
   // trampoline).
   __ Pop(ebx);
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
   __ Push(ebx);
 
   // Enter the bytecode dispatch.

src/mips/builtins-mips.cc

@@ -1190,19 +1190,18 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ li(a1, Operand(Smi::FromInt(static_cast<int>(type))));
     __ push(a1);
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use this for interpreter deopts).
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register.
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
 
   // Enter the bytecode dispatch.
   Generate_EnterBytecodeDispatch(masm);

src/mips64/builtins-mips64.cc

@@ -1182,19 +1182,18 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ li(a1, Operand(Smi::FromInt(static_cast<int>(type))));
     __ push(a1);
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use this for interpreter deopts).
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register.
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
 
   // Enter the bytecode dispatch.
   Generate_EnterBytecodeDispatch(masm);

src/ppc/builtins-ppc.cc

@@ -1191,14 +1191,15 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
     // Save accumulator register and pass the deoptimization type to
     // the runtime system.
     __ LoadSmiLiteral(r4, Smi::FromInt(static_cast<int>(type)));
-    __ Push(kInterpreterAccumulatorRegister, r4);
+    __ Push(r4);
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-    __ pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use these for interpreter deopts).
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register.
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
 
   // Enter the bytecode dispatch.
   Generate_EnterBytecodeDispatch(masm);

src/x64/builtins-x64.cc

@@ -857,22 +857,20 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ Push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ Push(Smi::FromInt(static_cast<int>(type)));
-
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ Pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use these for interpreter deopts) and push PC at top
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register and push PC at top
   // of stack (to simulate initial call to bytecode handler in interpreter entry
   // trampoline).
   __ Pop(rbx);
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
   __ Push(rbx);
 
   // Enter the bytecode dispatch.

src/x87/builtins-x87.cc

@@ -804,22 +804,20 @@ static void Generate_InterpreterNotifyDeoptimizedHelper(
   // Enter an internal frame.
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
-    __ Push(kInterpreterAccumulatorRegister);  // Save accumulator register.
 
     // Pass the deoptimization type to the runtime system.
     __ Push(Smi::FromInt(static_cast<int>(type)));
-
     __ CallRuntime(Runtime::kNotifyDeoptimized);
-
-    __ Pop(kInterpreterAccumulatorRegister);  // Restore accumulator register.
     // Tear down internal frame.
   }
 
-  // Drop state (we don't use these for interpreter deopts) and push PC at top
+  // Drop state (we don't use these for interpreter deopts) and and pop the
+  // accumulator value into the accumulator register and push PC at top
   // of stack (to simulate initial call to bytecode handler in interpreter entry
   // trampoline).
   __ Pop(ebx);
   __ Drop(1);
+  __ Pop(kInterpreterAccumulatorRegister);
   __ Push(ebx);
 
   // Enter the bytecode dispatch.

test/mjsunit/compiler/deopt-materialize-accumulator.js

+// Copyright 2016 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+//
+// Tests that Turbofan correctly materializes values which are in the
+// interpreters accumulator during deopt.
+
+var global = 3;
+function f(a) {
+  // This will trigger a deopt since global was previously a SMI, with the
+  // accumulator holding an unboxed double which needs materialized.
+  global = %_MathSqrt(a);
+}
+%OptimizeFunctionOnNextCall(f);
+f(0.25);
+assertEquals(0.5, global);