Skip to content

Commit c712894

Browse files
gergo-gergo-
committed
[GR-61877] CaptureStateBegin: adjust assertion, and CTW phase plan fuzzing fixes
PullRequest: graal/19976
2 parents 87f88bb + a799e24 commit c712894

File tree

5 files changed

+59
-24
lines changed

5 files changed

+59
-24
lines changed

‎compiler/ci/ci_common/gate.jsonnet

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@
283283
"weekly-compiler-ctw_phaseplan_fuzzing-labsjdk-latest-linux-amd64": {
284284
notify_groups: [],
285285
notify_emails: ["gergo.barany@oracle.com"],
286+
logs+: ["*/graal_dumps/*/*_failure.log"],
286287
},
287288

288289
"weekly-compiler-test_vec16-labsjdk-latest-linux-amd64": {},

‎compiler/mx.compiler/mx_compiler.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -249,13 +249,16 @@ def _is_jvmci_enabled(vmargs):
249249

250250
def _ctw_jvmci_export_args(arg_prefix='--'):
251251
"""
252-
Gets the VM args needed to export JVMCI API required by CTW.
252+
Gets the VM args needed to export JVMCI API and HotSpot internals required by CTW.
253253
"""
254-
args = ['add-exports=java.base/jdk.internal.module=ALL-UNNAMED',
255-
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.hotspot=ALL-UNNAMED',
256-
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED',
257-
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.services=ALL-UNNAMED',
258-
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.runtime=ALL-UNNAMED']
254+
args = [
255+
'add-exports=java.base/jdk.internal.module=ALL-UNNAMED',
256+
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.hotspot=ALL-UNNAMED',
257+
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED',
258+
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.services=ALL-UNNAMED',
259+
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.runtime=ALL-UNNAMED',
260+
'add-exports=jdk.graal.compiler/jdk.graal.compiler.hotspot=ALL-UNNAMED',
261+
]
259262
return [arg_prefix + arg for arg in args]
260263

261264
def _ctw_system_properties_suffix():
@@ -518,7 +521,10 @@ def compiler_gate_runner(suites, unit_test_runs, bootstrap_tests, tasks, extraVM
518521
'-DCompileTheWorld.MultiThreaded=true', '-Djdk.graal.InlineDuringParsing=false', '-Djdk.graal.TrackNodeSourcePosition=true',
519522
'-DCompileTheWorld.Verbose=false', '-XX:ReservedCodeCacheSize=300m',
520523
]
521-
ctw_phaseplan_fuzzing_flags = ['-DCompileTheWorld.FuzzPhasePlan=true', '-Djdk.graal.PrintGraphStateDiff=true']
524+
ctw_phaseplan_fuzzing_flags = [
525+
'-DCompileTheWorld.FuzzPhasePlan=true',
526+
'-Djdk.graal.PrintGraphStateDiff=true',
527+
]
522528
with Task('CTW:hosted', tasks, tags=GraalTags.ctw, report=True) as t:
523529
if t:
524530
ctw(ctw_flags, _remove_empty_entries(extraVMarguments))

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2013, 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
@@ -88,7 +88,6 @@
8888
import jdk.graal.compiler.debug.MetricKey;
8989
import jdk.graal.compiler.debug.TTY;
9090
import jdk.graal.compiler.hotspot.CompilationTask;
91-
import jdk.graal.compiler.hotspot.CompileTheWorldFuzzedSuitesCompilationTask;
9291
import jdk.graal.compiler.hotspot.GraalHotSpotVMConfig;
9392
import jdk.graal.compiler.hotspot.HotSpotBytecodeParser;
9493
import jdk.graal.compiler.hotspot.HotSpotGraalCompiler;
@@ -940,14 +939,20 @@ private void printSuspiciousThreads() {
940939
}
941940
}
942941

