[turbofan] Make the return value explicit in the deoptimization info.

With this change, the return value is not baked into the translations
for lazy deoptimization point. Instead, the translation contains
a position where the return value(s) should be written by
the deoptimizer. The deoptimizer then constructs the frame as it
would look before and during the call and then overwrites the relevant
slot(s) with the return value(s) from the callee.

The advantage is that we do not run the risk of overwriting captured
objects in the tranlations. Moreover, the translations correctly reflect
the frame during the call (e.g., if it is inspected by the debugger or
if an exception is thrown and no value is returned).

Bug: chromium:902608
Change-Id: I03824f0efec3dd476feacbcc0ab6102c3e6c94bf
Reviewed-on: https://chromium-review.googlesource.com/c/1329203
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57446}

src/compiler/backend/code-generator.cc

@@ -964,30 +964,12 @@ void CodeGenerator::TranslateStateValueDescriptor(
 
 void CodeGenerator::TranslateFrameStateDescriptorOperands(
     FrameStateDescriptor* desc, InstructionOperandIterator* iter,
-    OutputFrameStateCombine combine, Translation* translation) {
+    Translation* translation) {
   size_t index = 0;
   StateValueList* values = desc->GetStateValueDescriptors();
   for (StateValueList::iterator it = values->begin(); it != values->end();
        ++it, ++index) {
-    StateValueDescriptor* value_desc = (*it).desc;
-    if (!combine.IsOutputIgnored()) {
-      // The result of the call should be placed at position
-      // [index_from_top] in the stack (overwriting whatever was
-      // previously there).
-      size_t index_from_top = desc->GetSize() - 1 - combine.GetOffsetToPokeAt();
-      if (index >= index_from_top &&
-          index < index_from_top + iter->instruction()->OutputCount()) {
-        DCHECK_NOT_NULL(translation);
-        AddTranslationForOperand(
-            translation, iter->instruction(),
-            iter->instruction()->OutputAt(index - index_from_top),
-            MachineType::AnyTagged());
-        // Skip the instruction operands.
-        TranslateStateValueDescriptor(value_desc, (*it).nested, nullptr, iter);
-        continue;
-      }
-    }
-    TranslateStateValueDescriptor(value_desc, (*it).nested, translation, iter);
+    TranslateStateValueDescriptor((*it).desc, (*it).nested, translation, iter);
   }
   DCHECK_EQ(desc->GetSize(), index);
 }
@@ -998,8 +980,7 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
   // Outer-most state must be added to translation first.
   if (descriptor->outer_state() != nullptr) {
     BuildTranslationForFrameStateDescriptor(descriptor->outer_state(), iter,
-                                            translation,
-                                            OutputFrameStateCombine::Ignore());
+                                            translation, state_combine);
   }
 
   Handle<SharedFunctionInfo> shared_info;
@@ -1013,11 +994,19 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
       DefineDeoptimizationLiteral(DeoptimizationLiteral(shared_info));
 
   switch (descriptor->type()) {
-    case FrameStateType::kInterpretedFunction:
+    case FrameStateType::kInterpretedFunction: {
+      int return_offset = 0;
+      int return_count = 0;
+      if (!state_combine.IsOutputIgnored()) {
+        return_offset = static_cast<int>(state_combine.GetOffsetToPokeAt());
+        return_count = static_cast<int>(iter->instruction()->OutputCount());
+      }
       translation->BeginInterpretedFrame(
           descriptor->bailout_id(), shared_info_id,
-          static_cast<unsigned int>(descriptor->locals_count() + 1));
+          static_cast<unsigned int>(descriptor->locals_count() + 1),
+          return_offset, return_count);
       break;
+    }
     case FrameStateType::kArgumentsAdaptor:
       translation->BeginArgumentsAdaptorFrame(
           shared_info_id,
@@ -1055,8 +1044,7 @@ void CodeGenerator::BuildTranslationForFrameStateDescriptor(
     }
   }
 
