Skip to content

Commit

Permalink
8322743: C2: prevent lock region elimination in OSR compilation
Browse files Browse the repository at this point in the history
Backport-of: 742c776a922bc226a3beaa9e219ff0bd2baf7bc4
  • Loading branch information
TheRealMDoerr committed Jun 18, 2024
1 parent a5eb7f2 commit 4af7862
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4819,6 +4819,7 @@ void Compile::add_coarsened_locks(GrowableArray<AbstractLockNode*>& locks) {
// Locking regions (BoxLock) could be Unbalanced here:
// - its coarsened locks were eliminated in earlier
// macro nodes elimination followed by loop unroll
// - it is OSR locking region (no Lock node)
// Preserve Unbalanced status in such cases.
if (!this_box->is_unbalanced()) {
this_box->set_coarsened();
Expand Down
28 changes: 25 additions & 3 deletions src/hotspot/share/opto/parse1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,20 @@ void Parse::load_interpreter_state(Node* osr_buf) {
Node *monitors_addr = basic_plus_adr(osr_buf, osr_buf, (max_locals+mcnt*2-1)*wordSize);
for (index = 0; index < mcnt; index++) {
// Make a BoxLockNode for the monitor.
Node *box = new BoxLockNode(next_monitor());
BoxLockNode* osr_box = new BoxLockNode(next_monitor());
// Check for bailout after new BoxLockNode
if (failing()) { return; }
box = _gvn.transform(box);

// This OSR locking region is unbalanced because it does not have Lock node:
// locking was done in Interpreter.
// This is similar to Coarsened case when Lock node is eliminated
// and as result the region is marked as Unbalanced.

// Emulate Coarsened state transition from Regular to Unbalanced.
osr_box->set_coarsened();
osr_box->set_unbalanced();

Node* box = _gvn.transform(osr_box);

// Displaced headers and locked objects are interleaved in the
// temp OSR buffer. We only copy the locked objects out here.
Expand Down Expand Up @@ -1790,11 +1799,24 @@ void Parse::merge_common(Parse::Block* target, int pnum) {
const JVMState* jvms = map()->jvms();
if (EliminateNestedLocks &&
jvms->is_mon(j) && jvms->is_monitor_box(j)) {
// BoxLock nodes are not commoning.
// BoxLock nodes are not commoning when EliminateNestedLocks is on.
// Use old BoxLock node as merged box.
assert(newin->jvms()->is_monitor_box(j), "sanity");
// This assert also tests that nodes are BoxLock.
assert(BoxLockNode::same_slot(n, m), "sanity");
BoxLockNode* old_box = m->as_BoxLock();
if (n->as_BoxLock()->is_unbalanced() && !old_box->is_unbalanced()) {
// Preserve Unbalanced status.
//
// `old_box` can have only Regular or Coarsened status
// because this code is executed only during Parse phase and
// Incremental Inlining before EA and Macro nodes elimination.
//
// Incremental Inlining is executed after IGVN optimizations
// during which BoxLock can be marked as Coarsened.
old_box->set_coarsened(); // Verifies state
old_box->set_unbalanced();
}
C->gvn_replace_by(n, m);
} else if (!check_elide_phi || !target->can_elide_SEL_phi(j)) {
phi = ensure_phi(j, nophi);
Expand Down
70 changes: 70 additions & 0 deletions test/hotspot/jtreg/compiler/locks/TestLocksInOSR.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8322743
* @summary EA incorrectly marks locks for elimination for escaped object which comes from Interpreter in OSR compilation.
* @run main/othervm -XX:-TieredCompilation -Xcomp -XX:CompileCommand=compileonly,TestLocksInOSR*::* -XX:CompileCommand=quiet TestLocksInOSR
* @run main TestLocksInOSR
*/

public class TestLocksInOSR {

public static void main(String[] args) throws Exception {
// Triggers assert(this->held_monitor_count() == this->jni_monitor_count()) failed: held monitor count should be equal to jni: 1 != 0
test1();

// Triggers assert(current->held_monitor_count() == 0) failed: Should not be possible
test2();
}

static void test1() throws Exception {
Thread writeThread = new Thread(new Runnable() {
@Override
public void run() {
for (int i = 0; i < 2; ++i) {
synchronized (new Object()) {
// Trigger OSR compilation
for (int j = 0; j < 100_000; ++j) {
// We still have safepoint left in code
}
}
}
}
});
writeThread.start();
writeThread.join();
}

static void test2() {
for (int i = 0; i < 2; ++i) {
synchronized (new Object()) {
// Trigger OSR compilation
for (int j = 0; j < 100_000; ++j) {
// We still have safepoint left in code
}
}
}
}
}

1 comment on commit 4af7862

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.