Reland "[compiler] Track the maximal unoptimized frame size"

This is a reland of 1e472c42

No change, this was a speculative revert to unblock the roll.

TBR=jgruber

Original change's description:
> [compiler] Track the maximal unoptimized frame size
>
> This is another step towards considering the unoptimized frame size in
> stack checks within optimized code.
>
> With the changes in this CL, we now keep track of the maximal
> unoptimized frame size of the function that is currently being
> compiled. An optimized function may inline multiple unoptimized
> functions, so a single optimized frame can deopt to multiple
> frames. The real frame size thus differs in different parts of the
> optimized function.
>
> We only care about the maximal frame size, which we calculate
> conservatively as an over-approximation, and track in
> InstructionSelector::max_unoptimized_frame_height_ for now. In future
> work, this value will be passed on to codegen, where it will be
> applied as an offset to the stack pointer during the stack check.
>
> (The motivation behind this is to avoid stack overflows through deopts,
> caused by size differences between optimized and unoptimized frames.)
>
> Note that this offset only ensure that the topmost optimized frame can
> deopt without overflowing the stack limit. That's fine, because we only
> deopt optimized frames one at a time. Other (non-topmost) frames are
> only deoptimized once they are returned to.
>
> Drive-by: Print variable and total frame height in --trace-deopt.
>
> Bug: v8:9534
> Change-Id: I821684a9da93bff59c20c8ab226105e7e12d93eb
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1762024
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Auto-Submit: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63330}

Bug: v8:9534
Change-Id: I686f200e7be1f419e23e50789e11607a0b2886d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1766645
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63356}

src/compiler/backend/instruction-selector.cc

@@ -3039,8 +3039,9 @@ bool InstructionSelector::CanProduceSignalingNaN(Node* node) {
   return true;
 }
 
-FrameStateDescriptor* InstructionSelector::GetFrameStateDescriptor(
-    Node* state) {
+namespace {
+
+FrameStateDescriptor* GetFrameStateDescriptorInternal(Zone* zone, Node* state) {
   DCHECK_EQ(IrOpcode::kFrameState, state->opcode());
   DCHECK_EQ(kFrameStateInputCount, state->InputCount());
   FrameStateInfo state_info = FrameStateInfoOf(state->op());
@@ -3058,13 +3059,24 @@ FrameStateDescriptor* InstructionSelector::GetFrameStateDescriptor(
   FrameStateDescriptor* outer_state = nullptr;
   Node* outer_node = state->InputAt(kFrameStateOuterStateInput);
   if (outer_node->opcode() == IrOpcode::kFrameState) {
-    outer_state = GetFrameStateDescriptor(outer_node);
+    outer_state = GetFrameStateDescriptorInternal(zone, outer_node);
   }
 
-  return new (instruction_zone()) FrameStateDescriptor(
-      instruction_zone(), state_info.type(), state_info.bailout_id(),
-      state_info.state_combine(), parameters, locals, stack,
-      state_info.shared_info(), outer_state);
+  return new (zone)
+      FrameStateDescriptor(zone, state_info.type(), state_info.bailout_id(),
+                           state_info.state_combine(), parameters, locals,
+                           stack, state_info.shared_info(), outer_state);
+}
+
+}  // namespace
+
+FrameStateDescriptor* InstructionSelector::GetFrameStateDescriptor(
+    Node* state) {
+  auto* desc = GetFrameStateDescriptorInternal(instruction_zone(), state);
+  max_unoptimized_frame_height_ =
+      std::max(max_unoptimized_frame_height_,
+               desc->total_conservative_frame_size_in_bytes());
+  return desc;
 }
 
 // static

src/compiler/backend/instruction-selector.h

@@ -783,6 +783,10 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
   ZoneVector<std::pair<int, int>> instr_origins_;
   EnableTraceTurboJson trace_turbo_;
   TickCounter* const tick_counter_;
+
+  // Store the maximal unoptimized frame height. Later used to apply an offset
+  // to stack checks.
+  size_t max_unoptimized_frame_height_ = 0;
 };
 
 }  // namespace compiler

