[codegen][frames] Generalize argument padding slot code

- Removes kPadArguments boolean.
- Changes ShouldPadArguments to ArgumentPaddingSlots to reflect
  that on some architectures more than 1 padding slot may be needed.
- Adds AddArgumentPaddingSlots and ShouldPadArguments convenience
  functions.

Bug: v8:9198

Change-Id: Iba87518e071a75fb951b490d3f75a87ca715cc23
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2679109
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72605}

BUILD.gn

@@ -2608,7 +2608,6 @@ v8_source_set("v8_base_without_compiler") {
     "src/codegen/register-arch.h",
     "src/codegen/register-configuration.cc",
     "src/codegen/register-configuration.h",
-    "src/codegen/register.cc",
     "src/codegen/register.h",
     "src/codegen/reglist.h",
     "src/codegen/reloc-info.cc",

src/codegen/arm/register-arm.h

@@ -119,7 +119,12 @@ GENERAL_REGISTERS(DECLARE_REGISTER)
 #undef DECLARE_REGISTER
 constexpr Register no_reg = Register::no_reg();
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = false;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/arm64/register-arm64.h

@@ -241,7 +241,14 @@ class Register : public CPURegister {
 
 ASSERT_TRIVIALLY_COPYABLE(Register);
 
-constexpr bool kPadArguments = true;
+// Stack frame alignment and padding.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // Stack frames are aligned to 16 bytes.
+  constexpr int kStackFrameAlignment = 16;
+  constexpr int alignment_mask = kStackFrameAlignment / kSystemPointerSize - 1;
+  return argument_count & alignment_mask;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/ia32/register-ia32.h

@@ -76,7 +76,12 @@ GENERAL_REGISTERS(DEFINE_REGISTER)
 #undef DEFINE_REGISTER
 constexpr Register no_reg = Register::no_reg();
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/mips/register-mips.h

@@ -203,7 +203,12 @@ int ToNumber(Register reg);
 
 Register ToRegister(int num);
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/mips64/register-mips64.h

@@ -203,7 +203,12 @@ int ToNumber(Register reg);
 
 Register ToRegister(int num);
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/ppc/register-ppc.h

@@ -213,7 +213,12 @@ constexpr Register kConstantPoolRegister = r28;  // Constant pool.
 constexpr Register kRootRegister = r29;          // Roots array pointer.
 constexpr Register cp = r30;                     // JavaScript context pointer.
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/register-arch.h

@@ -30,4 +30,18 @@
 #error Unknown architecture.
 #endif
 
+namespace v8 {
+namespace internal {
+
+constexpr int AddArgumentPaddingSlots(int argument_count) {
+  return argument_count + ArgumentPaddingSlots(argument_count);
+}
+
+constexpr bool ShouldPadArguments(int argument_count) {
+  return ArgumentPaddingSlots(argument_count) != 0;
+}
+
+}  // namespace internal
+}  // namespace v8
+
 #endif  // V8_CODEGEN_REGISTER_ARCH_H_

src/codegen/register.cc

@@ -1,16 +0,0 @@
-// Copyright 2019 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.
-
-#include "src/codegen/register.h"
-#include "src/codegen/register-arch.h"
-
-namespace v8 {
-namespace internal {
-
-bool ShouldPadArguments(int argument_count) {
-  return kPadArguments && (argument_count % 2 != 0);
-}
-
-}  // namespace internal
-}  // namespace v8

src/codegen/register.h

@@ -70,9 +70,6 @@ class RegisterBase {
   int reg_code_;
 };
 
-// Whether padding is needed for the given stack argument count.
-bool ShouldPadArguments(int argument_count);
-
 template <typename RegType,
           typename = decltype(RegisterName(std::declval<RegType>()))>
 inline std::ostream& operator<<(std::ostream& os, RegType reg) {

src/codegen/s390/register-s390.h

@@ -167,7 +167,12 @@ constexpr Register no_reg = Register::no_reg();
 constexpr Register kRootRegister = r10;  // Roots array pointer.
 constexpr Register cp = r13;             // JavaScript context pointer.
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/codegen/x64/register-x64.h

@@ -146,7 +146,12 @@ constexpr Register arg_reg_4 = rcx;
   V(xmm13)                              \
   V(xmm14)
 
-constexpr bool kPadArguments = false;
+// Returns the number of padding slots needed for stack pointer alignment.
+constexpr int ArgumentPaddingSlots(int argument_count) {
+  // No argument padding required.
+  return 0;
+}
+
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/compiler/linkage.cc

@@ -85,22 +85,12 @@ int CallDescriptor::GetStackParameterDelta(
   // inputs to the TailCall node, since they already exist on the stack.
   if (IsTailCallForTierUp()) return 0;
 
-  int callee_slots_above_sp = GetOffsetToReturns();
-  int tail_caller_slots_above_sp = tail_caller->GetOffsetToReturns();
+  // Add padding if necessary before computing the stack parameter delta.
+  int callee_slots_above_sp = AddArgumentPaddingSlots(GetOffsetToReturns());
+  int tail_caller_slots_above_sp =
+      AddArgumentPaddingSlots(tail_caller->GetOffsetToReturns());
   int stack_param_delta = callee_slots_above_sp - tail_caller_slots_above_sp;
-  if (ShouldPadArguments(stack_param_delta)) {
-    if (callee_slots_above_sp % 2 != 0) {
-      // The delta is odd due to the callee - we will need to add one slot
-      // of padding.
-      ++stack_param_delta;
-    } else {
-      DCHECK_NE(tail_caller_slots_above_sp % 2, 0);
-      // The delta is odd because of the caller. We already have one slot of
-      // padding that we can reuse for arguments, so we will need one fewer
-      // slot.
-      --stack_param_delta;
-    }
-  }
+  DCHECK(!ShouldPadArguments(stack_param_delta));
   return stack_param_delta;
 }
 
@@ -137,7 +127,7 @@ int CallDescriptor::GetOffsetToReturns() const {
   // Otherwise, return the first unused slot before the parameters, with any
   // additional padding slot if it exists.
   end_of_returns = GetFirstUnusedStackSlot();
-  if (ShouldPadArguments(end_of_returns)) end_of_returns++;
+  end_of_returns += ArgumentPaddingSlots(end_of_returns);
 
   DCHECK_EQ(end_of_returns == 0, StackParameterCount() == 0);
   return end_of_returns;

src/compiler/wasm-compiler.cc

@@ -8236,8 +8236,7 @@ CallDescriptor* GetWasmCallDescriptor(
         kJSFunctionRegister.code(), MachineType::TaggedPointer()));
   }
 
-  int parameter_slots = params.NumStackSlots();
-  if (ShouldPadArguments(parameter_slots)) parameter_slots++;
+  int parameter_slots = AddArgumentPaddingSlots(params.NumStackSlots());
 
   // Add return location(s).
   LinkageLocationAllocator rets(wasm::kGpReturnRegisters,
@@ -8338,8 +8337,7 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
         kJSFunctionRegister.code(), MachineType::TaggedPointer()));
   }
 