-  TranslateFrameStateDescriptorOperands(descriptor, iter, state_combine,
-                                        translation);
+  TranslateFrameStateDescriptorOperands(descriptor, iter, translation);
 }
 
 int CodeGenerator::BuildTranslation(Instruction* instr, int pc_offset,

src/compiler/backend/code-generator.h

@@ -354,7 +354,6 @@ class CodeGenerator final : public GapResolver::Assembler {
                                      InstructionOperandIterator* iter);
   void TranslateFrameStateDescriptorOperands(FrameStateDescriptor* desc,
                                              InstructionOperandIterator* iter,
-                                             OutputFrameStateCombine combine,
                                              Translation* translation);
   void AddTranslationForOperand(Translation* translation, Instruction* instr,
                                 InstructionOperand* op, MachineType type);

src/deoptimizer.cc

@@ -953,10 +953,10 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
   // Read the context from the translations.
   Object* context = context_pos->GetRawValue();
   output_frame->SetContext(reinterpret_cast<intptr_t>(context));
-  frame_writer.PushTranslatedValue(context_pos, "context\n");
+  frame_writer.PushTranslatedValue(context_pos, "context");
 
   // The function was mentioned explicitly in the BEGIN_FRAME.
-  frame_writer.PushTranslatedValue(function_iterator, "function\n");
+  frame_writer.PushTranslatedValue(function_iterator, "function");
 
   // Set the bytecode array pointer.
   Object* bytecode_array = shared->HasBreakInfo()
@@ -975,9 +975,36 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
   }
 
   // Translate the rest of the interpreter registers in the frame.
+  // The return_value_offset is counted from the top. Here, we compute the
+  // register index (counted from the start).
+  int return_value_first_reg =
+      register_count - translated_frame->return_value_offset();
+  int return_value_count = translated_frame->return_value_count();
   for (int i = 0; i < register_count; ++i, ++value_iterator) {
+    // Ensure we write the return value if we have one and we are returning
+    // normally to a lazy deopt point.
+    if (is_topmost && !goto_catch_handler &&
+        deopt_kind_ == DeoptimizeKind::kLazy && i >= return_value_first_reg &&
+        i < return_value_first_reg + return_value_count) {
+      int return_index = i - return_value_first_reg;
+      if (return_index == 0) {
+        frame_writer.PushRawValue(input_->GetRegister(kReturnRegister0.code()),
+                                  "return value 0\n");
+        // We do not handle the situation when one return value should go into
+        // the accumulator and another one into an ordinary register. Since
+        // the interpreter should never create such situation, just assert
+        // this does not happen.
+        CHECK_LE(return_value_first_reg + return_value_count, register_count);
+      } else {
+        CHECK_EQ(return_index, 1);
+        frame_writer.PushRawValue(input_->GetRegister(kReturnRegister1.code()),
+                                  "return value 1\n");
+      }
+    } else {
+      // This is not return value, just write the value from the translations.
       frame_writer.PushTranslatedValue(value_iterator, "stack parameter");
     }
+  }
 
   int register_slots_written = register_count;
   DCHECK_LE(register_slots_written, register_stack_slot_count);
@@ -1003,10 +1030,20 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
           input_->GetRegister(kInterpreterAccumulatorRegister.code());
       frame_writer.PushRawObject(reinterpret_cast<Object*>(accumulator_value),
                                  "accumulator\n");
