[arm64] Pad function arguments.

This patch updates the instruction selector and code generator to pad arguments
for arm64 and drop an even number of slots when dropping the arguments. It also
updates the builtins that handle arguments. These changes need to be made at
the same time.

It also adds some tests for forwarding varargs, as this was affected by the
builtin changes and the existing tests did not catch all issues.

Bug: v8:6644
Change-Id: I81318d1d1c9ab2568f84f2bb868d2a2d4cb56053
Reviewed-on: https://chromium-review.googlesource.com/829933
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50259}

src/arm64/assembler-arm64.h

@@ -295,9 +295,7 @@ class Register : public CPURegister {
 static_assert(IS_TRIVIALLY_COPYABLE(Register),
               "Register can efficiently be passed by value");
 
-// TODO(arm64): Switch this on when the rest of the argument padding changes
-//              are done.
-constexpr bool kPadArguments = false;
+constexpr bool kPadArguments = true;
 constexpr bool kSimpleFPAliasing = true;
 constexpr bool kSimdMaskRegisters = false;
 

src/arm64/macro-assembler-arm64-inl.h

@@ -1339,21 +1339,31 @@ void TurboAssembler::Drop(const Register& count, uint64_t unit_size) {
 
 void TurboAssembler::DropArguments(const Register& count,
                                    ArgumentsCountMode mode) {
+  int extra_slots = 1;  // Padding slot.
   if (mode == kCountExcludesReceiver) {
-    UseScratchRegisterScope temps(this);
-    Register tmp = temps.AcquireX();
-    Add(tmp, count, 1);
-    Drop(tmp);
-  } else {
-    Drop(count);
+    // Add a slot for the receiver.
+    ++extra_slots;
+  }
+  UseScratchRegisterScope temps(this);
+  Register tmp = temps.AcquireX();
+  Add(tmp, count, extra_slots);
+  Bic(tmp, tmp, 1);
+  Drop(tmp, kXRegSize);
+}
+
+void TurboAssembler::DropArguments(int64_t count, ArgumentsCountMode mode) {
+  if (mode == kCountExcludesReceiver) {
+    // Add a slot for the receiver.
+    ++count;
   }
+  Drop(RoundUp(count, 2), kXRegSize);
 }
 
-void TurboAssembler::DropSlots(int64_t count, uint64_t unit_size) {
-  Drop(count, unit_size);
+void TurboAssembler::DropSlots(int64_t count) {
+  Drop(RoundUp(count, 2), kXRegSize);
 }
 
-void TurboAssembler::PushArgument(const Register& arg) { Push(arg); }
+void TurboAssembler::PushArgument(const Register& arg) { Push(padreg, arg); }
 
 void MacroAssembler::DropBySMI(const Register& count_smi, uint64_t unit_size) {
   DCHECK(unit_size == 0 || base::bits::IsPowerOfTwo(unit_size));

src/arm64/macro-assembler-arm64.cc

@@ -2159,20 +2159,27 @@ void TurboAssembler::PrepareForTailCall(const ParameterCount& callee_args_count,
   // after we drop current frame. We add kPointerSize to count the receiver
   // argument which is not included into formal parameters count.
   Register dst_reg = scratch0;
-  add(dst_reg, fp, Operand(caller_args_count_reg, LSL, kPointerSizeLog2));
-  add(dst_reg, dst_reg,
-      Operand(StandardFrameConstants::kCallerSPOffset + kPointerSize));
+  Add(dst_reg, fp, Operand(caller_args_count_reg, LSL, kPointerSizeLog2));
+  Add(dst_reg, dst_reg, StandardFrameConstants::kCallerSPOffset + kPointerSize);
+  // Round dst_reg up to a multiple of 16 bytes, so that we overwrite any
+  // potential padding.
+  Add(dst_reg, dst_reg, 15);
+  Bic(dst_reg, dst_reg, 15);
 
   Register src_reg = caller_args_count_reg;
   // Calculate the end of source area. +kPointerSize is for the receiver.
   if (callee_args_count.is_reg()) {
-    add(src_reg, jssp, Operand(callee_args_count.reg(), LSL, kPointerSizeLog2));
-    add(src_reg, src_reg, Operand(kPointerSize));
+    Add(src_reg, jssp, Operand(callee_args_count.reg(), LSL, kPointerSizeLog2));
+    Add(src_reg, src_reg, kPointerSize);
   } else {
-    add(src_reg, jssp,
-        Operand((callee_args_count.immediate() + 1) * kPointerSize));
+    Add(src_reg, jssp, (callee_args_count.immediate() + 1) * kPointerSize);
   }
 
+  // Round src_reg up to a multiple of 16 bytes, so we include any potential
+  // padding in the copy.
+  Add(src_reg, src_reg, 15);
+  Bic(src_reg, src_reg, 15);
+
   if (FLAG_debug_code) {
     Cmp(src_reg, dst_reg);
     Check(lo, kStackAccessBelowStackPointer);

src/arm64/macro-assembler-arm64.h

@@ -711,25 +711,22 @@ class TurboAssembler : public Assembler {
   inline void Drop(int64_t count, uint64_t unit_size = kXRegSize);
   inline void Drop(const Register& count, uint64_t unit_size = kXRegSize);
 
-  // Drop arguments from stack without actually accessing memory.
-  // This will currently drop 'count' arguments from the stack.
+  // Drop 'count' arguments from the stack, rounded up to a multiple of two,
+  // without actually accessing memory.
   // We assume the size of the arguments is the pointer size.
   // An optional mode argument is passed, which can indicate we need to
   // explicitly add the receiver to the count.
-  // TODO(arm64): Update this to round up the number of bytes dropped to
-  // a multiple of 16, so that we can remove jssp.
   enum ArgumentsCountMode { kCountIncludesReceiver, kCountExcludesReceiver };
   inline void DropArguments(const Register& count,
                             ArgumentsCountMode mode = kCountIncludesReceiver);
+  inline void DropArguments(int64_t count,
+                            ArgumentsCountMode mode = kCountIncludesReceiver);
 
-  // Drop slots from stack without actually accessing memory.
-  // This will currently drop 'count' slots of the given size from the stack.
-  // TODO(arm64): Update this to round up the number of bytes dropped to
-  // a multiple of 16, so that we can remove jssp.
-  inline void DropSlots(int64_t count, uint64_t unit_size = kXRegSize);
+  // Drop 'count' slots from stack, rounded up to a multiple of two, without
+  // actually accessing memory.
+  inline void DropSlots(int64_t count);
 
-  // Push a single argument to the stack.
-  // TODO(arm64): Update this to push a padding slot above the argument.
+  // Push a single argument, with padding, to the stack.
   inline void PushArgument(const Register& arg);
 
   // Re-synchronizes the system stack pointer (csp) with the current stack

src/compiler/arm64/code-generator-arm64.cc

@@ -597,6 +597,7 @@ void AdjustStackPointerForTailCall(TurboAssembler* tasm,
   int current_sp_offset = state->GetSPToFPSlotCount() +
                           StandardFrameConstants::kFixedSlotCountAboveFp;
   int stack_slot_delta = new_slot_above_sp - current_sp_offset;
+  DCHECK_EQ(stack_slot_delta % 2, 0);
   if (stack_slot_delta > 0) {
     tasm->Claim(stack_slot_delta);
     state->IncreaseSPDelta(stack_slot_delta);
@@ -616,8 +617,15 @@ void CodeGenerator::AssembleTailCallBeforeGap(Instruction* instr,
 
 void CodeGenerator::AssembleTailCallAfterGap(Instruction* instr,
                                              int first_unused_stack_slot) {
+  DCHECK_EQ(first_unused_stack_slot % 2, 0);
   AdjustStackPointerForTailCall(tasm(), frame_access_state(),
                                 first_unused_stack_slot);
+  DCHECK(instr->IsTailCall());
+  InstructionOperandConverter g(this, instr);
+  int optional_padding_slot = g.InputInt32(instr->InputCount() - 2);
+  if (optional_padding_slot % 2) {
+    __ Poke(padreg, optional_padding_slot * kPointerSize);
+  }
 }
 
 // Check if the code object is marked for deoptimization. If it is, then it
@@ -1369,7 +1377,7 @@ CodeGenerator::CodeGenResult CodeGenerator::AssembleArchInstruction(
       if (instr->InputAt(0)->IsFPRegister()) {
         __ Poke(i.InputFloat64Register(0), operand);
       } else {
-        __ Poke(i.InputRegister(0), operand);
+        __ Poke(i.InputOrZeroRegister64(0), operand);
       }
       __ SetStackPointer(prev);
       break;
@@ -2475,6 +2483,8 @@ void CodeGenerator::AssembleConstructFrame() {
     __ AssertCspAligned();
   }
 
+  // The frame has been previously padded in CodeGenerator::FinishFrame().
+  DCHECK_EQ(frame()->GetTotalFrameSlotCount() % 2, 0);
   int shrink_slots =
       frame()->GetTotalFrameSlotCount() - descriptor->CalculateFixedFrameSize();
 
@@ -2629,28 +2639,20 @@ void CodeGenerator::AssembleReturn(InstructionOperand* pop) {
       } else {
         __ Bind(&return_label_);
         AssembleDeconstructFrame();
-        if (descriptor->UseNativeStack()) {
-          pop_count += (pop_count & 1);  // align
-        }
       }
     } else {
       AssembleDeconstructFrame();
-      if (descriptor->UseNativeStack()) {
-        pop_count += (pop_count & 1);  // align
-      }
     }
-  } else if (descriptor->UseNativeStack()) {
-    pop_count += (pop_count & 1);  // align
   }
 
   if (pop->IsImmediate()) {
     DCHECK_EQ(Constant::kInt32, g.ToConstant(pop).type());
     pop_count += g.ToConstant(pop).ToInt32();
-    __ Drop(pop_count);
+    __ DropArguments(pop_count);
   } else {
     Register pop_reg = g.ToRegister(pop);
     __ Add(pop_reg, pop_reg, pop_count);
-    __ Drop(pop_reg);
+    __ DropArguments(pop_reg);
   }
 
   if (descriptor->UseNativeStack()) {

src/compiler/arm64/instruction-selector-arm64.cc

@@ -1740,6 +1740,7 @@ void InstructionSelector::EmitPrepareArguments(
 
   int claim_count = static_cast<int>(arguments->size());
   int slot = claim_count - 1;
+  claim_count = RoundUp(claim_count, 2);
   // Bump the stack pointer(s).
   if (claim_count > 0 || always_claim) {
     // TODO(titzer): claim and poke probably take small immediates.
@@ -1751,8 +1752,14 @@ void InstructionSelector::EmitPrepareArguments(
     Emit(claim, g.NoOutput(), g.TempImmediate(claim_count));
   }
 
-  // Poke the arguments into the stack.
   ArchOpcode poke = to_native_stack ? kArm64PokeCSP : kArm64PokeJSSP;
+  if (claim_count > 0) {
+    // Store padding, which might be overwritten.
+    Emit(poke, g.NoOutput(), g.UseImmediate(0),
+         g.TempImmediate(claim_count - 1));
+  }
+
+  // Poke the arguments into the stack.
   while (slot >= 0) {
     Emit(poke, g.NoOutput(), g.UseRegister((*arguments)[slot].node),
          g.TempImmediate(slot));

src/compiler/instruction-selector.cc

@@ -2505,6 +2505,14 @@ void InstructionSelector::VisitTailCall(Node* node) {
 
   Emit(kArchPrepareTailCall, g.NoOutput());
 
+  // Add an immediate operand that represents the first slot that is unused
+  // with respect to the stack pointer that has been updated for the tail call
+  // instruction. This is used by backends that need to pad arguments for stack
+  // alignment, in order to store an optional slot of padding above the
+  // arguments.
+  int optional_padding_slot = callee->GetFirstUnusedStackSlot();
+  buffer.instruction_args.push_back(g.TempImmediate(optional_padding_slot));
+
   int first_unused_stack_slot =
       (V8_TARGET_ARCH_STORES_RETURN_ADDRESS_ON_STACK ? 1 : 0) +
       stack_param_delta;

src/compiler/linkage.cc

@@ -75,33 +75,25 @@ bool CallDescriptor::HasSameReturnLocationsAs(
   return true;
 }
 
-int CallDescriptor::GetStackParameterDelta(
-    CallDescriptor const* tail_caller) const {
-  int callee_slots_above_sp = 0;
+int CallDescriptor::GetFirstUnusedStackSlot() const {
+  int slots_above_sp = 0;
   for (size_t i = 0; i < InputCount(); ++i) {
     LinkageLocation operand = GetInputLocation(i);
     if (!operand.IsRegister()) {
       int new_candidate =
           -operand.GetLocation() + operand.GetSizeInPointers() - 1;
-      if (new_candidate > callee_slots_above_sp) {
-        callee_slots_above_sp = new_candidate;
-      }
-    }
-  }
-  int tail_caller_slots_above_sp = 0;
-  if (tail_caller != nullptr) {
-    for (size_t i = 0; i < tail_caller->InputCount(); ++i) {
-      LinkageLocation operand = tail_caller->GetInputLocation(i);
-      if (!operand.IsRegister()) {
-        int new_candidate =
-            -operand.GetLocation() + operand.GetSizeInPointers() - 1;
-        if (new_candidate > tail_caller_slots_above_sp) {
-          tail_caller_slots_above_sp = new_candidate;
-        }
+      if (new_candidate > slots_above_sp) {
+        slots_above_sp = new_candidate;
       }
     }
   }
+  return slots_above_sp;
+}
 
+int CallDescriptor::GetStackParameterDelta(
+    CallDescriptor const* tail_caller) const {
+  int callee_slots_above_sp = GetFirstUnusedStackSlot();
+  int tail_caller_slots_above_sp = tail_caller->GetFirstUnusedStackSlot();
   int stack_param_delta = callee_slots_above_sp - tail_caller_slots_above_sp;
   if (kPadArguments) {
     // Adjust stack delta when it is odd.

src/compiler/linkage.h

@@ -298,6 +298,9 @@ class V8_EXPORT_PRIVATE CallDescriptor final
 
   bool HasSameReturnLocationsAs(const CallDescriptor* other) const;
 
+  // Returns the first stack slot that is not used by the stack parameters.
+  int GetFirstUnusedStackSlot() const;
+
   int GetStackParameterDelta(const CallDescriptor* tail_caller) const;
 
   bool CanTailCall(const Node* call) const;

test/cctest/compiler/test-code-generator.cc

@@ -780,10 +780,12 @@ class CodeGeneratorTester {
   }
 
   Instruction* CreateTailCall(int stack_slot_delta) {
+    int optional_padding_slot = stack_slot_delta;
     InstructionOperand callee[] = {
+        ImmediateOperand(ImmediateOperand::INLINE, optional_padding_slot),
         ImmediateOperand(ImmediateOperand::INLINE, stack_slot_delta)};
     Instruction* tail_call = Instruction::New(zone_, kArchTailCallCodeObject, 0,
-                                              nullptr, 1, callee, 0, nullptr);
+                                              nullptr, 2, callee, 0, nullptr);
     return tail_call;
   }
 
@@ -851,13 +853,15 @@ class CodeGeneratorTester {
     // We use either zero or one slots.
     int first_unused_stack_slot =
         V8_TARGET_ARCH_STORES_RETURN_ADDRESS_ON_STACK ? 1 : 0;
+    int optional_padding_slot = first_unused_stack_slot;
     InstructionOperand callee[] = {
         AllocatedOperand(LocationOperand::REGISTER,
                          MachineRepresentation::kTagged,
                          kReturnRegister0.code()),
+        ImmediateOperand(ImmediateOperand::INLINE, optional_padding_slot),
         ImmediateOperand(ImmediateOperand::INLINE, first_unused_stack_slot)};
     Instruction* tail_call = Instruction::New(zone_, kArchTailCallCodeObject, 0,
-                                              nullptr, 2, callee, 0, nullptr);
+                                              nullptr, 3, callee, 0, nullptr);
     sequence->AddInstruction(tail_call);
     sequence->EndBlock(RpoNumber::FromInt(0));
 

test/mjsunit/compiler/varargs.js

+// Copyright 2017 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.
+
+x = "a";
+
+function test_varargs(...args) {
+  var sum = this.x;
+  for (i in args) {
+    sum += "," + args[i];
+  }
+  return sum;
+}
+
+assertEquals("a", test_varargs());
+assertEquals("a,b", test_varargs("b"));
+assertEquals("a,b,c", test_varargs("b", "c"));
+assertEquals("a,b,c,d", test_varargs("b", "c", "d"));
+assertEquals("a,b,c,d,e", test_varargs("b", "c", "d", "e"));
+
+function forward_varargs(...args) {
+  return test_varargs(...args);
+}
+
+assertEquals("a", forward_varargs());
+assertEquals("a,b", forward_varargs("b"));
+assertEquals("a,b,c", forward_varargs("b", "c"));
+assertEquals("a,b,c,d", forward_varargs("b", "c", "d"));
+assertEquals("a,b,c,d,e", forward_varargs("b", "c", "d", "e"));
+
+function forward_varargs_one_arg(x, ...args) {
+  return test_varargs(x, ...args);
+}
+
+assertEquals("a,undefined", forward_varargs_one_arg());
+assertEquals("a,b", forward_varargs_one_arg("b"));
+assertEquals("a,b,c", forward_varargs_one_arg("b", "c"));
+assertEquals("a,b,c,d", forward_varargs_one_arg("b", "c", "d"));
+assertEquals("a,b,c,d,e", forward_varargs_one_arg("b", "c", "d", "e"));
+
+function forward_varargs_two_args(x, y, ...args) {
+  return test_varargs(x, y, ...args);
+}
+
+assertEquals("a,undefined,undefined", forward_varargs_two_args());
+assertEquals("a,b,undefined", forward_varargs_two_args("b"));
+assertEquals("a,b,c", forward_varargs_two_args("b", "c"));
+assertEquals("a,b,c,d", forward_varargs_two_args("b", "c", "d"));
+assertEquals("a,b,c,d,e", forward_varargs_two_args("b", "c", "d", "e"));