Skip to content

Commit 56cebab

Browse files
mur47x111mur47x111
committed
[GR-63471] Fix various bugs on lock virtualization
PullRequest: graal/20394
2 parents 7412dd7 + 2caae23 commit 56cebab

File tree

10 files changed

+417
-83
lines changed

10 files changed

+417
-83
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/MonitorPEATest.java‎

Lines changed: 282 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -32,13 +32,8 @@
3232
import org.junit.Before;
3333
import org.junit.Test;
3434

35-
import jdk.graal.compiler.graph.Node;
36-
import jdk.graal.compiler.graph.NodeFlood;
35+
import jdk.graal.compiler.api.directives.GraalDirectives;
3736
import jdk.graal.compiler.hotspot.replacements.HotSpotReplacementsUtil;
38-
import jdk.graal.compiler.nodes.AbstractEndNode;
39-
import jdk.graal.compiler.nodes.StructuredGraph;
40-
import jdk.graal.compiler.nodes.java.MonitorEnterNode;
41-
import jdk.graal.compiler.nodes.java.MonitorExitNode;
4237

4338
/**
4439
* Tests that PEA preserves the monitorenter order. This is essential for lightweight locking.
@@ -163,33 +158,292 @@ public void testSnippet4() {
163158
test("snippet4", new Object(), true, true);
164159
}
165160

166-
@Override
167-
protected void checkMidTierGraph(StructuredGraph graph) {
168-
// The following lock depth check only works with simple control flow.
169-
NodeFlood flood = graph.createNodeFlood();
170-
flood.add(graph.start());
161+
public static class B {
162+
Float f = 1.0f;
163+
}
164+
165+
public static void snippet5() {
166+
synchronized (new B()) {
167+
synchronized (new StringBuilder()) {
168+
synchronized (B.class) {
169+
}
170+
}
171+
}
172+
}
173+
174+
@Test
175+
public void testSnippet5() {
176+
test("snippet5");
177+
}
178+
179+
static class C {
180+
B b;
171181

172-
int lockDepth = -1;
182+
C(B b) {
183+
this.b = b;
184+
}
185+
}
173186

174-
for (Node current : flood) {
175-
if (current instanceof AbstractEndNode) {
176-
AbstractEndNode end = (AbstractEndNode) current;
177-
flood.add(end.merge());
178-
} else {
179-
if (current instanceof MonitorEnterNode enter) {
180-
int depth = enter.getMonitorId().getLockDepth();
181-
assertTrue(lockDepth < depth);
182-
lockDepth = depth;
183-
} else if (current instanceof MonitorExitNode exit) {
184-
int depth = exit.getMonitorId().getLockDepth();
185-
assertTrue(lockDepth >= depth);
186-
lockDepth = depth;
187+
public static void snippet6() {
188+
B b = new B();
189+
C c = new C(b);
190+
synchronized (c) {
191+
synchronized (b) {
192+
synchronized (B.class) {
187193
}
194+
}
195+
}
196+
}
197+
198+
@Test
199+
public void testSnippet6() {
200+
test("snippet6");
201+
}
202+
203+
public static void snippet7() {
204+
B b = new B();
205+
C c = new C(b);
206+
synchronized (c) {
207+
synchronized (c) {
208+
synchronized (b) {
209+
synchronized (b) {
210+
synchronized (B.class) {
211+
}
212+
}
213+
}
214+
}
215+
}
216+
}
217+
218+
@Test
219+
public void testSnippet7() {
220+
test("snippet7");
221+
}
222+
223+
public static void snippet8() {
224+
B b = new B();
225+
C c = new C(b);
226+
synchronized (c) {
227+
synchronized (b) {
228+
synchronized (c) {
229+
synchronized (b) {
230+
synchronized (B.class) {
231+
}
232+
}
233+
}
234+
}
235+
}
236+
}
237+
238+
@Test
239+
public void testSnippet8() {
240+
test("snippet8");
241+
}
188242

189-
for (Node successor : current.successors()) {
190-
flood.add(successor);
243+
public static void snippet9() {
244+
B b = new B();
245+
C c = new C(b);
246+
synchronized (c) {
247+
synchronized (b) {
248+
synchronized (b) {
249+
synchronized (c) {
250+
synchronized (B.class) {
251+
}
252+
}
191253
}
192254
}
193255
}
194256
}
257+
258+
@Test
259+
public void testSnippet9() {
260+
test("snippet9");
261+
}
262+
263+
@BytecodeParserNeverInline
264+
public static void callee0() {
265+
synchronized (B.class) {
266+
}
267+
}
268+
269+
@BytecodeParserNeverInline
270+
public static void callee1() {
271+
staticObj = new B();
272+
callee0();
273+
}
274+
275+
@BytecodeParserNeverInline
276+
public static void callee() {
277+
callee1();
278+
}
279+
280+
public static void snippet10() {
281+
B b = new B();
282+
synchronized (b) {
283+
callee();
284+
}
285+
}
286+
287+
@Test
288+
public void testSnippet10() {
289+
test("snippet10");
290+
}
291+
292+
static RuntimeException cachedException = new RuntimeException();
293+
294+
public static void snippet11(boolean flag) {
295+
B b = new B();
296+
synchronized (b) {
297+
if (flag) {
298+
staticObj = b;
299+
throw cachedException;
300+
}
301+
}
302+
}
303+
304+
@Test
305+
public void testSnippet11() {
306+
test("snippet11", true);
307+
}
308+
309+
public static void snippet12() {
310+
B b = new B();
311+
synchronized (b) {
312+
callee();
313+
staticObj = b;
314+
}
315+
}
316+
317+
@Test
318+
public void testSnippet12() {
319+
test("snippet12");
320+
}
321+
322+
public static void snippet13(boolean flag, boolean escape) {
323+
B b = new B();
324+
325+
synchronized (b) {
326+
synchronized (b) {
327+
if (GraalDirectives.injectBranchProbability(0.9, flag)) {
328+
GraalDirectives.controlFlowAnchor();
329+
if (GraalDirectives.injectBranchProbability(0.1, escape)) {
330+
staticObj = b;
331+
}
332+
} else {
333+
staticObj1 = b;
334+
throw cachedException;
335+
}
336+
}
337+
}
338+
}
339+
340+
@Test
341+
public void testSnippet13() {
342+
test("snippet13", true, true);
343+
}
344+
345+
public static void snippet15(boolean deoptimize) {
346+
synchronized (new B()) {
347+
synchronized (new StringBuilder()) {
348+
synchronized (B.class) {
349+
if (deoptimize) {
350+
GraalDirectives.deoptimize();
351+
}
352+
}
353+
}
354+
}
355+
}
356+
357+
@Test
358+
public void testSnippet15() {
359+
test("snippet15", true);
360+
}
361+
362+
public static void snippet16(boolean deoptimize) {
363+
B b = new B();
364+
C c = new C(b);
365+
synchronized (c) {
366+
synchronized (b) {
367+
synchronized (B.class) {
368+
if (deoptimize) {
369+
GraalDirectives.deoptimize();
370+
}
371+
}
372+
}
373+
}
374+
}
375+
376+
@Test
377+
public void testSnippet16() {
378+
test("snippet16", true);
379+
}
380+
381+
public static void snippet17(boolean deoptimize) {
382+
B b = new B();
383+
C c = new C(b);
384+
synchronized (c) {
385+
synchronized (c) {
386+
synchronized (b) {
387+
synchronized (b) {
388+
synchronized (B.class) {
389+
if (deoptimize) {
390+
GraalDirectives.deoptimize();
391+
}
392+
}
393+
}
394+
}
395+
}
396+
}
397+
}
398+
399+
@Test
400+
public void testSnippet17() {
401+
test("snippet17", true);
402+
}
403+
404+
public static void snippet18(boolean deoptimize) {
405+
B b = new B();
406+
C c = new C(b);
407+
synchronized (c) {
408+
synchronized (b) {
409+
synchronized (c) {
410+
synchronized (b) {
411+
synchronized (B.class) {
412+
if (deoptimize) {
413+
GraalDirectives.deoptimize();
414+
}
415+
}
416+
}
417+
}
418+
}
419+
}
420+
}
421+
422+
@Test
423+
public void testSnippet18() {
424+
test("snippet18", true);
425+
}
426+
427+
public static void snippet19(boolean deoptimize) {
428+
B b = new B();
429+
C c = new C(b);
430+
synchronized (c) {
431+
synchronized (b) {
432+
synchronized (b) {
433+
synchronized (c) {
434+
synchronized (B.class) {
435+
if (deoptimize) {
436+
GraalDirectives.deoptimize();
437+
}
438+
}
439+
}
440+
}
441+
}
442+
}
443+
}
444+
445+
@Test
446+
public void testSnippet19() {
447+
test("snippet19", true);
448+
}
195449
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/HotSpotDebugInfoBuilder.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import jdk.graal.compiler.nodes.extended.ForeignCall;
4646
import jdk.graal.compiler.nodes.spi.NodeValueMap;
4747
import jdk.graal.compiler.nodes.spi.NodeWithState;
48-
4948
import jdk.vm.ci.code.BytecodeFrame;
5049
import jdk.vm.ci.code.StackLockValue;
5150
import jdk.vm.ci.code.VirtualObject;
@@ -102,7 +101,7 @@ protected void verifyFrameState(NodeWithState node, FrameState topState) {
102101

103102
FrameState current = topState;
104103
while (current != null) {
105-
assert current.getMethod() instanceof HotSpotResolvedJavaMethod : current;
104+
GraalError.guarantee(current.getMethod() instanceof HotSpotResolvedJavaMethod, "unsupported method type %s", current.getMethod());
106105
current = current.outerFrameState();
107106
}
108107

@@ -112,7 +111,7 @@ protected void verifyFrameState(NodeWithState node, FrameState topState) {
112111
if (DefaultHotSpotLoweringProvider.RuntimeCalls.runtimeCalls.containsValue(descriptor.getSignature())) {
113112
// This case is special because we can't use rethrowException. This path must be
114113
// marked as reexecutable so it doesn't fail some internal asserts even though the
115-
// actual deopt is done with Unpack_exception which overrides the reexectuable flag
114+
// actual deopt is done with Unpack_exception which overrides the reexecutable flag
116115
GraalError.guarantee(topState.getStackState() == FrameState.StackState.BeforePop && topState.bci >= 0 && topState.stackSize() == 0, "invalid state %s for bytecode exception %s",
117116
topState, descriptor.getSignature());
118117
GraalError.guarantee(!topState.getStackState().duringCall, "must be not duringCall to set reexecute bit");

0 commit comments

Comments
 (0)