942+
/**
943+
* Determine if the message describes a failure we should ignore in the context of CTW. Since in
944+
* CTW we try to compile all methods as compilation roots indiscriminately, we may hit upon a
945+
* helper method that expects to only be inlined into a snippet. Such compilations may fail with
946+
* a well-known message complaining about node intrinsics used outside a snippet. We can ignore
947+
* these failures.
948+
*/
949+
public static boolean shouldIgnoreFailure(String failureMessage) {
950+
return failureMessage.startsWith(HotSpotBytecodeParser.BAD_NODE_INTRINSIC_PLUGIN_CONTEXT);
951+
}
952+
943953
@Override
944954
protected void handleFailure(HotSpotCompilationRequestResult result) {
945-
/*
946-
* Ignore bailouts caused by calling node intrinsics outside a Snippet. Since in CTW we try
947-
* to compile all methods as compilation roots indiscriminately, we may hit upon a helper
948-
* method that expects to only be inlined into a snippet.
949-
*/
950-
if (!result.getFailureMessage().startsWith(HotSpotBytecodeParser.BAD_NODE_INTRINSIC_PLUGIN_CONTEXT)) {
955+
if (!shouldIgnoreFailure(result.getFailureMessage())) {
951956

952957
if (Options.FuzzPhasePlan.getValue(harnessOptions)) {
953958
HotSpotJVMCIRuntime jvmciRuntime = HotSpotJVMCIRuntime.runtime();

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/CompileTheWorldFuzzedSuitesCompilationTask.java renamed to ‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/test/CompileTheWorldFuzzedSuitesCompilationTask.java

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2022, 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
@@ -22,7 +22,7 @@
2222
* or visit www.oracle.com if you need additional information or have any
2323
* questions.
2424
*/
25-
package jdk.graal.compiler.hotspot;
25+
package jdk.graal.compiler.hotspot.test;
2626

2727
import java.io.ByteArrayOutputStream;
2828
import java.io.IOException;
@@ -31,9 +31,13 @@
3131
import java.nio.file.Paths;
3232

3333
import jdk.graal.compiler.core.common.CancellationBailoutException;
34+
import jdk.graal.compiler.core.common.PermanentBailoutException;
3435
import jdk.graal.compiler.core.phases.fuzzing.FuzzedSuites;
3536
import jdk.graal.compiler.debug.DebugContext;
3637
import jdk.graal.compiler.debug.GraalError;
38+
import jdk.graal.compiler.debug.TTY;
39+
import jdk.graal.compiler.hotspot.CompilationTask;
40+
import jdk.graal.compiler.hotspot.HotSpotGraalCompiler;
3741
import jdk.graal.compiler.hotspot.meta.HotSpotFuzzedSuitesProvider;
3842
import jdk.vm.ci.hotspot.HotSpotCompilationRequest;
3943
import jdk.vm.ci.hotspot.HotSpotCompilationRequestResult;
@@ -53,15 +57,32 @@ public CompileTheWorldFuzzedSuitesCompilationTask(HotSpotJVMCIRuntime jvmciRunti
5357

5458
private final class HotSpotFuzzedCompilationWrapper extends HotSpotCompilationWrapper {
5559

60+
private static boolean shouldIgnoreFailure(Throwable cause) {
61+
if (cause instanceof CancellationBailoutException) {
62+
/* Cancelled compilations are not failed compilations. */
63+
return true;
64+
}
65+
if (cause instanceof PermanentBailoutException bailout) {
66+
/* Some bailouts related to node intrinsics can be ignored in the context of CTW. */
67+
return CompileTheWorld.shouldIgnoreFailure(bailout.getMessage());
68+
}
69+
return false;
70+
}
71+
5672
@Override
5773
protected HotSpotCompilationRequestResult handleFailure(DebugContext initialDebug, Throwable cause) {
58-
if (!(cause instanceof CancellationBailoutException)) {
59-
String dumpPath = initialDebug.getDumpPath("", false);
74+
if (!shouldIgnoreFailure(cause)) {
75+
76+
String dumpPath = initialDebug.getDumpPath("", false, false);
6077

6178
HotSpotFuzzedSuitesProvider suitesProvider = (HotSpotFuzzedSuitesProvider) compiler.getGraalRuntime().getHostBackend().getProviders().getSuites();
6279
FuzzedSuites phasePlan = suitesProvider.getLastSuitesForThread(initialDebug.getOptions());
6380
phasePlan.saveFuzzedSuites(dumpPath);
6481
try {
82+
final String logFileName = dumpPath + "_failure.log";
83+
TTY.println("CompileTheWorld : %s error compiling method: %s", cause.getClass(), this.toString());
84+
TTY.println("CompileTheWorld : Retry from seed with: -D%s=%s", HotSpotFuzzedSuitesProvider.SEED_SYSTEM_PROPERTY, suitesProvider.getLastSeed().get());
85+
TTY.println("CompileTheWorld : See the file %s for details.", logFileName);
6586
ByteArrayOutputStream baos = new ByteArrayOutputStream();
6687
try (PrintStream ps = new PrintStream(baos)) {
6788
ps.printf("CompileTheWorld : %s error compiling method: %s%n%n", cause.getClass(), this.toString());
@@ -75,7 +96,7 @@ protected HotSpotCompilationRequestResult handleFailure(DebugContext initialDebu
7596
ps.printf("The stack trace:%n");
7697
cause.printStackTrace(ps);
7798
}
78-
Files.write(Paths.get(dumpPath + "_failure.log"), baos.toByteArray());
99+
Files.write(Paths.get(logFileName), baos.toByteArray());
79100
} catch (IOException e) {
80101
GraalError.shouldNotReachHere(e, "Error saving log of failed phase plan to " + dumpPath + "_failure.log"); // ExcludeFromJacocoGeneratedReport
81102
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/CaptureStateBeginNode.java

Lines changed: 6 additions & 4 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
@@ -37,6 +37,7 @@
3737
import jdk.graal.compiler.nodes.BeginStateSplitNode;
3838
import jdk.graal.compiler.nodes.LoopExitNode;
3939
import jdk.graal.compiler.nodes.MemoryProxyNode;
40+
import jdk.graal.compiler.nodes.SafepointNode;
4041
import jdk.graal.compiler.nodes.ValueProxyNode;
4142
import jdk.graal.compiler.nodes.spi.Canonicalizable;
4243
import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
@@ -70,10 +71,11 @@ public Node canonical(CanonicalizerTool tool) {
7071
public boolean verifyNode() {
7172
if (predecessor() instanceof LoopExitNode loopExit) {
7273
/*
73-
* Must guarantee only value and memory proxies are attached to the loop exit. Anything
74-
* else should be attached to this node
74+
* Must guarantee that only value and memory proxies or safepoints are attached to the
75+
* loop exit. Anything else should be attached to this node.
7576
*/
76-
assert loopExit.usages().stream().allMatch(NodePredicates.isA(ValueProxyNode.class).or(MemoryProxyNode.class)) : String.format("LoopExit has disallowed usages %s", loopExit);
77+
assert loopExit.usages().stream().allMatch(NodePredicates.isA(ValueProxyNode.class).or(MemoryProxyNode.class).or(SafepointNode.class)) : String.format(
78+
"LoopExit %s has disallowed usages %s", loopExit, loopExit.usages().snapshot());
7779
}
7880

7981
return super.verifyNode();

0 commit comments

Comments
 (0)