Skip to content

Commit

Permalink
[arm64] Pad registers in interpreter frame.
Browse files Browse the repository at this point in the history
Add padding for the interpreter registers when needed, to make the
interpreter frame a multiple of 16 bytes. The padding needs to be added
in the InterpreterEntryTrampoline and when generating an interpreter
frame in the deoptimizer. It also needs to be considered when
calculating the size of the interpreter frame during OSR and stack
unwinding.

Bug: v8:6644
Change-Id: Icfec94079cf0785fc8a2506ff555b5f9e89e3d13
Reviewed-on: https://chromium-review.googlesource.com/664563
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48121}
  • Loading branch information
g-kouv authored and Commit Bot committed Sep 22, 2017
1 parent 2098e9c commit 9f01414
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 12 deletions.
4 changes: 4 additions & 0 deletions src/arm/frame-constants-arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Register JavaScriptFrame::fp_register() { return v8::internal::fp; }
Register JavaScriptFrame::context_register() { return cp; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
5 changes: 5 additions & 0 deletions src/arm64/frame-constants-arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ Register JavaScriptFrame::fp_register() { return v8::internal::fp; }
Register JavaScriptFrame::context_register() { return cp; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
// Round up to a multiple of two, to make the frame a multiple of 16 bytes.
return RoundUp(register_count, 2);
}

} // namespace internal
} // namespace v8

Expand Down
6 changes: 4 additions & 2 deletions src/builtins/arm64/builtins-arm64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -981,9 +981,11 @@ void Builtins::Generate_InterpreterEntryTrampoline(MacroAssembler* masm) {
// register in the register file.
Label loop_header;
__ LoadRoot(x10, Heap::kUndefinedValueRootIndex);
// TODO(rmcilroy): Ensure we always have an even number of registers to
// allow stack to be 16 bit aligned (and remove need for jssp).
__ Lsr(x11, x11, kPointerSizeLog2);
// Round up the number of registers to a multiple of 2, to align the stack
// to 16 bytes.
__ Add(x11, x11, 1);
__ Bic(x11, x11, 1);
__ PushMultipleTimes(x10, x11);
__ Bind(&loop_header);
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/osr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ OsrHelper::OsrHelper(CompilationInfo* info)
: parameter_count_(
info->shared_info()->bytecode_array()->parameter_count()),
stack_slot_count_(
info->shared_info()->bytecode_array()->register_count() +
InterpreterFrameConstants::RegisterStackSlotCount(
info->shared_info()->bytecode_array()->register_count()) +
InterpreterFrameConstants::kExtraSlotCount) {}

void OsrHelper::SetupFrame(Frame* frame) {
Expand Down
29 changes: 21 additions & 8 deletions src/deoptimizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -736,14 +736,16 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
int input_index = 0;

int bytecode_offset = translated_frame->node_id().ToInt();
unsigned height = translated_frame->height();
unsigned height_in_bytes = height * kPointerSize;
int height = translated_frame->height();
int register_count = height - 1; // Exclude accumulator.
int register_stack_slot_count =
InterpreterFrameConstants::RegisterStackSlotCount(register_count);
int height_in_bytes = register_stack_slot_count * kPointerSize;

// All tranlations for interpreted frames contain the accumulator and hence
// are assumed to be in bailout state {BailoutState::TOS_REGISTER}. However
// such a state is only supported for the topmost frame. We need to skip
// pushing the accumulator for any non-topmost frame.
if (!is_topmost) height_in_bytes -= kPointerSize;
// The topmost frame is assumed to be in bailout state
// {BailoutState::TOS_REGISTER} and will contain the accumulator, which we
// add to the frame height here.
if (is_topmost) height_in_bytes += kPointerSize;

JSFunction* function = JSFunction::cast(value_iterator->GetRawValue());
value_iterator++;
Expand Down Expand Up @@ -912,12 +914,23 @@ void Deoptimizer::DoComputeInterpretedFrame(TranslatedFrame* translated_frame,
}

// Translate the rest of the interpreter registers in the frame.
for (unsigned i = 0; i < height - 1; ++i) {
for (int i = 0; i < register_count; ++i) {
output_offset -= kPointerSize;
WriteTranslatedValueToOutput(&value_iterator, &input_index, frame_index,
output_offset);
}

int register_slots_written = register_count;
DCHECK_LE(register_slots_written, register_stack_slot_count);
// Some architectures must pad the stack frame with extra stack slots
// to ensure the stack frame is aligned. Do this now.
while (register_slots_written < register_stack_slot_count) {
register_slots_written++;
output_offset -= kPointerSize;
WriteValueToOutput(isolate()->heap()->the_hole_value(), 0, frame_index,
output_offset, "padding ");
}

// Translate the accumulator register (depending on frame position).
if (is_topmost) {
// For topmost frame, put the accumulator on the stack. The bailout state
Expand Down
5 changes: 5 additions & 0 deletions src/frame-constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ class InterpreterFrameConstants : public AllStatic {
static const int kBytecodeArrayExpressionIndex = -2;
static const int kBytecodeOffsetExpressionIndex = -1;
static const int kRegisterFileExpressionIndex = 0;

// Returns the number of stack slots needed for 'register_count' registers.
// This is needed because some architectures must pad the stack frame with
// additional stack slots to ensure the stack pointer is aligned.
static int RegisterStackSlotCount(int register_count);
};

inline static int FPOffsetToFrameSlot(int frame_offset) {
Expand Down
4 changes: 4 additions & 0 deletions src/ia32/frame-constants-ia32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Register JavaScriptFrame::fp_register() { return ebp; }
Register JavaScriptFrame::context_register() { return esi; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
3 changes: 2 additions & 1 deletion src/isolate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,8 @@ Object* Isolate::UnwindAndFindHandler() {
// For interpreted frame we perform a range lookup in the handler table.
if (!catchable_by_js) break;
InterpretedFrame* js_frame = static_cast<InterpretedFrame*>(frame);
int register_slots = js_frame->GetBytecodeArray()->register_count();
int register_slots = InterpreterFrameConstants::RegisterStackSlotCount(
js_frame->GetBytecodeArray()->register_count());
int context_reg = 0; // Will contain register index holding context.
int offset =
js_frame->LookupExceptionHandlerInTable(&context_reg, nullptr);
Expand Down
4 changes: 4 additions & 0 deletions src/mips/frame-constants-mips.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Register JavaScriptFrame::fp_register() { return v8::internal::fp; }
Register JavaScriptFrame::context_register() { return cp; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
4 changes: 4 additions & 0 deletions src/mips64/frame-constants-mips64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Register JavaScriptFrame::fp_register() { return v8::internal::fp; }
Register JavaScriptFrame::context_register() { return cp; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
4 changes: 4 additions & 0 deletions src/ppc/frame-constants-ppc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ Register JavaScriptFrame::constant_pool_pointer_register() {
return kConstantPoolRegister;
}

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
4 changes: 4 additions & 0 deletions src/s390/frame-constants-s390.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Register JavaScriptFrame::fp_register() { return v8::internal::fp; }
Register JavaScriptFrame::context_register() { return cp; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down
4 changes: 4 additions & 0 deletions src/x64/frame-constants-x64.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Register JavaScriptFrame::fp_register() { return rbp; }
Register JavaScriptFrame::context_register() { return rsi; }
Register JavaScriptFrame::constant_pool_pointer_register() { UNREACHABLE(); }

int InterpreterFrameConstants::RegisterStackSlotCount(int register_count) {
return register_count;
}

} // namespace internal
} // namespace v8

Expand Down

0 comments on commit 9f01414

Please sign in to comment.