Add dynamic stack frame alignment to optimized functions with untagged doubles on the stack.

src/ia32/deoptimizer-ia32.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 422 matching lines @@
     unsigned pc_offset = data->OsrPcOffset()->value();
     uint32_t pc = reinterpret_cast<uint32_t>(
         optimized_code_->entry() + pc_offset);
     output_[0]->SetPc(pc);
   }
   Code* continuation =
       function->GetIsolate()->builtins()->builtin(Builtins::kNotifyOSR);
   output_[0]->SetContinuation(
       reinterpret_cast<uint32_t>(continuation->entry()));
 
+  // All OSR stack frames are dynamically aligned to an 8-byte boundary.
+  int frame_pointer = output_[0]->GetRegister(ebp.code());

[Kevin Millikin (Chromium), 2011/09/23 09:38:49]

Don't put this here (i.e., don't set ebp register and then patch it up just
later).  It's too easy to miss what's going on.

Please just set it to the value you want in the first place, about 8 lines
above.

+  if ((frame_pointer & 0x4) == 0) {
+    // Return address at FP + 4 should be aligned, so FP mod 8 should be 4.
+    output_[0]->SetRegister(ebp.code(), frame_pointer - kPointerSize);
+    has_alignment_padding_ = 1;
+  }
+
   if (FLAG_trace_osr) {
     PrintF("[on-stack replacement translation %s: 0x%08" V8PRIxPTR " ",
            ok ? "finished" : "aborted",
            reinterpret_cast<intptr_t>(function));
     function->PrintName();
     PrintF(" => pc=0x%0x]\n", output_[0]->GetPc());
   }
 }
 
 
@@ skipping 30 matching lines @@
   bool is_bottommost = (0 == frame_index);
   bool is_topmost = (output_count_ - 1 == frame_index);
   ASSERT(frame_index >= 0 && frame_index < output_count_);
   ASSERT(output_[frame_index] == NULL);
   output_[frame_index] = output_frame;
 
   // The top address for the bottommost output frame can be computed from
   // the input frame pointer and the output frame's height.  For all
   // subsequent output frames, it can be computed from the previous one's
   // top address and the current frame's size.
+
+  // If the optimized frame had alignment padding, adjust the frame pointer

[Kevin Millikin (Chromium), 2011/09/23 09:38:49]

I also don't like this here.  First, it breaks up the comment about setting the
top address so it's now far from the code that sets the top address.

Worse, I think it's a bug waiting to happen to just spoof the value in the input
ebp register.  I'd rather leave the correct value there and adjust for padding
in a couple of places.

[William Hesse, 2011/09/23 12:50:11]

Done.

+  // to point to the new position of the old frame pointer after padding
+  // is removed.
+  if (has_alignment_padding_ == 1) {
+    input_->SetRegister(ebp.code(),
+                        input_->GetRegister(ebp.code()) + kPointerSize);
+  }
+
   uint32_t top_address;
   if (is_bottommost) {
     // 2 = context and function in the frame.
     top_address =
         input_->GetRegister(ebp.code()) - (2 * kPointerSize) - height_in_bytes;

[Kevin Millikin (Chromium), 2011/09/23 09:38:49]

Here:

input_->GetRegister(ebp.code()) - (2 * kPointerSize) - height_in_bytes +
(has_alignment_padding_ * kPointerSize)

[William Hesse, 2011/09/23 12:50:11]

Done.

   } else {
     top_address = output_[frame_index - 1]->GetTop() - output_frame_size;
   }
   output_frame->SetTop(top_address);
 
   // Compute the incoming parameter translation.
   int parameter_count = function->shared()->formal_parameter_count() + 1;
   unsigned output_offset = output_frame_size;
   unsigned input_offset = input_frame_size;
   for (int i = 0; i < parameter_count; ++i) {
@@ skipping 30 matching lines @@
   // pointer.
   output_offset -= kPointerSize;
   input_offset -= kPointerSize;
   if (is_bottommost) {
     value = input_->GetFrameSlot(input_offset);
   } else {
     value = output_[frame_index - 1]->GetFp();
   }
   output_frame->SetFrameSlot(output_offset, value);
   intptr_t fp_value = top_address + output_offset;
   ASSERT(!is_bottommost || input_->GetRegister(ebp.code()) == fp_value);

[Kevin Millikin (Chromium), 2011/09/23 09:38:49]

Here:

ASSERT(!is_bottommost ||
       input_->GetRegister(ebp.code()) + has_alignment_padding * kPointerSize ==
fp_value);

[William Hesse, 2011/09/23 12:50:11]

Done.

   output_frame->SetFp(fp_value);
   if (is_topmost) output_frame->SetRegister(ebp.code(), fp_value);
   if (FLAG_trace_deopt) {
     PrintF("    0x%08x: [top + %d] <- 0x%08x ; caller's fp\n",
            fp_value, output_offset, value);
   }
 
   // For the bottommost output frame the context can be gotten from the input
   // frame. For all subsequent output frames it can be gotten from the function
   // so long as we don't inline functions that need local contexts.
@@ skipping 168 matching lines @@
   // limit and copy the contents of the activation frame to the input
   // frame description.
   __ lea(edx, Operand(ebx, FrameDescription::frame_content_offset()));
   Label pop_loop;
   __ bind(&pop_loop);
   __ pop(Operand(edx, 0));
   __ add(Operand(edx), Immediate(sizeof(uint32_t)));
   __ cmp(ecx, Operand(esp));
   __ j(not_equal, &pop_loop);
 
+  // If frame was dynamically aligned, pop padding.
+  Label sentinel, sentinel_done;
+  __ pop(Operand(ecx));
+  __ cmp(ecx, Operand(eax, Deoptimizer::frame_alignment_marker_offset()));
+  __ j(equal, &sentinel);
+  __ push(Operand(ecx));
+  __ jmp(&sentinel_done);
+  __ bind(&sentinel);
+  __ mov(Operand(eax, Deoptimizer::has_alignment_padding_offset()),
+         Immediate(1));
+  __ bind(&sentinel_done);
   // Compute the output frame in the deoptimizer.
   __ push(eax);
   __ PrepareCallCFunction(1, ebx);
   __ mov(Operand(esp, 0 * kPointerSize), eax);
   {
     AllowExternalCallThatCantCauseGC scope(masm());
     __ CallCFunction(
         ExternalReference::compute_output_frames_function(isolate), 1);
   }
   __ pop(eax);
 
+  if (type() == OSR) {
+    // If alignment padding is added, push the sentinel.
+    Label no_osr_padding;
+    __ cmp(Operand(eax, Deoptimizer::has_alignment_padding_offset()),
+           Immediate(0));
+    __ j(equal, &no_osr_padding, Label::kNear);
+    __ push(Operand(eax, Deoptimizer::frame_alignment_marker_offset()));
+    __ bind(&no_osr_padding);
+  }
+
+
   // Replace the current frame with the output frames.
   Label outer_push_loop, inner_push_loop;
   // Outer loop state: eax = current FrameDescription**, edx = one past the
   // last FrameDescription**.
   __ mov(edx, Operand(eax, Deoptimizer::output_count_offset()));
   __ mov(eax, Operand(eax, Deoptimizer::output_offset()));
   __ lea(edx, Operand(eax, edx, times_4, 0));
   __ bind(&outer_push_loop);
   // Inner loop state: ebx = current FrameDescription*, ecx = loop index.
   __ mov(ebx, Operand(eax, 0));
@@ skipping 50 matching lines @@
   }
   __ bind(&done);
 }
 
 #undef __
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32