[Liftoff] Support arbitrary frame sizes

Instead of limiting the number of used spill slots and bailing out if
the limit is exceeded, we now store the number of spill slots used and
patch the stack frame size after generating all code.
This removes a lot of checks and bailouts.

Drive-by: Fix a bug with spilling f64 caller frame slots which was
uncovered by the additional test coverage after this CL.

R=titzer@chromium.org

Bug: v8:6600
Change-Id: I25d856f99451642cc15239c0461402e51487d0a1
Reviewed-on: https://chromium-review.googlesource.com/929162Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51500}

src/wasm/baseline/ia32/liftoff-assembler-ia32.h

@@ -338,6 +338,7 @@ void LiftoffAssembler::Move(DoubleRegister dst, DoubleRegister src,
 
 void LiftoffAssembler::Spill(uint32_t index, LiftoffRegister reg,
                              ValueType type) {
+  RecordUsedSpillSlot(index);
   Operand dst = liftoff::GetStackSlot(index);
   switch (type) {
     case kWasmI32:
@@ -359,6 +360,7 @@ void LiftoffAssembler::Spill(uint32_t index, LiftoffRegister reg,
 }
 
 void LiftoffAssembler::Spill(uint32_t index, WasmValue value) {
+  RecordUsedSpillSlot(index);
   Operand dst = liftoff::GetStackSlot(index);
   switch (value.type()) {
     case kWasmI32:
@@ -689,12 +691,12 @@ void LiftoffAssembler::PushCallerFrameSlot(const VarState& src,
                                            RegPairHalf half) {
   switch (src.loc()) {
     case VarState::kStack:
-      push(liftoff::GetHalfStackSlot(2 * src_index +
-                                     (half == kLowWord ? 0 : 1)));
       if (src.type() == kWasmF64) {
         DCHECK_EQ(kLowWord, half);
-        push(liftoff::GetHalfStackSlot(2 * src_index + 1));
+        push(liftoff::GetHalfStackSlot(2 * src_index - 1));
       }
+      push(liftoff::GetHalfStackSlot(2 * src_index +
+                                     (half == kLowWord ? 0 : 1)));
       break;
     case VarState::kRegister:
       if (src.type() == kWasmI64) {

src/wasm/baseline/liftoff-assembler.cc

@@ -123,9 +123,6 @@ class StackTransferRecipe {
           LoadStackSlot(register_moves_.back().dst, next_spill_slot, rm.type);
           DCHECK_EQ(1, src_reg_use_count[spill_reg.liftoff_code()]);
           src_reg_use_count[spill_reg.liftoff_code()] = 0;
-          if (next_spill_slot > max_used_spill_slot_) {
-            max_used_spill_slot_ = next_spill_slot;
-          }
           ++next_spill_slot;
           executed_moves = 1;
         }
@@ -255,8 +252,6 @@ class StackTransferRecipe {
     register_loads_.push_back(RegisterLoad::HalfStack(dst, half_stack_index));
   }
 
-  uint32_t max_used_spill_slot() const { return max_used_spill_slot_; }
-
  private:
   // TODO(clemensh): Avoid unconditionally allocating on the heap.
   std::vector<RegisterMove> register_moves_;
@@ -264,7 +259,6 @@ class StackTransferRecipe {
   LiftoffRegList move_dst_regs_;
   LiftoffRegList move_src_regs_;
   LiftoffAssembler* const asm_;
-  uint32_t max_used_spill_slot_ = 0;
 };
 
 }  // namespace
@@ -466,7 +460,6 @@ void LiftoffAssembler::SpillAllRegisters() {
 
 void LiftoffAssembler::PrepareCall(wasm::FunctionSig* sig,
                                    compiler::CallDescriptor* call_descriptor,
-                                   uint32_t* max_used_spill_slot,
                                    Register* target,
                                    LiftoffRegister* explicit_context) {
   uint32_t num_params = static_cast<uint32_t>(sig->parameter_count());
@@ -557,10 +550,6 @@ void LiftoffAssembler::PrepareCall(wasm::FunctionSig* sig,
   // Execute the stack transfers before filling the context register.
   stack_transfers.Execute();
 
-  // Record the maximum used stack slot index, such that we can bail out if the
-  // stack grew too large.
-  *max_used_spill_slot = stack_transfers.max_used_spill_slot();
-
   // Pop parameters from the value stack.
   auto stack_end = cache_state_.stack_state.end();
   cache_state_.stack_state.erase(stack_end - num_params, stack_end);
@@ -649,10 +638,6 @@ void LiftoffAssembler::set_num_locals(uint32_t num_locals) {
   }
 }
 
-uint32_t LiftoffAssembler::GetTotalFrameSlotCount() const {
-  return num_locals() + kMaxValueStackHeight;
-}
-
 std::ostream& operator<<(std::ostream& os, VarState slot) {
   os << WasmOpcodes::TypeName(slot.type()) << ":";
   switch (slot.loc()) {

src/wasm/baseline/liftoff-assembler.h

@@ -27,10 +27,6 @@ struct ModuleEnv;
 
 class LiftoffAssembler : public TurboAssembler {
  public:
-  // TODO(clemensh): Remove this limitation by allocating more stack space if
-  // needed.
-  static constexpr int kMaxValueStackHeight = 8;
-
   // Each slot in our stack frame currently has exactly 8 bytes.
   static constexpr uint32_t kStackSlotSize = 8;
 
@@ -304,13 +300,17 @@ class LiftoffAssembler : public TurboAssembler {
   void SpillLocals();
   void SpillAllRegisters();
 
+  // Call this method whenever spilling something, such that the number of used
+  // spill slot can be tracked and the stack frame will be allocated big enough.
+  void RecordUsedSpillSlot(uint32_t index) {
+    if (index >= num_used_spill_slots_) num_used_spill_slots_ = index + 1;
+  }
+
   // Load parameters into the right registers / stack slots for the call.
   // Move {*target} into another register if needed and update {*target} to that
   // register, or {no_reg} if target was spilled to the stack.
-  // TODO(clemensh): Remove {max_used_spill_slot} once we support arbitrary
-  // stack sizes.
   void PrepareCall(wasm::FunctionSig*, compiler::CallDescriptor*,
-                   uint32_t* max_used_spill_slot, Register* target = nullptr,
+                   Register* target = nullptr,
                    LiftoffRegister* explicit_context = nullptr);
   // Process return values of the call.
   void FinishCall(wasm::FunctionSig*, compiler::CallDescriptor*);
@@ -436,7 +436,9 @@ class LiftoffAssembler : public TurboAssembler {
   uint32_t num_locals() const { return num_locals_; }
   void set_num_locals(uint32_t num_locals);
 
-  uint32_t GetTotalFrameSlotCount() const;
+  uint32_t GetTotalFrameSlotCount() const {
+    return num_locals_ + num_used_spill_slots_;
+  }
 
   ValueType local_type(uint32_t index) {
     DCHECK_GT(num_locals_, index);
@@ -466,6 +468,7 @@ class LiftoffAssembler : public TurboAssembler {
   static_assert(sizeof(ValueType) == 1,
                 "Reconsider this inlining if ValueType gets bigger");
   CacheState cache_state_;
+  uint32_t num_used_spill_slots_ = 0;
   const char* bailout_reason_ = nullptr;
 
   LiftoffRegister SpillOneRegister(LiftoffRegList candidates,

src/wasm/baseline/liftoff-compiler.cc

@@ -200,14 +200,6 @@ class LiftoffCompiler {
 #endif
   }
 
-  void CheckStackSizeLimit(Decoder* decoder) {
-    DCHECK_GE(__ cache_state()->stack_height(), __ num_locals());
-    int stack_height = __ cache_state()->stack_height() - __ num_locals();
-    if (stack_height > LiftoffAssembler::kMaxValueStackHeight) {
-      unsupported(decoder, "value stack grows too large");
-    }
-  }
-
   void StartFunction(Decoder* decoder) {
     int num_locals = decoder->NumLocals();
     __ set_num_locals(num_locals);
@@ -337,7 +329,6 @@ class LiftoffCompiler {
     StackCheck(0);
 
     DCHECK_EQ(__ num_locals(), __ cache_state()->stack_height());
-    CheckStackSizeLimit(decoder);
   }
 
   void GenerateOutOfLineCode(OutOfLineCode& ool) {
@@ -647,7 +638,6 @@ class LiftoffCompiler {
 
   void I32Const(Decoder* decoder, Value* result, int32_t value) {
     __ cache_state()->stack_state.emplace_back(kWasmI32, value);
-    CheckStackSizeLimit(decoder);
   }
 
   void I64Const(Decoder* decoder, Value* result, int64_t value) {
@@ -663,21 +653,18 @@ class LiftoffCompiler {
       __ LoadConstant(reg, WasmValue(value));
       __ PushRegister(kWasmI64, reg);
     }
-    CheckStackSizeLimit(decoder);
   }
 
   void F32Const(Decoder* decoder, Value* result, float value) {
     LiftoffRegister reg = __ GetUnusedRegister(kFpReg);
     __ LoadConstant(reg, WasmValue(value));
     __ PushRegister(kWasmF32, reg);
-    CheckStackSizeLimit(decoder);
   }
 
   void F64Const(Decoder* decoder, Value* result, double value) {
     LiftoffRegister reg = __ GetUnusedRegister(kFpReg);
     __ LoadConstant(reg, WasmValue(value));
     __ PushRegister(kWasmF64, reg);
-    CheckStackSizeLimit(decoder);
   }
 
   void Drop(Decoder* decoder, const Value& value) {
@@ -723,7 +710,6 @@ class LiftoffCompiler {
         break;
       }
     }
-    CheckStackSizeLimit(decoder);
   }
 
   void SetLocalFromStackSlot(LiftoffAssembler::VarState& dst_slot,
@@ -794,7 +780,6 @@ class LiftoffCompiler {
       return unsupported(decoder, "global > kPointerSize");
     __ Load(value, addr, no_reg, global->offset, type, pinned);
     __ PushRegister(global->type, value);
-    CheckStackSizeLimit(decoder);
   }
 
   void SetGlobal(Decoder* decoder, const Value& value,
@@ -1051,7 +1036,6 @@ class LiftoffCompiler {
                        protected_load_pc);
     }
     __ PushRegister(value_type, value);
-    CheckStackSizeLimit(decoder);
 
     if (FLAG_wasm_trace_memory) {
       TraceMemoryOperation(false, type.mem_type().representation(), index,
@@ -1111,12 +1095,7 @@ class LiftoffCompiler {
     call_descriptor =
         GetLoweredCallDescriptor(compilation_zone_, call_descriptor);
 
-    uint32_t max_used_spill_slot = 0;
-    __ PrepareCall(operand.sig, call_descriptor, &max_used_spill_slot);
-    if (max_used_spill_slot >
-        __ num_locals() + LiftoffAssembler::kMaxValueStackHeight) {
-      unsupported(decoder, "value stack grows too large in call");
-    }
+    __ PrepareCall(operand.sig, call_descriptor);
 
     source_position_table_builder_->AddPosition(
         __ pc_offset(), SourcePosition(decoder->position()), false);
@@ -1285,15 +1264,9 @@ class LiftoffCompiler {
     call_descriptor =
         GetLoweredCallDescriptor(compilation_zone_, call_descriptor);
 
-    uint32_t max_used_spill_slot = 0;
     Register target = scratch.gp();
-    __ PrepareCall(operand.sig, call_descriptor, &max_used_spill_slot, &target,
-                   explicit_context);
+    __ PrepareCall(operand.sig, call_descriptor, &target, explicit_context);
     __ CallIndirect(operand.sig, call_descriptor, target);
-    if (max_used_spill_slot >
-        __ num_locals() + LiftoffAssembler::kMaxValueStackHeight) {
-      unsupported(decoder, "value stack grows too large in indirect call");
-    }
 
     safepoint_table_builder_.DefineSafepoint(asm_, Safepoint::kSimple, 0,
                                              Safepoint::kNoLazyDeopt);

src/wasm/baseline/x64/liftoff-assembler-x64.h

@@ -278,6 +278,7 @@ void LiftoffAssembler::Move(DoubleRegister dst, DoubleRegister src,
 
 void LiftoffAssembler::Spill(uint32_t index, LiftoffRegister reg,
                              ValueType type) {
+  RecordUsedSpillSlot(index);
   Operand dst = liftoff::GetStackSlot(index);
   switch (type) {
     case kWasmI32:
@@ -298,6 +299,7 @@ void LiftoffAssembler::Spill(uint32_t index, LiftoffRegister reg,
 }
 
 void LiftoffAssembler::Spill(uint32_t index, WasmValue value) {
+  RecordUsedSpillSlot(index);
   Operand dst = liftoff::GetStackSlot(index);
   switch (value.type()) {
     case kWasmI32: