New calling convention for Zero and Shark
Gary Benson
gbenson at redhat.com
Fri May 14 06:05:07 PDT 2010
Hi all,
This commit fixes PR icedtea/484, a bug where the native code for
deoptimized methods could be freed when in fact they were still on
the stack and waiting to return.
To make this change I have had to change the calling convention
within Zero and Shark. All method entries (the C function that
executes the method) now return an integer which is the number
of deoptimized frames they have left on the stack. Whenever a
method is called it is now the caller's responsibility to check
whether frames have been deoptimized and reenter the interpreter
if they have.
The ARM interpreter needs updating to follow the new convention.
I have made a note of this in zeroshark.make, where the code that
enables the ARM interpreter is currently commented out.
Cheers,
Gary
--
http://gbenson.net/
-------------- next part --------------
diff -r f50bc2a9120e ChangeLog
--- a/ChangeLog Fri May 14 11:16:36 2010 +0100
+++ b/ChangeLog Fri May 14 13:54:40 2010 +0100
@@ -1,3 +1,46 @@
+2010-05-14 Gary Benson <gbenson at redhat.com>
+
+ PR icedtea/484
+ * ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.hpp
+ (CppInterpreter::normal_entry): Return int instead of void.
+ (CppInterpreter::native_entry): Likewise.
+ (CppInterpreter::accessor_entry): Likewise.
+ (CppInterpreter::empty_entry): Likewise.
+ * ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp
+ (CppInterpreter::normal_entry): Return 0.
+ (CppInterpreter::native_entry): Likewise.
+ (CppInterpreter::accessor_entry): Likewise.
+ (CppInterpreter::empty_entry): Likewise.
+ * ports/hotspot/src/cpu/zero/vm/entry_zero.hpp
+ (ZeroEntry::NormalEntryFunc): Return int instead of void.
+ (ZeroEntry::OSREntryFunc): Likewise.
+ (ZeroEntry::invoke): Deoptimize where necessary.
+ (ZeroEntry::invoke_osr): Likewise.
+ (ZeroEntry::maybe_deoptimize): New method.
+ * ports/hotspot/src/share/vm/shark/sharkBuilder.hpp
+ (SharkBuilder::deoptimized_entry_point): New method.
+ * ports/hotspot/src/share/vm/shark/sharkBuilder.hpp
+ (SharkBuilder::uncommon_trap): Return int instead of void.
+ (SharkBuilder::deoptimized_entry_point): New method.
+ * ports/hotspot/src/share/vm/shark/sharkContext.cpp
+ (SharkContext::SharkContext): Updated entry point types.
+ * ports/hotspot/src/share/vm/shark/sharkNativeWrapper.cpp
+ (SharkNativeWrapper::initialize): Make generated wrappers
+ return 0 instead of void.
+ * ports/hotspot/src/share/vm/shark/sharkRuntime.cpp
+ (SharkRuntime::uncommon_trap): Return int instead of void.
+ * ports/hotspot/src/share/vm/shark/sharkRuntime.cpp
+ (SharkRuntime::uncommon_trap): Don't enter the interpreter,
+ just return the number of frames that have been deoptimized.
+ * ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
+ (SharkTopLevelBlock::do_trap): Return the number of deoptimized
+ frames instead of void.
+ (SharkTopLevelBlock::handle_return): Return 0 instead of void.
+ (SharkTopLevelBlock::do_call): Deoptimize where necessary.
+
+ * ports/hotspot/make/linux/makefiles/zeroshark.make:
+ Note that ARM interpreter needs updating for this change too.
+
2010-05-14 Gary Benson <gbenson at redhat.com>
* ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp
diff -r f50bc2a9120e ports/hotspot/make/linux/makefiles/zeroshark.make
--- a/ports/hotspot/make/linux/makefiles/zeroshark.make Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/make/linux/makefiles/zeroshark.make Fri May 14 13:54:40 2010 +0100
@@ -31,7 +31,8 @@
Obj_Files += cppInterpreter_arm.o
Obj_Files += thumb2.o
-#XXX disabled until it has the updated frame anchor code
+#XXX disabled until it has the updated frame anchor code (PR icedtea/323)
+#XXX and the updated calling convention for deopt (PR icedtea/484)
#CFLAGS += -DHOTSPOT_ASM
%.o: %.S
diff -r f50bc2a9120e ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp
--- a/ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp Fri May 14 13:54:40 2010 +0100
@@ -37,15 +37,18 @@
thread->reset_last_Java_frame(); \
fixup_after_potential_safepoint()
-void CppInterpreter::normal_entry(methodOop method, intptr_t UNUSED, TRAPS) {
+int CppInterpreter::normal_entry(methodOop method, intptr_t UNUSED, TRAPS) {
JavaThread *thread = (JavaThread *) THREAD;
// Allocate and initialize our frame.
- InterpreterFrame *frame = InterpreterFrame::build(method, CHECK);
+ InterpreterFrame *frame = InterpreterFrame::build(method, CHECK_0);
thread->push_zero_frame(frame);
// Execute those bytecodes!
main_loop(0, THREAD);
+
+ // No deoptimized frames on the stack
+ return 0;
}
void CppInterpreter::main_loop(int recurse, TRAPS) {
@@ -165,7 +168,7 @@
stack->push(result[-i]);
}
-void CppInterpreter::native_entry(methodOop method, intptr_t UNUSED, TRAPS) {
+int CppInterpreter::native_entry(methodOop method, intptr_t UNUSED, TRAPS) {
// Make sure method is native and not abstract
assert(method->is_native() && !method->is_abstract(), "should be");
@@ -173,7 +176,7 @@
ZeroStack *stack = thread->zero_stack();
// Allocate and initialize our frame
- InterpreterFrame *frame = InterpreterFrame::build(method, CHECK);
+ InterpreterFrame *frame = InterpreterFrame::build(method, CHECK_0);
thread->push_zero_frame(frame);
interpreterState istate = frame->interpreter_state();
intptr_t *locals = istate->locals();
@@ -430,25 +433,26 @@
ShouldNotReachHere();
}
}
+
+ // No deoptimized frames on the stack
+ return 0;
}
-void CppInterpreter::accessor_entry(methodOop method, intptr_t UNUSED, TRAPS) {
+int CppInterpreter::accessor_entry(methodOop method, intptr_t UNUSED, TRAPS) {
JavaThread *thread = (JavaThread *) THREAD;
ZeroStack *stack = thread->zero_stack();
intptr_t *locals = stack->sp();
// Drop into the slow path if we need a safepoint check
if (SafepointSynchronize::do_call_back()) {
- normal_entry(method, 0, THREAD);
- return;
+ return normal_entry(method, 0, THREAD);
}
// Load the object pointer and drop into the slow path
// if we have a NullPointerException
oop object = LOCALS_OBJECT(0);
if (object == NULL) {
- normal_entry(method, 0, THREAD);
- return;
+ return normal_entry(method, 0, THREAD);
}
// Read the field index from the bytecode, which looks like this:
@@ -470,15 +474,14 @@
constantPoolCacheOop cache = method->constants()->cache();
ConstantPoolCacheEntry* entry = cache->entry_at(index);
if (!entry->is_resolved(Bytecodes::_getfield)) {
- normal_entry(method, 0, THREAD);
- return;
+ return normal_entry(method, 0, THREAD);
}
// Get the result and push it onto the stack
switch (entry->flag_state()) {
case ltos:
case dtos:
- stack->overflow_check(1, CHECK);
+ stack->overflow_check(1, CHECK_0);
stack->alloc(wordSize);
break;
}
@@ -558,20 +561,25 @@
ShouldNotReachHere();
}
}
+
+ // No deoptimized frames on the stack
+ return 0;
}
-void CppInterpreter::empty_entry(methodOop method, intptr_t UNUSED, TRAPS) {
+int CppInterpreter::empty_entry(methodOop method, intptr_t UNUSED, TRAPS) {
JavaThread *thread = (JavaThread *) THREAD;
ZeroStack *stack = thread->zero_stack();
// Drop into the slow path if we need a safepoint check
if (SafepointSynchronize::do_call_back()) {
- normal_entry(method, 0, THREAD);
- return;
+ return normal_entry(method, 0, THREAD);
}
// Pop our parameters
stack->set_sp(stack->sp() + method->size_of_parameters());
+
+ // No deoptimized frames on the stack
+ return 0;
}
InterpreterFrame *InterpreterFrame::build(const methodOop method, TRAPS) {
diff -r f50bc2a9120e ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.hpp
--- a/ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.hpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/cpu/zero/vm/cppInterpreter_zero.hpp Fri May 14 13:54:40 2010 +0100
@@ -29,10 +29,10 @@
public:
// Method entries
- static void normal_entry(methodOop method, intptr_t UNUSED, TRAPS);
- static void native_entry(methodOop method, intptr_t UNUSED, TRAPS);
- static void accessor_entry(methodOop method, intptr_t UNUSED, TRAPS);
- static void empty_entry(methodOop method, intptr_t UNUSED, TRAPS);
+ static int normal_entry(methodOop method, intptr_t UNUSED, TRAPS);
+ static int native_entry(methodOop method, intptr_t UNUSED, TRAPS);
+ static int accessor_entry(methodOop method, intptr_t UNUSED, TRAPS);
+ static int empty_entry(methodOop method, intptr_t UNUSED, TRAPS);
public:
// Main loop of normal_entry
diff -r f50bc2a9120e ports/hotspot/src/cpu/zero/vm/entry_zero.hpp
--- a/ports/hotspot/src/cpu/zero/vm/entry_zero.hpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/cpu/zero/vm/entry_zero.hpp Fri May 14 13:54:40 2010 +0100
@@ -1,6 +1,6 @@
/*
* Copyright 2003-2007 Sun Microsystems, Inc. All Rights Reserved.
- * Copyright 2008, 2009 Red Hat, Inc.
+ * Copyright 2008, 2009, 2010 Red Hat, Inc.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -41,20 +41,30 @@
}
private:
- typedef void (*NormalEntryFunc)(methodOop method,
- intptr_t base_pc,
- TRAPS);
- typedef void (*OSREntryFunc)(methodOop method,
- address osr_buf,
- intptr_t base_pc,
- TRAPS);
+ typedef int (*NormalEntryFunc)(methodOop method,
+ intptr_t base_pc,
+ TRAPS);
+ typedef int (*OSREntryFunc)(methodOop method,
+ address osr_buf,
+ intptr_t base_pc,
+ TRAPS);
public:
void invoke(methodOop method, TRAPS) const {
- ((NormalEntryFunc) entry_point())(method, (intptr_t) this, THREAD);
+ maybe_deoptimize(
+ ((NormalEntryFunc) entry_point())(method, (intptr_t) this, THREAD),
+ THREAD);
}
void invoke_osr(methodOop method, address osr_buf, TRAPS) const {
- ((OSREntryFunc) entry_point())(method, osr_buf, (intptr_t) this, THREAD);
+ maybe_deoptimize(
+ ((OSREntryFunc) entry_point())(method, osr_buf, (intptr_t) this, THREAD),
+ THREAD);
+ }
+
+ private:
+ static void maybe_deoptimize(int deoptimized_frames, TRAPS) {
+ if (deoptimized_frames)
+ CppInterpreter::main_loop(deoptimized_frames - 1, THREAD);
}
public:
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkBuilder.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkBuilder.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkBuilder.cpp Fri May 14 13:54:40 2010 +0100
@@ -326,7 +326,11 @@
}
Value* SharkBuilder::uncommon_trap() {
- return make_function((address) SharkRuntime::uncommon_trap, "Ti", "v");
+ return make_function((address) SharkRuntime::uncommon_trap, "Ti", "i");
+}
+
+Value* SharkBuilder::deoptimized_entry_point() {
+ return make_function((address) CppInterpreter::main_loop, "iT", "v");
}
// Native-Java transition
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkBuilder.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkBuilder.hpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkBuilder.hpp Fri May 14 13:54:40 2010 +0100
@@ -133,6 +133,7 @@
public:
llvm::Value* throw_StackOverflowError();
llvm::Value* uncommon_trap();
+ llvm::Value* deoptimized_entry_point();
// Intrinsics and external functions, part 4: Native-Java transition.
// This is a special case in that it is invoked during a thread
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkContext.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkContext.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkContext.cpp Fri May 14 13:54:40 2010 +0100
@@ -1,6 +1,6 @@
/*
* Copyright 1999-2007 Sun Microsystems, Inc. All Rights Reserved.
- * Copyright 2009 Red Hat, Inc.
+ * Copyright 2009, 2010 Red Hat, Inc.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -76,14 +76,14 @@
params.push_back(methodOop_type());
params.push_back(intptr_type());
params.push_back(thread_type());
- _entry_point_type = FunctionType::get(void_type(), params, false);
+ _entry_point_type = FunctionType::get(jint_type(), params, false);
params.clear();
params.push_back(methodOop_type());
params.push_back(PointerType::getUnqual(jbyte_type()));
params.push_back(intptr_type());
params.push_back(thread_type());
- _osr_entry_point_type = FunctionType::get(void_type(), params, false);
+ _osr_entry_point_type = FunctionType::get(jint_type(), params, false);
// Create mappings
for (int i = 0; i < T_CONFLICT; i++) {
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkNativeWrapper.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkNativeWrapper.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkNativeWrapper.cpp Fri May 14 13:54:40 2010 +0100
@@ -283,7 +283,7 @@
builder()->SetInsertPoint(exception);
CreateResetHandleBlock();
stack()->CreatePopFrame(0);
- builder()->CreateRetVoid();
+ builder()->CreateRet(LLVMValue::jint_constant(0));
builder()->SetInsertPoint(no_exception);
@@ -348,5 +348,5 @@
result_addr,
PointerType::getUnqual(SharkType::to_stackType(result_type))));
}
- builder()->CreateRetVoid();
+ builder()->CreateRet(LLVMValue::jint_constant(0));
}
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkRuntime.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkRuntime.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkRuntime.cpp Fri May 14 13:54:40 2010 +0100
@@ -192,14 +192,14 @@
return object_klass->klass_part()->is_subtype_of(check_klass);
}
-void SharkRuntime::uncommon_trap(JavaThread* thread, int trap_request) {
+int SharkRuntime::uncommon_trap(JavaThread* thread, int trap_request) {
+ Thread *THREAD = thread;
+
// In C2, uncommon_trap_blob creates a frame, so all the various
// deoptimization functions expect to find the frame of the method
// being deopted one frame down on the stack. We create a dummy
// frame to mirror this.
- FakeStubFrame *stubframe = FakeStubFrame::build(thread);
- if (thread->has_pending_exception())
- return;
+ FakeStubFrame *stubframe = FakeStubFrame::build(CHECK_0);
thread->push_zero_frame(stubframe);
// Initiate the trap
@@ -216,16 +216,12 @@
int number_of_frames = urb->number_of_frames();
for (int i = 0; i < number_of_frames; i++) {
intptr_t size = urb->frame_sizes()[i];
- InterpreterFrame *frame = InterpreterFrame::build(size, thread);
- if (thread->has_pending_exception())
- return;
+ InterpreterFrame *frame = InterpreterFrame::build(size, CHECK_0);
thread->push_zero_frame(frame);
}
// Push another dummy frame
- stubframe = FakeStubFrame::build(thread);
- if (thread->has_pending_exception())
- return;
+ stubframe = FakeStubFrame::build(CHECK_0);
thread->push_zero_frame(stubframe);
// Fill in the skeleton frames
@@ -236,12 +232,8 @@
// Pop our dummy frame
thread->pop_zero_frame();
- // Jump into the interpreter
-#ifdef CC_INTERP
- CppInterpreter::main_loop(number_of_frames - 1, thread);
-#else
- Unimplemented();
-#endif // CC_INTERP
+ // Fall back into the interpreter
+ return number_of_frames;
}
FakeStubFrame* FakeStubFrame::build(TRAPS) {
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkRuntime.hpp
--- a/ports/hotspot/src/share/vm/shark/sharkRuntime.hpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkRuntime.hpp Fri May 14 13:54:40 2010 +0100
@@ -1,6 +1,6 @@
/*
* Copyright 1999-2007 Sun Microsystems, Inc. All Rights Reserved.
- * Copyright 2008, 2009 Red Hat, Inc.
+ * Copyright 2008, 2009, 2010 Red Hat, Inc.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -79,5 +79,5 @@
public:
static void dump(const char *name, intptr_t value);
static bool is_subtype_of(klassOop check_klass, klassOop object_klass);
- static void uncommon_trap(JavaThread* thread, int trap_request);
+ static int uncommon_trap(JavaThread* thread, int trap_request);
};
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkStack.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkStack.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkStack.cpp Fri May 14 13:54:40 2010 +0100
@@ -135,7 +135,7 @@
// Handle overflows
builder()->SetInsertPoint(overflow);
builder()->CreateCall(builder()->throw_StackOverflowError(), thread());
- builder()->CreateRetVoid();
+ builder()->CreateRet(LLVMValue::jint_constant(0));
builder()->SetInsertPoint(abi_ok);
}
diff -r f50bc2a9120e ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp
--- a/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Fri May 14 11:16:36 2010 +0100
+++ b/ports/hotspot/src/share/vm/shark/sharkTopLevelBlock.cpp Fri May 14 13:54:40 2010 +0100
@@ -594,11 +594,11 @@
void SharkTopLevelBlock::do_trap(int trap_request) {
decache_for_trap();
- builder()->CreateCall2(
- builder()->uncommon_trap(),
- thread(),
- LLVMValue::jint_constant(trap_request));
- builder()->CreateRetVoid();
+ builder()->CreateRet(
+ builder()->CreateCall2(
+ builder()->uncommon_trap(),
+ thread(),
+ LLVMValue::jint_constant(trap_request)));
}
void SharkTopLevelBlock::call_register_finalizer(Value *receiver) {
@@ -677,7 +677,7 @@
PointerType::getUnqual(SharkType::to_stackType(type))));
}
- builder()->CreateRetVoid();
+ builder()->CreateRet(LLVMValue::jint_constant(0));
}
void SharkTopLevelBlock::do_arraylength() {
@@ -1203,7 +1203,25 @@
// Make the call
decache_for_Java_call(call_method);
- builder()->CreateCall3(entry_point, callee, base_pc, thread());
+ Value *deoptimized_frames = builder()->CreateCall3(
+ entry_point, callee, base_pc, thread());
+
+ // If the callee got deoptimized then reexecute in the interpreter
+ BasicBlock *reexecute = function()->CreateBlock("reexecute");
+ BasicBlock *call_completed = function()->CreateBlock("call_completed");
+ builder()->CreateCondBr(
+ builder()->CreateICmpNE(deoptimized_frames, LLVMValue::jint_constant(0)),
+ reexecute, call_completed);
+
+ builder()->SetInsertPoint(reexecute);
+ builder()->CreateCall2(
+ builder()->deoptimized_entry_point(),
+ builder()->CreateSub(deoptimized_frames, LLVMValue::jint_constant(1)),
+ thread());
+ builder()->CreateBr(call_completed);
+
+ // Cache after the call
+ builder()->SetInsertPoint(call_completed);
cache_after_Java_call(call_method);
// Check for pending exceptions
More information about the distro-pkg-dev
mailing list