src/compiler/backend/instruction.cc

@@ -6,12 +6,14 @@
 
 #include <iomanip>
 
+#include "src/codegen/interface-descriptors.h"
 #include "src/codegen/register-configuration.h"
 #include "src/codegen/source-position.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/graph.h"
 #include "src/compiler/schedule.h"
 #include "src/compiler/state-values-utils.h"
+#include "src/execution/frames.h"
 
 namespace v8 {
 namespace internal {
@@ -1002,6 +1004,59 @@ void InstructionSequence::SetRegisterConfigurationForTesting(
   GetRegConfig = InstructionSequence::RegisterConfigurationForTesting;
 }
 
+namespace {
+
+size_t GetConservativeFrameSizeInBytes(FrameStateType type,
+                                       size_t parameters_count,
+                                       size_t locals_count,
+                                       BailoutId bailout_id) {
+  switch (type) {
+    case FrameStateType::kInterpretedFunction: {
+      auto info = InterpretedFrameInfo::Conservative(
+          static_cast<int>(parameters_count), static_cast<int>(locals_count));
+      return info.frame_size_in_bytes();
+    }
+    case FrameStateType::kArgumentsAdaptor: {
+      auto info = ArgumentsAdaptorFrameInfo::Conservative(
+          static_cast<int>(parameters_count));
+      return info.frame_size_in_bytes();
+    }
+    case FrameStateType::kConstructStub: {
+      auto info = ConstructStubFrameInfo::Conservative(
+          static_cast<int>(parameters_count));
+      return info.frame_size_in_bytes();
+    }
+    case FrameStateType::kBuiltinContinuation:
+    case FrameStateType::kJavaScriptBuiltinContinuation:
+    case FrameStateType::kJavaScriptBuiltinContinuationWithCatch: {
+      const RegisterConfiguration* config = RegisterConfiguration::Default();
+      auto info = BuiltinContinuationFrameInfo::Conservative(
+          static_cast<int>(parameters_count),
+          Builtins::CallInterfaceDescriptorFor(
+              Builtins::GetBuiltinFromBailoutId(bailout_id)),
+          config);
+      return info.frame_size_in_bytes();
+    }
+  }
+  UNREACHABLE();
+}
+
+size_t GetTotalConservativeFrameSizeInBytes(FrameStateType type,
+                                            size_t parameters_count,
+                                            size_t locals_count,
+                                            BailoutId bailout_id,
+                                            FrameStateDescriptor* outer_state) {
+  size_t outer_total_conservative_frame_size_in_bytes =
+      (outer_state == nullptr)
+          ? 0
+          : outer_state->total_conservative_frame_size_in_bytes();
+  return GetConservativeFrameSizeInBytes(type, parameters_count, locals_count,
+                                         bailout_id) +
+         outer_total_conservative_frame_size_in_bytes;
+}
+
+}  // namespace
+
 FrameStateDescriptor::FrameStateDescriptor(
     Zone* zone, FrameStateType type, BailoutId bailout_id,
     OutputFrameStateCombine state_combine, size_t parameters_count,
@@ -1014,6 +1069,9 @@ FrameStateDescriptor::FrameStateDescriptor(
       parameters_count_(parameters_count),
       locals_count_(locals_count),
       stack_count_(stack_count),
+      total_conservative_frame_size_in_bytes_(
+          GetTotalConservativeFrameSizeInBytes(
+              type, parameters_count, locals_count, bailout_id, outer_state)),
       values_(zone),
       shared_info_(shared_info),
       outer_state_(outer_state) {}

src/compiler/backend/instruction.h

@@ -1277,6 +1277,13 @@ class FrameStateDescriptor : public ZoneObject {
   // slot kinds depend on the frame type.
   size_t GetHeight() const;
 
+  // Returns an overapproximation of the unoptimized stack frame size in bytes,
+  // as later produced by the deoptimizer. Considers both this and the chain of
+  // outer states.
+  size_t total_conservative_frame_size_in_bytes() const {
+    return total_conservative_frame_size_in_bytes_;
+  }
+
   size_t GetSize() const;
   size_t GetTotalSize() const;
   size_t GetFrameCount() const;
@@ -1290,12 +1297,13 @@ class FrameStateDescriptor : public ZoneObject {
   FrameStateType type_;
   BailoutId bailout_id_;
   OutputFrameStateCombine frame_state_combine_;
-  size_t parameters_count_;
-  size_t locals_count_;
-  size_t stack_count_;
+  const size_t parameters_count_;
+  const size_t locals_count_;
+  const size_t stack_count_;
+  const size_t total_conservative_frame_size_in_bytes_;
   StateValueList values_;
   MaybeHandle<SharedFunctionInfo> const shared_info_;
-  FrameStateDescriptor* outer_state_;
+  FrameStateDescriptor* const outer_state_;
 };
 
 // A deoptimization entry is a pair of the reason why we deoptimize and the

src/deoptimizer/deoptimizer.cc

@@ -824,9 +824,10 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
     PrintF(trace_scope_->file(), "  translating interpreted frame ");
     std::unique_ptr<char[]> name = shared.DebugName().ToCString();
     PrintF(trace_scope_->file(), "%s", name.get());
-    PrintF(trace_scope_->file(), " => bytecode_offset=%d, height=%d%s\n",
+    PrintF(trace_scope_->file(),
+           " => bytecode_offset=%d, variable_frame_size=%d, frame_size=%d%s\n",
            real_bytecode_offset, frame_info.frame_size_in_bytes_without_fixed(),
-           goto_catch_handler ? " (throw)" : "");
+           output_frame_size, goto_catch_handler ? " (throw)" : "");
   }
 
   // Allocate and store the output frame description.
@@ -1062,16 +1063,17 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(
   const int parameters_count = translated_frame->height();
   ArgumentsAdaptorFrameInfo frame_info =
       ArgumentsAdaptorFrameInfo::Precise(parameters_count);
+  const uint32_t output_frame_size = frame_info.frame_size_in_bytes();
 
   TranslatedFrame::iterator function_iterator = value_iterator++;
   if (trace_scope_ != nullptr) {
     PrintF(trace_scope_->file(),
-           "  translating arguments adaptor => height=%d\n",
-           frame_info.frame_size_in_bytes_without_fixed());
+           "  translating arguments adaptor => variable_frame_size=%d, "
+           "frame_size=%d\n",
+           frame_info.frame_size_in_bytes_without_fixed(), output_frame_size);
   }
 
   // Allocate and store the output frame description.
-  const uint32_t output_frame_size = frame_info.frame_size_in_bytes();
   FrameDescription* output_frame = new (output_frame_size)
       FrameDescription(output_frame_size, parameters_count);
   FrameWriter frame_writer(this, output_frame, trace_scope_);
@@ -1169,18 +1171,19 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
   const int parameters_count = translated_frame->height();
   ConstructStubFrameInfo frame_info =
       ConstructStubFrameInfo::Precise(parameters_count, is_topmost);
+  const uint32_t output_frame_size = frame_info.frame_size_in_bytes();
 
   TranslatedFrame::iterator function_iterator = value_iterator++;
   if (trace_scope_ != nullptr) {
     PrintF(trace_scope_->file(),
-           "  translating construct stub => bailout_id=%d (%s), height=%d\n",
+           "  translating construct stub => bailout_id=%d (%s), "
+           "variable_frame_size=%d, frame_size=%d\n",
            bailout_id.ToInt(),
            bailout_id == BailoutId::ConstructStubCreate() ? "create" : "invoke",
-           frame_info.frame_size_in_bytes_without_fixed());
+           frame_info.frame_size_in_bytes_without_fixed(), output_frame_size);
   }
 
   // Allocate and store the output frame description.
-  const unsigned output_frame_size = frame_info.frame_size_in_bytes();
   FrameDescription* output_frame = new (output_frame_size)
       FrameDescription(output_frame_size, parameters_count);
   FrameWriter frame_writer(this, output_frame, trace_scope_);
@@ -1470,10 +1473,10 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   if (trace_scope_ != nullptr) {
     PrintF(trace_scope_->file(),
            "  translating BuiltinContinuation to %s,"
-           " register param count %d,"
-           " stack param count %d\n",
+           " => register_param_count=%d,"
+           " stack_param_count=%d, frame_size=%d\n",
            Builtins::name(builtin_name), register_parameter_count,
-           frame_info.stack_parameter_count());
+           frame_info.stack_parameter_count(), output_frame_size);
   }
 
   FrameDescription* output_frame = new (output_frame_size)

src/execution/frames.cc

@@ -2342,9 +2342,6 @@ ArgumentsAdaptorFrameInfo::ArgumentsAdaptorFrameInfo(int translation_height) {
 ConstructStubFrameInfo::ConstructStubFrameInfo(int translation_height,
                                                bool is_topmost,
                                                FrameInfoKind frame_info_kind) {
-  // TODO(jgruber): Support conservative frame layout calculation.
-  DCHECK_EQ(frame_info_kind, FrameInfoKind::kPrecise);
-
   // Note: This is according to the Translation's notion of 'parameters' which
   // differs to that of the SharedFunctionInfo, e.g. by including the receiver.
   const int parameters_count = translation_height;
@@ -2359,9 +2356,11 @@ ConstructStubFrameInfo::ConstructStubFrameInfo(int translation_height,
   static constexpr int kTheResult = 1;
   const int argument_padding = ArgumentPaddingSlots(parameters_count);
 
-  const int adjusted_height = is_topmost ? parameters_count + argument_padding +
-                                               kTheResult + kTopOfStackPadding
-                                         : parameters_count + argument_padding;
+  const int adjusted_height =
+      (is_topmost || frame_info_kind == FrameInfoKind::kConservative)
+          ? parameters_count + argument_padding + kTheResult +
+                kTopOfStackPadding
+          : parameters_count + argument_padding;
   frame_size_in_bytes_without_fixed_ = adjusted_height * kSystemPointerSize;
   frame_size_in_bytes_ = frame_size_in_bytes_without_fixed_ +
                          ConstructFrameConstants::kFixedFrameSize;
@@ -2373,19 +2372,20 @@ BuiltinContinuationFrameInfo::BuiltinContinuationFrameInfo(
     const RegisterConfiguration* register_config, bool is_topmost,
     DeoptimizeKind deopt_kind, BuiltinContinuationMode continuation_mode,
     FrameInfoKind frame_info_kind) {
-  // TODO(jgruber): Support conservative frame layout calculation.
-  DCHECK_EQ(frame_info_kind, FrameInfoKind::kPrecise);
+  const bool is_conservative = frame_info_kind == FrameInfoKind::kConservative;
 
   // Note: This is according to the Translation's notion of 'parameters' which
   // differs to that of the SharedFunctionInfo, e.g. by including the receiver.
   const int parameters_count = translation_height;
-
   frame_has_result_stack_slot_ =
       !is_topmost || deopt_kind == DeoptimizeKind::kLazy;
-  const int result_slot_count = frame_has_result_stack_slot_ ? 1 : 0;
+  const int result_slot_count =
+      (frame_has_result_stack_slot_ || is_conservative) ? 1 : 0;
 
   const int exception_slot_count =
-      (BuiltinContinuationModeIsWithCatch(continuation_mode) ? 1 : 0);
+      (BuiltinContinuationModeIsWithCatch(continuation_mode) || is_conservative)
+          ? 1
+          : 0;
 
   const int allocatable_register_count =
       register_config->num_allocatable_general_registers();
@@ -2410,7 +2410,7 @@ BuiltinContinuationFrameInfo::BuiltinContinuationFrameInfo(
   static constexpr int kTopOfStackPadding = TopOfStackRegisterPaddingSlots();
   static constexpr int kTheResult = 1;
   const int push_result_count =
-      is_topmost ? kTheResult + kTopOfStackPadding : 0;
+      (is_topmost || is_conservative) ? kTheResult + kTopOfStackPadding : 0;
 
   frame_size_in_bytes_ =
       kSystemPointerSize * (stack_parameter_count_ + stack_param_pad_count +

src/execution/frames.h

@@ -1321,6 +1321,13 @@ class SafeStackFrameIterator : public StackFrameIteratorBase {
 // How to calculate the frame layout information. Precise, when all information
 // is available during deoptimization. Conservative, when an overapproximation
 // is fine.
+// TODO(jgruber): Investigate whether the conservative kind can be removed. It
+// seems possible: 1. is_topmost should be known through the outer_state chain
+// of FrameStateDescriptor; 2. the deopt_kind may be a property of the bailout
+// id; 3. for continuation_mode, we only care whether it is a mode with catch,
+// and that is likewise known at compile-time.
+// There is nothing specific blocking this, the investigation just requires time
+// and it is not that important to get the exact frame height at compile-time.
 enum class FrameInfoKind {
   kPrecise,
   kConservative,
@@ -1336,14 +1343,18 @@ enum class BuiltinContinuationMode {
 
 class InterpretedFrameInfo {
  public:
-  // Note: parameters_count includes the receiver, it's thus equal to
-  // `SharedFunctionInfo::internal_formal_parameter_count + 1`.
   static InterpretedFrameInfo Precise(int parameters_count_with_receiver,
                                       int translation_height, bool is_topmost) {
     return {parameters_count_with_receiver, translation_height, is_topmost,
             FrameInfoKind::kPrecise};
   }
 
+  static InterpretedFrameInfo Conservative(int parameters_count_with_receiver,
+                                           int locals_count) {
+    return {parameters_count_with_receiver, locals_count, false,
+            FrameInfoKind::kConservative};
+  }
+
   uint32_t register_stack_slot_count() const {
     return register_stack_slot_count_;
   }
@@ -1368,6 +1379,10 @@ class ArgumentsAdaptorFrameInfo {
     return ArgumentsAdaptorFrameInfo{translation_height};
   }
 
+  static ArgumentsAdaptorFrameInfo Conservative(int parameters_count) {
+    return ArgumentsAdaptorFrameInfo{parameters_count};
+  }
+
   uint32_t frame_size_in_bytes_without_fixed() const {
     return frame_size_in_bytes_without_fixed_;
   }
@@ -1387,6 +1402,10 @@ class ConstructStubFrameInfo {
     return {translation_height, is_topmost, FrameInfoKind::kPrecise};
   }
 
+  static ConstructStubFrameInfo Conservative(int parameters_count) {
+    return {parameters_count, false, FrameInfoKind::kConservative};
+  }
+
   uint32_t frame_size_in_bytes_without_fixed() const {
     return frame_size_in_bytes_without_fixed_;
   }
@@ -1420,6 +1439,21 @@ class BuiltinContinuationFrameInfo {
             FrameInfoKind::kPrecise};
   }
 
+  static BuiltinContinuationFrameInfo Conservative(
+      int parameters_count,
+      const CallInterfaceDescriptor& continuation_descriptor,
+      const RegisterConfiguration* register_config) {
+    // It doesn't matter what we pass as is_topmost, deopt_kind and
+    // continuation_mode; these values are ignored in conservative mode.
+    return {parameters_count,
+            continuation_descriptor,
+            register_config,
+            false,
+            DeoptimizeKind::kEager,
+            BuiltinContinuationMode::STUB,
+            FrameInfoKind::kConservative};
+  }
+
   bool frame_has_result_stack_slot() const {
     return frame_has_result_stack_slot_;
   }