-      ++value_iterator;  // Skip the accumulator.
     } else {
-      frame_writer.PushTranslatedValue(value_iterator++, "accumulator");
+      // If we are lazily deoptimizing make sure we store the deopt
+      // return value into the appropriate slot.
+      if (deopt_kind_ == DeoptimizeKind::kLazy &&
+          translated_frame->return_value_offset() == 0 &&
+          translated_frame->return_value_count() > 0) {
+        CHECK_EQ(translated_frame->return_value_count(), 1);
+        frame_writer.PushRawValue(input_->GetRegister(kReturnRegister0.code()),
+                                  "return value 0\n");
+      } else {
+        frame_writer.PushTranslatedValue(value_iterator, "accumulator");
       }
+    }
+    ++value_iterator;  // Move over the accumulator.
   } else {
     // For non-topmost frames, skip the accumulator translation. For those
     // frames, the return value from the callee will become the accumulator.
@@ -1255,7 +1292,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
   intptr_t marker = StackFrame::TypeToMarker(StackFrame::CONSTRUCT);
   frame_writer.PushRawValue(marker, "context (construct stub sentinel)\n");
 
-  frame_writer.PushTranslatedValue(value_iterator++, "context\n");
+  frame_writer.PushTranslatedValue(value_iterator++, "context");
 
   // Number of incoming arguments.
   frame_writer.PushRawObject(Smi::FromInt(parameter_count - 1), "argc\n");
@@ -1958,11 +1995,15 @@ void Translation::BeginArgumentsAdaptorFrame(int literal_id, unsigned height) {
 }
 
 void Translation::BeginInterpretedFrame(BailoutId bytecode_offset,
-                                        int literal_id, unsigned height) {
+                                        int literal_id, unsigned height,
+                                        int return_value_offset,
+                                        int return_value_count) {
   buffer_->Add(INTERPRETED_FRAME);
   buffer_->Add(bytecode_offset.ToInt());
   buffer_->Add(literal_id);
   buffer_->Add(height);
+  buffer_->Add(return_value_offset);
+  buffer_->Add(return_value_count);
 }
 
 void Translation::ArgumentsElements(CreateArgumentsType type) {
@@ -2106,12 +2147,13 @@ int Translation::NumberOfOperandsFor(Opcode opcode) {
     case UPDATE_FEEDBACK:
       return 2;
     case BEGIN:
-    case INTERPRETED_FRAME:
     case CONSTRUCT_STUB_FRAME:
     case BUILTIN_CONTINUATION_FRAME:
     case JAVA_SCRIPT_BUILTIN_CONTINUATION_FRAME:
     case JAVA_SCRIPT_BUILTIN_CONTINUATION_WITH_CATCH_FRAME:
       return 3;
+    case INTERPRETED_FRAME:
+      return 5;
   }
   FATAL("Unexpected translation type");
   return -1;
@@ -2659,10 +2701,11 @@ void TranslatedValue::Handlify() {
   }
 }
 
-
 TranslatedFrame TranslatedFrame::InterpretedFrame(
-    BailoutId bytecode_offset, SharedFunctionInfo* shared_info, int height) {
-  TranslatedFrame frame(kInterpretedFunction, shared_info, height);
+    BailoutId bytecode_offset, SharedFunctionInfo* shared_info, int height,
+    int return_value_offset, int return_value_count) {
+  TranslatedFrame frame(kInterpretedFunction, shared_info, height,
+                        return_value_offset, return_value_count);
   frame.node_id_ = bytecode_offset;
   return frame;
 }
@@ -2749,16 +2792,21 @@ TranslatedFrame TranslatedState::CreateNextTranslatedFrame(
       SharedFunctionInfo* shared_info =
           SharedFunctionInfo::cast(literal_array->get(iterator->Next()));
       int height = iterator->Next();
+      int return_value_offset = iterator->Next();
+      int return_value_count = iterator->Next();
       if (trace_file != nullptr) {
         std::unique_ptr<char[]> name = shared_info->DebugName()->ToCString();
         PrintF(trace_file, "  reading input frame %s", name.get());
         int arg_count = shared_info->internal_formal_parameter_count() + 1;
         PrintF(trace_file,
-               " => bytecode_offset=%d, args=%d, height=%d; inputs:\n",
-               bytecode_offset.ToInt(), arg_count, height);
+               " => bytecode_offset=%d, args=%d, height=%d, retval=%i(#%i); "
+               "inputs:\n",
+               bytecode_offset.ToInt(), arg_count, height, return_value_offset,
+               return_value_count);
       }
       return TranslatedFrame::InterpretedFrame(bytecode_offset, shared_info,
-                                               height);
+                                               height, return_value_offset,
+                                               return_value_count);
     }
 
     case Translation::ARGUMENTS_ADAPTOR_FRAME: {

src/deoptimizer.h

@@ -170,6 +170,8 @@ class TranslatedFrame {
   BailoutId node_id() const { return node_id_; }
   Handle<SharedFunctionInfo> shared_info() const { return shared_info_; }
   int height() const { return height_; }
+  int return_value_offset() const { return return_value_offset_; }
+  int return_value_count() const { return return_value_count_; }
 
   SharedFunctionInfo* raw_shared_info() const {
     CHECK_NOT_NULL(raw_shared_info_);
@@ -185,8 +187,8 @@ class TranslatedFrame {
     }
 
     iterator operator++(int) {
+      iterator original(position_, input_index_);
       ++input_index_;
-      iterator original(position_);
       AdvanceIterator(&position_);
       return original;
     }
@@ -207,8 +209,9 @@ class TranslatedFrame {
    private:
     friend TranslatedFrame;
 
-    explicit iterator(std::deque<TranslatedValue>::iterator position)
-        : position_(position), input_index_(0) {}
+    explicit iterator(std::deque<TranslatedValue>::iterator position,
+                      int input_index = 0)
+        : position_(position), input_index_(input_index) {}
 
     std::deque<TranslatedValue>::iterator position_;
     int input_index_;
@@ -229,7 +232,8 @@ class TranslatedFrame {
   // Constructor static methods.
   static TranslatedFrame InterpretedFrame(BailoutId bytecode_offset,
                                           SharedFunctionInfo* shared_info,
-                                          int height);
+                                          int height, int return_value_offset,
+                                          int return_value_count);
   static TranslatedFrame AccessorFrame(Kind kind,
                                        SharedFunctionInfo* shared_info);
   static TranslatedFrame ArgumentsAdaptorFrame(SharedFunctionInfo* shared_info,
@@ -250,11 +254,14 @@ class TranslatedFrame {
   static void AdvanceIterator(std::deque<TranslatedValue>::iterator* iter);
 
   TranslatedFrame(Kind kind, SharedFunctionInfo* shared_info = nullptr,
-                  int height = 0)
+                  int height = 0, int return_value_offset = 0,
+                  int return_value_count = 0)
       : kind_(kind),
         node_id_(BailoutId::None()),
         raw_shared_info_(shared_info),
-        height_(height) {}
+        height_(height),
+        return_value_offset_(return_value_offset),
+        return_value_count_(return_value_count) {}
 
   void Add(const TranslatedValue& value) { values_.push_back(value); }
   TranslatedValue* ValueAt(int index) { return &(values_[index]); }
@@ -265,6 +272,8 @@ class TranslatedFrame {
   SharedFunctionInfo* raw_shared_info_;
   Handle<SharedFunctionInfo> shared_info_;
   int height_;
+  int return_value_offset_;
+  int return_value_count_;
 
   typedef std::deque<TranslatedValue> ValuesContainer;
 
@@ -950,7 +959,8 @@ class Translation {
 
   // Commands.
   void BeginInterpretedFrame(BailoutId bytecode_offset, int literal_id,
-                             unsigned height);
+                             unsigned height, int return_value_offset,
+                             int return_value_count);
   void BeginArgumentsAdaptorFrame(int literal_id, unsigned height);
   void BeginConstructStubFrame(BailoutId bailout_id, int literal_id,
                                unsigned height);

src/objects.cc

@@ -14790,10 +14790,13 @@ void DeoptimizationData::DeoptimizationDataPrint(std::ostream& os) {  // NOLINT
           int bytecode_offset = iterator.Next();
           int shared_info_id = iterator.Next();
           unsigned height = iterator.Next();
+          int return_value_offset = iterator.Next();
+          int return_value_count = iterator.Next();
           Object* shared_info = LiteralArray()->get(shared_info_id);
           os << "{bytecode_offset=" << bytecode_offset << ", function="
              << Brief(SharedFunctionInfo::cast(shared_info)->DebugName())
-             << ", height=" << height << "}";
+             << ", height=" << height << ", retval=@" << return_value_offset
+             << "(#" << return_value_count << ")}";
           break;
         }
 

src/profiler/profiler-listener.cc

@@ -226,6 +226,8 @@ void ProfilerListener::RecordInliningInfo(CodeEntry* entry,
       it.Next();  // Skip ast_id
       int shared_info_id = it.Next();
       it.Next();  // Skip height
+      it.Next();  // Skip return value offset
+      it.Next();  // Skip return value count
       SharedFunctionInfo* shared_info = SharedFunctionInfo::cast(
           deopt_input_data->LiteralArray()->get(shared_info_id));
       if (!depth++) continue;  // Skip the current function itself.