Skip to content

Commit 742c776

Browse files
author
Vladimir Kozlov
committedFeb 29, 2024
8322743: C2: prevent lock region elimination in OSR compilation
Reviewed-by: epeter, dlong, vlivanov
1 parent d29cefb commit 742c776

File tree

3 files changed

+96
-3
lines changed

3 files changed

+96
-3
lines changed
 

‎src/hotspot/share/opto/compile.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -4879,6 +4879,7 @@ void Compile::add_coarsened_locks(GrowableArray<AbstractLockNode*>& locks) {
48794879
// Locking regions (BoxLock) could be Unbalanced here:
48804880
// - its coarsened locks were eliminated in earlier
48814881
// macro nodes elimination followed by loop unroll
4882+
// - it is OSR locking region (no Lock node)
48824883
// Preserve Unbalanced status in such cases.
48834884
if (!this_box->is_unbalanced()) {
48844885
this_box->set_coarsened();

‎src/hotspot/share/opto/parse1.cpp

+25-3
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,20 @@ void Parse::load_interpreter_state(Node* osr_buf) {
224224
Node *monitors_addr = basic_plus_adr(osr_buf, osr_buf, (max_locals+mcnt*2-1)*wordSize);
225225
for (index = 0; index < mcnt; index++) {
226226
// Make a BoxLockNode for the monitor.
227-
Node *box = new BoxLockNode(next_monitor());
227+
BoxLockNode* osr_box = new BoxLockNode(next_monitor());
228228
// Check for bailout after new BoxLockNode
229229
if (failing()) { return; }
230-
box = _gvn.transform(box);
231230

231+
// This OSR locking region is unbalanced because it does not have Lock node:
232+
// locking was done in Interpreter.
233+
// This is similar to Coarsened case when Lock node is eliminated
234+
// and as result the region is marked as Unbalanced.
235+
236+
// Emulate Coarsened state transition from Regular to Unbalanced.
237+
osr_box->set_coarsened();
238+
osr_box->set_unbalanced();
239+
240+
Node* box = _gvn.transform(osr_box);
232241

233242
// Displaced headers and locked objects are interleaved in the
234243
// temp OSR buffer. We only copy the locked objects out here.
@@ -1805,11 +1814,24 @@ void Parse::merge_common(Parse::Block* target, int pnum) {
18051814
const JVMState* jvms = map()->jvms();
18061815
if (EliminateNestedLocks &&
18071816
jvms->is_mon(j) && jvms->is_monitor_box(j)) {
1808-
// BoxLock nodes are not commoning.
1817+
// BoxLock nodes are not commoning when EliminateNestedLocks is on.
18091818
// Use old BoxLock node as merged box.
18101819
assert(newin->jvms()->is_monitor_box(j), "sanity");
18111820
// This assert also tests that nodes are BoxLock.
18121821
assert(BoxLockNode::same_slot(n, m), "sanity");
1822+
BoxLockNode* old_box = m->as_BoxLock();
1823+
if (n->as_BoxLock()->is_unbalanced() && !old_box->is_unbalanced()) {
1824+
// Preserve Unbalanced status.
1825+
//
1826+
// `old_box` can have only Regular or Coarsened status
1827+
// because this code is executed only during Parse phase and
1828+
// Incremental Inlining before EA and Macro nodes elimination.
1829+
//
1830+
// Incremental Inlining is executed after IGVN optimizations
1831+
// during which BoxLock can be marked as Coarsened.
1832+
old_box->set_coarsened(); // Verifies state
1833+
old_box->set_unbalanced();
1834+
}
18131835
C->gvn_replace_by(n, m);
18141836
} else if (!check_elide_phi || !target->can_elide_SEL_phi(j)) {
18151837
phi = ensure_phi(j, nophi);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8322743
27+
* @summary EA incorrectly marks locks for elimination for escaped object which comes from Interpreter in OSR compilation.
28+
* @run main/othervm -XX:-TieredCompilation -Xcomp -XX:CompileCommand=compileonly,TestLocksInOSR*::* -XX:CompileCommand=quiet TestLocksInOSR
29+
* @run main TestLocksInOSR
30+
*/
31+
32+
public class TestLocksInOSR {
33+
34+
public static void main(String[] args) throws Exception {
35+
// Triggers assert(this->held_monitor_count() == this->jni_monitor_count()) failed: held monitor count should be equal to jni: 1 != 0
36+
test1();
37+
38+
// Triggers assert(current->held_monitor_count() == 0) failed: Should not be possible
39+
test2();
40+
}
41+
42+
static void test1() throws Exception {
43+
Thread writeThread = new Thread(new Runnable() {
44+
@Override
45+
public void run() {
46+
for (int i = 0; i < 2; ++i) {
47+
synchronized (new Object()) {
48+
// Trigger OSR compilation
49+
for (int j = 0; j < 100_000; ++j) {
50+
// We still have safepoint left in code
51+
}
52+
}
53+
}
54+
}
55+
});
56+
writeThread.start();
57+
writeThread.join();
58+
}
59+
60+
static void test2() {
61+
for (int i = 0; i < 2; ++i) {
62+
synchronized (new Object()) {
63+
// Trigger OSR compilation
64+
for (int j = 0; j < 100_000; ++j) {
65+
// We still have safepoint left in code
66+
}
67+
}
68+
}
69+
}
70+
}

0 commit comments

Comments
 (0)
Please sign in to comment.