stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8320310: CompiledMethod::has_monitors flag can be incorrect

Browse Source 
Reviewed-by: vlivanov, thartmann
This commit is contained in:
Jorn Vernee 2024-01-08 14:55:17 +00:00
parent 57a65fe436
commit c8fa3e21e6
5 changed files with 31 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/hotspot/share/c1/c1_Compilation.cpp
View File

@ -390,10 +390,6 @@ int Compilation::compile_java_method() {
BAILOUT_("mdo allocation failed", no_frame_size); BAILOUT_("mdo allocation failed", no_frame_size);
} }
if (method()->is_synchronized()) {
set_has_monitors(true);
}
{ {
PhaseTraceTime timeit(_t_buildIR); PhaseTraceTime timeit(_t_buildIR);
build_hir(); build_hir();
@ -581,7 +577,7 @@ Compilation::Compilation(AbstractCompiler* compiler, ciEnv* env, ciMethod* metho
, _would_profile(false) , _would_profile(false)
, _has_method_handle_invokes(false) , _has_method_handle_invokes(false)
, _has_reserved_stack_access(method->has_reserved_stack_access()) , _has_reserved_stack_access(method->has_reserved_stack_access())
, _has_monitors(false) , _has_monitors(method->is_synchronized() || method->has_monitor_bytecodes())
, _install_code(install_code) , _install_code(install_code)
, _bailout_msg(nullptr) , _bailout_msg(nullptr)
, _first_failure_details(nullptr) , _first_failure_details(nullptr)

22
src/hotspot/share/c1/c1_GraphBuilder.cpp
View File

@ -2316,7 +2316,6 @@ void GraphBuilder::instance_of(int klass_index) {
void GraphBuilder::monitorenter(Value x, int bci) { void GraphBuilder::monitorenter(Value x, int bci) {
// save state before locking in case of deoptimization after a NullPointerException // save state before locking in case of deoptimization after a NullPointerException
ValueStack* state_before = copy_state_for_exception_with_bci(bci); ValueStack* state_before = copy_state_for_exception_with_bci(bci);
compilation()->set_has_monitors(true);
append_with_bci(new MonitorEnter(x, state()->lock(x), state_before), bci); append_with_bci(new MonitorEnter(x, state()->lock(x), state_before), bci);
kill_all(); kill_all();
} }
@ -3510,6 +3509,15 @@ int GraphBuilder::recursive_inline_level(ciMethod* cur_callee) const {
return recur_level; return recur_level;
} }
static void set_flags_for_inlined_callee(Compilation* compilation, ciMethod* callee) {
if (callee->has_reserved_stack_access()) {
compilation->set_has_reserved_stack_access(true);
}
if (callee->is_synchronized() || callee->has_monitor_bytecodes()) {
compilation->set_has_monitors(true);
}
}
bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_return, Bytecodes::Code bc, Value receiver) { bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_return, Bytecodes::Code bc, Value receiver) {
const char* msg = nullptr; const char* msg = nullptr;
@ -3526,9 +3534,7 @@ bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_r
// method handle invokes // method handle invokes
if (callee->is_method_handle_intrinsic()) { if (callee->is_method_handle_intrinsic()) {
if (try_method_handle_inline(callee, ignore_return)) { if (try_method_handle_inline(callee, ignore_return)) {
if (callee->has_reserved_stack_access()) { set_flags_for_inlined_callee(compilation(), callee);
compilation()->set_has_reserved_stack_access(true);
}
return true; return true;
} }
return false; return false;
@ -3539,9 +3545,7 @@ bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_r
callee->check_intrinsic_candidate()) { callee->check_intrinsic_candidate()) {
if (try_inline_intrinsics(callee, ignore_return)) { if (try_inline_intrinsics(callee, ignore_return)) {
print_inlining(callee, "intrinsic"); print_inlining(callee, "intrinsic");
if (callee->has_reserved_stack_access()) { set_flags_for_inlined_callee(compilation(), callee);
compilation()->set_has_reserved_stack_access(true);
}
return true; return true;
} }
// try normal inlining // try normal inlining
@ -3559,9 +3563,7 @@ bool GraphBuilder::try_inline(ciMethod* callee, bool holder_known, bool ignore_r
bc = code(); bc = code();
} }
if (try_inline_full(callee, holder_known, ignore_return, bc, receiver)) { if (try_inline_full(callee, holder_known, ignore_return, bc, receiver)) {
if (callee->has_reserved_stack_access()) { set_flags_for_inlined_callee(compilation(), callee);
compilation()->set_has_reserved_stack_access(true);
}
return true; return true;
} }

6
src/hotspot/share/opto/locknode.cpp
View File

@ -179,8 +179,6 @@ void FastLockNode::create_rtm_lock_counter(JVMState* state) {
void Parse::do_monitor_enter() { void Parse::do_monitor_enter() {
kill_dead_locals(); kill_dead_locals();
C->set_has_monitors(true);
// Null check; get casted pointer. // Null check; get casted pointer.
Node* obj = null_check(peek()); Node* obj = null_check(peek());
// Check for locking null object // Check for locking null object
@ -198,10 +196,6 @@ void Parse::do_monitor_enter() {
void Parse::do_monitor_exit() { void Parse::do_monitor_exit() {
kill_dead_locals(); kill_dead_locals();
// need to set it for monitor exit as well.
// OSR compiled methods can start with lock taken
C->set_has_monitors(true);
pop(); // Pop oop to unlock pop(); // Pop oop to unlock
// Because monitors are guaranteed paired (else we bail out), we know // Because monitors are guaranteed paired (else we bail out), we know
// the matching Lock for this Unlock. Hence we know there is no need // the matching Lock for this Unlock. Hence we know there is no need

2
src/hotspot/share/opto/parse1.cpp
View File

@ -423,7 +423,7 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses)
C->set_has_reserved_stack_access(true); C->set_has_reserved_stack_access(true);
} }
if (parse_method->is_synchronized()) { if (parse_method->is_synchronized() || parse_method->has_monitor_bytecodes()) {
C->set_has_monitors(true); C->set_has_monitors(true);
} }

34
src/hotspot/share/runtime/continuationFreezeThaw.cpp
View File

@ -1498,21 +1498,21 @@ static void jvmti_yield_cleanup(JavaThread* thread, ContinuationWrapper& cont) {
#endif // INCLUDE_JVMTI #endif // INCLUDE_JVMTI
#ifdef ASSERT #ifdef ASSERT
// static bool monitors_on_stack(JavaThread* thread) { static bool monitors_on_stack(JavaThread* thread) {
// ContinuationEntry* ce = thread->last_continuation(); ContinuationEntry* ce = thread->last_continuation();
// RegisterMap map(thread, RegisterMap map(thread,
// RegisterMap::UpdateMap::include, RegisterMap::UpdateMap::include,
// RegisterMap::ProcessFrames::include, RegisterMap::ProcessFrames::include,
// RegisterMap::WalkContinuation::skip); RegisterMap::WalkContinuation::skip);
// map.set_include_argument_oops(false); map.set_include_argument_oops(false);
// for (frame f = thread->last_frame(); Continuation::is_frame_in_continuation(ce, f); f = f.sender(&map)) { for (frame f = thread->last_frame(); Continuation::is_frame_in_continuation(ce, f); f = f.sender(&map)) {
// if ((f.is_interpreted_frame() && ContinuationHelper::InterpretedFrame::is_owning_locks(f)) || if ((f.is_interpreted_frame() && ContinuationHelper::InterpretedFrame::is_owning_locks(f)) ||
// (f.is_compiled_frame() && ContinuationHelper::CompiledFrame::is_owning_locks(map.thread(), &map, f))) { (f.is_compiled_frame() && ContinuationHelper::CompiledFrame::is_owning_locks(map.thread(), &map, f))) {
// return true; return true;
// } }
// } }
// return false; return false;
// } }
bool FreezeBase::interpreted_native_or_deoptimized_on_stack() { bool FreezeBase::interpreted_native_or_deoptimized_on_stack() {
ContinuationEntry* ce = _thread->last_continuation(); ContinuationEntry* ce = _thread->last_continuation();
@ -1575,8 +1575,8 @@ static inline int freeze_internal(JavaThread* current, intptr_t* const sp) {
assert(entry->is_virtual_thread() == (entry->scope(current) == java_lang_VirtualThread::vthread_scope()), ""); assert(entry->is_virtual_thread() == (entry->scope(current) == java_lang_VirtualThread::vthread_scope()), "");
// assert(monitors_on_stack(current) == ((current->held_monitor_count() - current->jni_monitor_count()) > 0), assert(monitors_on_stack(current) == ((current->held_monitor_count() - current->jni_monitor_count()) > 0),
// "Held monitor count and locks on stack invariant: " INT64_FORMAT " JNI: " INT64_FORMAT, (int64_t)current->held_monitor_count(), (int64_t)current->jni_monitor_count()); "Held monitor count and locks on stack invariant: " INT64_FORMAT " JNI: " INT64_FORMAT, (int64_t)current->held_monitor_count(), (int64_t)current->jni_monitor_count());
if (entry->is_pinned() || current->held_monitor_count() > 0) { if (entry->is_pinned() || current->held_monitor_count() > 0) {
log_develop_debug(continuations)("PINNED due to critical section/hold monitor"); log_develop_debug(continuations)("PINNED due to critical section/hold monitor");