-  int parameter_slots = params.NumStackSlots();
-  if (ShouldPadArguments(parameter_slots)) parameter_slots++;
+  int parameter_slots = AddArgumentPaddingSlots(params.NumStackSlots());
 
   LinkageLocationAllocator rets(wasm::kGpReturnRegisters,
                                 wasm::kFpReturnRegisters, parameter_slots);

src/deoptimizer/deoptimizer.cc

@@ -1037,8 +1037,10 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
 
   // Compute the incoming parameter translation.
   ReadOnlyRoots roots(isolate());
-  if (should_pad_arguments && ShouldPadArguments(parameters_count)) {
-    frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
+  if (should_pad_arguments) {
+    for (int i = 0; i < ArgumentPaddingSlots(parameters_count); ++i) {
+      frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
+    }
   }
 
   // Note: parameters_count includes the receiver.
@@ -1190,7 +1192,7 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
 
   // Translate the accumulator register (depending on frame position).
   if (is_topmost) {
-    if (kPadArguments) {
+    for (int i = 0; i < ArgumentPaddingSlots(1); ++i) {
       frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
     }
     // For topmost frame, put the accumulator on the stack. The
@@ -1294,11 +1296,10 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(
       argument_count_without_receiver - formal_parameter_count;
   // The number of pushed arguments is the maximum of the actual argument count
   // and the formal parameter count + the receiver.
-  const bool should_pad_args = ShouldPadArguments(
+  const int padding = ArgumentPaddingSlots(
       std::max(argument_count_without_receiver, formal_parameter_count) + 1);
   const int output_frame_size =
-      std::max(0, extra_argument_count * kSystemPointerSize) +
-      (should_pad_args ? kSystemPointerSize : 0);
+      (std::max(0, extra_argument_count) + padding) * kSystemPointerSize;
   if (verbose_tracing_enabled()) {
     PrintF(trace_scope_->file(),
            "  translating arguments adaptor => variable_size=%d\n",
@@ -1321,7 +1322,7 @@ void Deoptimizer::DoComputeArgumentsAdaptorFrame(
   FrameWriter frame_writer(this, output_frame, verbose_trace_scope());
 
   ReadOnlyRoots roots(isolate());
-  if (should_pad_args) {
+  for (int i = 0; i < padding; ++i) {
     frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
   }
 
@@ -1383,7 +1384,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
   output_frame->SetTop(top_address);
 
   ReadOnlyRoots roots(isolate());
-  if (ShouldPadArguments(parameters_count)) {
+  for (int i = 0; i < ArgumentPaddingSlots(parameters_count); ++i) {
     frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
   }
 
@@ -1449,7 +1450,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
   frame_writer.PushTranslatedValue(receiver_iterator, debug_hint);
 
   if (is_topmost) {
-    if (kPadArguments) {
+    for (int i = 0; i < ArgumentPaddingSlots(1); ++i) {
       frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
     }
     // Ensure the result is restored back when we return to the stub.
@@ -1733,7 +1734,8 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   ++value_iterator;
 
   ReadOnlyRoots roots(isolate());
-  if (ShouldPadArguments(frame_info.stack_parameter_count())) {
+  const int padding = ArgumentPaddingSlots(frame_info.stack_parameter_count());
+  for (int i = 0; i < padding; ++i) {
     frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
   }
 
@@ -1888,7 +1890,7 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   }
 
   if (is_topmost) {
-    if (kPadArguments) {
+    for (int i = 0; i < ArgumentPaddingSlots(1); ++i) {
       frame_writer.PushRawObject(roots.the_hole_value(), "padding\n");
     }
 

src/deoptimizer/frame-description.h

@@ -112,7 +112,9 @@ class FrameDescription {
 
   unsigned GetLastArgumentSlotOffset(bool pad_arguments = true) {
     int parameter_slots = parameter_count();
-    if (pad_arguments && ShouldPadArguments(parameter_slots)) parameter_slots++;
+    if (pad_arguments) {
+      parameter_slots = AddArgumentPaddingSlots(parameter_slots);
+    }
     return GetFrameSize() - parameter_slots * kSystemPointerSize;
   }
 

src/execution/frames.cc

@@ -2175,13 +2175,11 @@ InnerPointerToCodeCache::GetCacheEntry(Address inner_pointer) {
 
 namespace {
 
-int ArgumentPaddingSlots(int arg_count) {
-  return ShouldPadArguments(arg_count) ? 1 : 0;
-}
-
 // Some architectures need to push padding together with the TOS register
 // in order to maintain stack alignment.
-constexpr int TopOfStackRegisterPaddingSlots() { return kPadArguments ? 1 : 0; }
+constexpr int TopOfStackRegisterPaddingSlots() {
+  return ArgumentPaddingSlots(1);
+}
 
 bool BuiltinContinuationModeIsWithCatch(BuiltinContinuationMode mode) {
   switch (mode) {

test/unittests/compiler/linkage-tail-call-unittest.cc

@@ -169,8 +169,8 @@ TEST_F(LinkageTailCall, MoreRegisterAndStackParametersCallee) {
   Node* const node = Node::New(zone(), 1, op, 0, nullptr, false);
   EXPECT_TRUE(desc1->CanTailCall(CallDescriptorOf(node->op())));
   int stack_param_delta = desc2->GetStackParameterDelta(desc1);
-  // We might need to add one slot of padding to the callee arguments.
-  int expected = kPadArguments ? 2 : 1;
+  // We might need to add padding slots to the callee arguments.
+  int expected = 1 + ArgumentPaddingSlots(1);
   EXPECT_EQ(expected, stack_param_delta);
 }
 
@@ -192,8 +192,8 @@ TEST_F(LinkageTailCall, MoreRegisterAndStackParametersCaller) {
   Node* const node = Node::New(zone(), 1, op, 0, nullptr, false);
   EXPECT_TRUE(desc1->CanTailCall(CallDescriptorOf(node->op())));
   int stack_param_delta = desc2->GetStackParameterDelta(desc1);
-  // We might need to drop one slot of padding from the caller's arguments.
-  int expected = kPadArguments ? -2 : -1;
+  // We might need to drop padding slots from the caller's arguments.
+  int expected = -1 - ArgumentPaddingSlots(1);
   EXPECT_EQ(expected, stack_param_delta);
 }
 
@@ -329,8 +329,8 @@ TEST_F(LinkageTailCall, MatchingStackParametersExtraCallerRegistersAndStack) {
       Node::New(zone(), 1, op, arraysize(parameters), parameters, false);
   EXPECT_TRUE(desc1->CanTailCall(CallDescriptorOf(node->op())));
   int stack_param_delta = desc2->GetStackParameterDelta(desc1);
-  // We might need to add one slot of padding to the callee arguments.
-  int expected = kPadArguments ? 0 : -1;
+  // We might need to add padding slots to the callee arguments.
+  int expected = ArgumentPaddingSlots(1) - 1;
   EXPECT_EQ(expected, stack_param_delta);
 }
 
@@ -359,8 +359,8 @@ TEST_F(LinkageTailCall, MatchingStackParametersExtraCalleeRegistersAndStack) {
       Node::New(zone(), 1, op, arraysize(parameters), parameters, false);
   EXPECT_TRUE(desc1->CanTailCall(CallDescriptorOf(node->op())));
   int stack_param_delta = desc2->GetStackParameterDelta(desc1);
-  // We might need to drop one slot of padding from the caller's arguments.
-  int expected = kPadArguments ? 0 : 1;
+  // We might need to drop padding slots from the caller's arguments.
+  int expected = 1 - ArgumentPaddingSlots(1);
   EXPECT_EQ(expected, stack_param_delta);
 }
 

test/unittests/wasm/wasm-compiler-unittest.cc

@@ -97,7 +97,7 @@ TEST_F(WasmCallDescriptorTest, Regress_1174500) {
 
   // The stack return slot should be right above our last parameter, and any
   // argument padding slots. The return slot itself should not be padded.
-  const int padding = kPadArguments ? 1 : 0;
+  const int padding = ShouldPadArguments(1);
   const int first_return_slot = -1 - (padding + 1);
   compiler::LinkageLocation return_location =
       desc->GetReturnLocation(kReturns - 1);