[arm64] Update BuiltinContinuation frames for jssp alignment.

Adds some necessary padding to ensure the frame is 16-byte aligned.
We don't yet consider the bailout state, which will be handled separately.

This patch also improves the code generated for ContinueTo*Builtin* stubs.

Finally, it adds a test that checks the return value for Array.map in
the case where a LAZY deopt results in a topmost builtin continuation
frame - this is easy to break if the padding for the result is done
incorrectly in NotifyBuiltinContinuation, but was not detected by existing
tests.

Bug: v8:6644
Change-Id: Id1a294950cdf535e2bfdb0ed27c67f077ec34f8a
Reviewed-on: https://chromium-review.googlesource.com/704835
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48465}

src/arm/frame-constants-arm.cc

@@ -24,6 +24,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/arm64/frame-constants-arm64.cc

@@ -24,6 +24,14 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return RoundUp(register_count, 2);
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  // Round the total slot count up to a multiple of two, to make the frame a
+  // multiple of 16 bytes.
+  int slot_count = kFixedSlotCount + register_count;
+  int rounded_slot_count = RoundUp(slot_count, 2);
+  return rounded_slot_count - slot_count;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/builtins/arm64/builtins-arm64.cc

@@ -1511,10 +1511,10 @@ void Builtins::Generate_NotifyBuiltinContinuation(MacroAssembler* masm) {
   {
     FrameScope scope(masm, StackFrame::INTERNAL);
     // Preserve possible return result from lazy deopt.
-    __ Push(x0);
+    __ Push(x0, padreg);
     // Pass the function and deoptimization type to the runtime system.
     __ CallRuntime(Runtime::kNotifyStubFailure, false);
-    __ Pop(x0);
+    __ Pop(padreg, x0);
   }
 
   // Jump to the ContinueToBuiltin stub. Deoptimizer::EntryGenerator::Generate
@@ -1528,30 +1528,55 @@ void Generate_ContinueToBuiltinHelper(MacroAssembler* masm,
                                       bool with_result) {
   const RegisterConfiguration* config(RegisterConfiguration::Default());
   int allocatable_register_count = config->num_allocatable_general_registers();
+  int frame_size = BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp +
+                   (allocatable_register_count +
+                    BuiltinContinuationFrameConstants::PaddingSlotCount(
+                        allocatable_register_count)) *
+                       kPointerSize;
+
+  // Set up frame pointer.
+  __ Add(fp, jssp, frame_size);
+
   if (with_result) {
     // Overwrite the hole inserted by the deoptimizer with the return value from
     // the LAZY deopt point.
-    __ Str(x0, MemOperand(
-                   jssp,
-                   config->num_allocatable_general_registers() * kPointerSize +
-                       BuiltinContinuationFrameConstants::kFixedFrameSize));
-  }
-  for (int i = allocatable_register_count - 1; i >= 0; --i) {
-    int code = config->GetAllocatableGeneralCode(i);
-    __ Pop(Register::from_code(code));
-    if (java_script_builtin && code == kJavaScriptCallArgCountRegister.code()) {
-      __ SmiUntag(Register::from_code(code));
-    }
-  }
-  __ ldr(fp,
-         MemOperand(jssp,
-                    BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp));
-  __ Pop(ip0);
-  __ Add(jssp, jssp,
-         Operand(BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp));
-  __ Pop(lr);
-  __ Add(ip0, ip0, Operand(Code::kHeaderSize - kHeapObjectTag));
-  __ Br(ip0);
+    __ Str(x0,
+           MemOperand(fp, BuiltinContinuationFrameConstants::kCallerSPOffset));
+  }
+
+  // Restore registers in pairs.
+  int offset = -BuiltinContinuationFrameConstants::kFixedFrameSizeFromFp -
+               allocatable_register_count * kPointerSize;
+  for (int i = allocatable_register_count - 1; i > 0; i -= 2) {
+    int code1 = config->GetAllocatableGeneralCode(i);
+    int code2 = config->GetAllocatableGeneralCode(i - 1);
+    Register reg1 = Register::from_code(code1);
+    Register reg2 = Register::from_code(code2);
+    __ Ldp(reg1, reg2, MemOperand(fp, offset));
+    offset += 2 * kPointerSize;
+  }
+
+  // Restore first register separately, if number of registers is odd.
+  if (allocatable_register_count % 2 != 0) {
+    int code = config->GetAllocatableGeneralCode(0);
+    __ Ldr(Register::from_code(code), MemOperand(fp, offset));
+  }
+
+  if (java_script_builtin) __ SmiUntag(kJavaScriptCallArgCountRegister);
+
+  // Load builtin object.
+  UseScratchRegisterScope temps(masm);
+  Register builtin = temps.AcquireX();
+  __ Ldr(builtin,
+         MemOperand(fp, BuiltinContinuationFrameConstants::kBuiltinOffset));
+
+  // Restore fp, lr.
+  __ Mov(__ StackPointer(), fp);
+  __ Pop(fp, lr);
+
+  // Call builtin.
+  __ Add(builtin, builtin, Code::kHeaderSize - kHeapObjectTag);
+  __ Br(builtin);
 }
 }  // namespace
 

src/deoptimizer.cc

@@ -1581,6 +1581,9 @@ void Deoptimizer::DoComputeBuiltinContinuation(
 
   const RegisterConfiguration* config(RegisterConfiguration::Default());
   int allocatable_register_count = config->num_allocatable_general_registers();
+  int padding_slot_count = BuiltinContinuationFrameConstants::PaddingSlotCount(
+      allocatable_register_count);
+
   int register_parameter_count =
       continuation_descriptor.GetRegisterParameterCount();
   // Make sure to account for the context by removing it from the register
@@ -1588,8 +1591,9 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   int stack_param_count = height_in_words - register_parameter_count - 1;
   if (must_handle_result) stack_param_count++;
   int output_frame_size =
-      kPointerSize * (stack_param_count + allocatable_register_count) +
-      TYPED_FRAME_SIZE(2);  // For destination builtin code and registers
+      kPointerSize * (stack_param_count + allocatable_register_count +
+                      padding_slot_count) +
+      BuiltinContinuationFrameConstants::kFixedFrameSize;
 
   // Validate types of parameters. They must all be tagged except for argc for
   // JS builtins.
@@ -1633,7 +1637,9 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   }
   output_frame->SetTop(top_address);
 
-  // Get the possible JSFunction for the case that
+  // Get the possible JSFunction for the case that this is a
+  // JavaScriptBuiltinContinuationFrame, which needs the JSFunction pointer
+  // like a normal JavaScriptFrame.
   intptr_t maybe_function =
       reinterpret_cast<intptr_t>(value_iterator->GetRawValue());
   ++input_index;
@@ -1799,6 +1805,14 @@ void Deoptimizer::DoComputeBuiltinContinuation(
   Register fp_reg = JavaScriptFrame::fp_register();
   output_frame->SetRegister(fp_reg.code(), output_[frame_index - 1]->GetFp());
 
+  // Some architectures must pad the stack frame with extra stack slots
+  // to ensure the stack frame is aligned.
+  for (int i = 0; i < padding_slot_count; ++i) {
+    output_frame_offset -= kPointerSize;
+    WriteValueToOutput(isolate()->heap()->the_hole_value(), 0, frame_index,
+                       output_frame_offset, "padding ");
+  }
+
   Code* continue_to_builtin =
       java_script_builtin
           ? (must_handle_result

src/frame-constants.h

@@ -251,8 +251,16 @@ class BuiltinContinuationFrameConstants : public TypedFrameConstants {
   // FP-relative.
   static const int kFunctionOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(0);
   static const int kBuiltinOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(1);
+  // The argument count is in the first allocatable register, stored below the
+  // fixed part of the frame and therefore is not part of the fixed frame size.
   static const int kArgCOffset = TYPED_FRAME_PUSHED_VALUE_OFFSET(2);
   DEFINE_TYPED_FRAME_SIZES(2);
+
+  // Returns the number of padding stack slots needed when we have
+  // 'register_count' register slots.
+  // This is needed on some architectures to ensure the stack pointer is
+  // aligned.
+  static int PaddingSlotCount(int register_count);
 };
 
 // Behaves like an exit frame but with target and new target args.

src/frames.cc

@@ -1119,6 +1119,10 @@ int JavaScriptFrame::ComputeParametersCount() const {
 }
 
 int JavaScriptBuiltinContinuationFrame::ComputeParametersCount() const {
+  // Assert that the first allocatable register is also the argument count
+  // register.
+  DCHECK_EQ(RegisterConfiguration::Default()->GetAllocatableGeneralCode(0),
+            kJavaScriptCallArgCountRegister.code());
   Object* argc_object =
       Memory::Object_at(fp() + BuiltinContinuationFrameConstants::kArgCOffset);
   return Smi::ToInt(argc_object);

src/ia32/frame-constants-ia32.cc

@@ -22,6 +22,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/mips/frame-constants-mips.cc

@@ -22,6 +22,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/mips64/frame-constants-mips64.cc

@@ -22,6 +22,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/ppc/frame-constants-ppc.cc

@@ -27,6 +27,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/s390/frame-constants-s390.cc

@@ -24,6 +24,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/x64/frame-constants-x64.cc

@@ -22,6 +22,11 @@ int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
   return register_count;
 }
 
+int BuiltinContinuationFrameConstants::PaddingSlotCount(int register_count) {
+  USE(register_count);
+  return 0;
+}
+
 }  // namespace internal
 }  // namespace v8
 

test/mjsunit/optimized-map.js

@@ -101,6 +101,27 @@ var c = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25];
   lazyDeopt();
 })();
 
+// Escape analyzed array where callback function isn't inlined, forcing a lazy
+// deopt. Check that the result of the callback function is passed correctly
+// to the lazy deopt and that the final result of map is as expected.
+(function() {
+  var lazyDeopt = function(deopt) {
+    var b = [1,2,3];
+    var callback = function(v,i,o) {
+      if (i == 1 && deopt) {
+        %DeoptimizeFunction(lazyDeopt);
+      }
+      return 2 * v;
+    };
+    %NeverOptimizeFunction(callback);
+    return b.map(callback);
+  }
+  assertEquals([2,4,6], lazyDeopt());
+  assertEquals([2,4,6], lazyDeopt());
+  %OptimizeFunctionOnNextCall(lazyDeopt);
+  assertEquals([2,4,6], lazyDeopt(true));
+})();
+
 // Lazy deopt from runtime call from inlined callback function.
 (function() {
   var result = 0;