Skip to content

Commit 918380b

Browse files
jchaloujchalou
committed
[GR-69167] Minor cleanups in Truffle code
PullRequest: graal/22005
2 parents c502eb2 + e8c3dd4 commit 918380b

File tree

21 files changed

+140
-145
lines changed

21 files changed

+140
-145
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/VerifyDebugUsage.java‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ protected void verify(StructuredGraph graph, CoreProviders context) {
137137
MethodSource.of("com.oracle.graal.pointsto.phases.InlineBeforeAnalysis", "decodeGraph"),
138138
MethodSource.of("com.oracle.svm.hosted.classinitialization.SimulateClassInitializerSupport", "decodeGraph"),
139139
MethodSource.of("com.oracle.svm.hosted.classinitialization.SimulateClassInitializerAbortException", "doAbort"),
140-
MethodSource.of(CallTree.class, "dumpBasic"),
141-
MethodSource.of(CallTree.class, "GraphManager", "peRoot"));
140+
MethodSource.of(CallTree.class, "dumpBasic"));
142141

143142
/**
144143
* The set of methods allowed to call a {@code Debug.dump(...)} method with the {@code level}
@@ -156,7 +155,17 @@ protected void verify(StructuredGraph graph, CoreProviders context) {
156155
MethodSource.of(SnippetTemplate.class, "instantiate"),
157156
MethodSource.of(SnippetTemplate.class, "<init>"),
158157
MethodSource.of(SymbolicSnippetEncoder.class, "verifySnippetEncodeDecode"),
159-
MethodSource.of(CallTree.class, "dumpInfo"),
158+
MethodSource.of(CallTree.class, "dumpInfo"));
159+
160+
/**
161+
* The set of methods allowed to call a {@code Debug.dump(...)} method with a variable
162+
* {@code level} parameter and the {@code object} parameter bound to a {@link StructuredGraph}
163+
* value.
164+
*
165+
* If you add a *justified* graph dump with variable level parameter, then update the allow
166+
* list.
167+
*/
168+
private static final Set<MethodSource> ParameterizedLevelStructuredGraphDumpAllowList = CollectionsUtil.setOf(
160169
MethodSource.of(CallTree.class, "GraphManager", "pe"));
161170

162171
@Override
@@ -175,7 +184,10 @@ protected void verifyParameters(MetaAccessProvider metaAccess1, MethodCallTarget
175184
allowedClasses.add(ReportHotCodePhase.class.getName());
176185
String callerClassName = debugCallTarget.graph().method().format("%H");
177186
if (!allowedClasses.contains(callerClassName)) {
178-
int dumpLevel = verifyDumpLevelParameter(debugCallTarget, verifiedCallee, args.get(1));
187+
ResolvedJavaMethod callerMethod = debugCallTarget.graph().method();
188+
Integer dumpLevel = ParameterizedLevelStructuredGraphDumpAllowList.stream().noneMatch(ms -> ms.matches(callerMethod))
189+
? verifyDumpLevelParameter(debugCallTarget, verifiedCallee, args.get(1))
190+
: null;
179191
verifyDumpObjectParameter(debugCallTarget, args.get(2), verifiedCallee, dumpLevel);
180192
}
181193
}
@@ -220,7 +232,13 @@ protected void verifyDumpObjectParameter(MethodCallTargetNode debugCallTarget, V
220232
protected void verifyStructuredGraphDumping(MethodCallTargetNode debugCallTarget, ResolvedJavaMethod verifiedCallee, Integer dumpLevel)
221233
throws VerifyPhase.VerificationError {
222234
ResolvedJavaMethod method = debugCallTarget.graph().method();
223-
if (dumpLevel == DebugContext.BASIC_LEVEL) {
235+
if (dumpLevel == null) {
236+
if (ParameterizedLevelStructuredGraphDumpAllowList.stream().noneMatch(ms -> ms.matches(method))) {
237+
throw new VerificationError(
238+
debugCallTarget, "call to %s with parameterized level not in %s.ParameterizedLevelStructuredGraphDumpAllowList.%n", verifiedCallee.format("%H.%n(%p)"),
239+
getClass().getName());
240+
}
241+
} else if (dumpLevel == DebugContext.BASIC_LEVEL) {
224242
if (BasicLevelStructuredGraphDumpAllowList.stream().noneMatch(ms -> ms.matches(method))) {
225243
throw new VerificationError(
226244
debugCallTarget, "call to %s with level == DebugContext.BASIC_LEVEL not in %s.BasicLevelStructuredGraphDumpAllowList.%n", verifiedCallee.format("%H.%n(%p)"),

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/replacements/test/PEGraphDecoderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
import jdk.graal.compiler.nodes.spi.CoreProviders;
6161
import jdk.graal.compiler.options.OptionValues;
6262
import jdk.graal.compiler.phases.OptimisticOptimizations;
63-
import jdk.graal.compiler.replacements.CachingPEGraphDecoder;
63+
import jdk.graal.compiler.truffle.CachingPEGraphDecoder;
6464
import jdk.graal.compiler.util.CollectionsUtil;
6565
import jdk.vm.ci.meta.JavaKind;
6666
import jdk.vm.ci.meta.ResolvedJavaMethod;

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/truffle/test/AgnosticInliningPhaseTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public boolean hasNextTier() {
8585
return false;
8686
}
8787
}, null);
88-
final AgnosticInliningPhase agnosticInliningPhase = new AgnosticInliningPhase(partialEvaluator,
88+
final AgnosticInliningPhase agnosticInliningPhase = new AgnosticInliningPhase(
8989
new PostPartialEvaluationSuite(compiler.getOrCreateCompilerOptions(callTarget), false));
9090
agnosticInliningPhase.apply(context.graph, context);
9191
return context.graph;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/BytecodeParser.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ public boolean canMergeIntrinsicReturns() {
537537
* A scoped object for tasks to be performed after inlining during parsing such as processing
538538
* {@linkplain BytecodeFrame#isPlaceholderBci(int) placeholder} frames states.
539539
*/
540-
static class InliningScope implements AutoCloseable {
540+
protected static class InliningScope implements AutoCloseable {
541541
final ResolvedJavaMethod callee;
542542
FrameState stateBefore;
543543
final Mark mark;
@@ -971,7 +971,7 @@ protected BytecodeParser(GraphBuilderPhase.Instance graphBuilderInstance, Struct
971971
int entryBCI, IntrinsicContext intrinsicContext) {
972972
super(graphBuilderInstance.providers);
973973
invocationPluginReceiver = new InvocationPluginReceiver(this);
974-
this.bytecodeProvider = intrinsicContext == null ? new ResolvedJavaMethodBytecodeProvider() : intrinsicContext.getBytecodeProvider();
974+
this.bytecodeProvider = intrinsicContext == null ? ResolvedJavaMethodBytecodeProvider.INSTANCE : intrinsicContext.getBytecodeProvider();
975975
this.code = bytecodeProvider.getBytecode(method);
976976
this.method = code.getMethod();
977977
this.graphBuilderInstance = graphBuilderInstance;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,9 +2156,9 @@ protected Object readObject(MethodScope methodScope) {
21562156
/**
21572157
* Removes unnecessary nodes from the graph after decoding.
21582158
*
2159-
* @param methodScope The current method.
2159+
* @param rootMethodScope The current method.
21602160
*/
2161-
protected void cleanupGraph(MethodScope methodScope) {
2161+
protected void cleanupGraph(MethodScope rootMethodScope) {
21622162
for (MergeNode merge : graph.getNodes(MergeNode.TYPE)) {
21632163
for (ProxyPlaceholder placeholder : merge.usages().filter(ProxyPlaceholder.class).snapshot()) {
21642164
placeholder.replaceAndDelete(placeholder.value);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/MemoryMapControlSinkNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import jdk.graal.compiler.nodes.memory.MemoryMapNode;
3434

3535
/**
36-
* {@link ControlSinkNode}that might have a {@link MemoryMapNode} attached. This map can be used to
36+
* {@link ControlSinkNode} that might have a {@link MemoryMapNode} attached. This map can be used to
3737
* update the memory graph during inlining.
3838
*/
3939
@NodeInfo

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/SimplifyingGraphDecoder.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ public SimplifyingGraphDecoder(Architecture architecture, StructuredGraph graph,
141141
}
142142

143143
@Override
144-
protected void cleanupGraph(MethodScope methodScope) {
144+
protected void cleanupGraph(MethodScope rootMethodScope) {
145145
GraphUtil.normalizeLoops(graph);
146-
super.cleanupGraph(methodScope);
146+
super.cleanupGraph(rootMethodScope);
147147

148148
/*
149149
* To avoid calling tryKillUnused for each individual floating node we remove during
@@ -152,7 +152,7 @@ protected void cleanupGraph(MethodScope methodScope) {
152152
*/
153153
final NodeBitMap unusedNodes = new NodeBitMap(graph);
154154

155-
for (Node node : graph.getNewNodes(methodScope.methodStartMark)) {
155+
for (Node node : graph.getNewNodes(rootMethodScope.methodStartMark)) {
156156
if (node instanceof MergeNode mergeNode) {
157157
if (mergeNode.forwardEndCount() == 1) {
158158
graph.reduceTrivialMerge(mergeNode, false, unusedNodes);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/PEGraphDecoder.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,7 @@ public PEGraphDecoder(Architecture architecture, StructuredGraph graph, CoreProv
878878
this.forceLink = forceLink;
879879
}
880880

881-
protected static LoopExplosionKind loopExplosionKind(ResolvedJavaMethod method, LoopExplosionPlugin loopExplosionPlugin) {
881+
private static LoopExplosionKind loopExplosionKind(ResolvedJavaMethod method, LoopExplosionPlugin loopExplosionPlugin) {
882882
if (loopExplosionPlugin == null) {
883883
return LoopExplosionKind.NONE;
884884
} else {
@@ -917,8 +917,8 @@ protected PEMethodScope createMethodScope(StructuredGraph targetGraph, PEMethodS
917917
}
918918

919919
@Override
920-
protected void cleanupGraph(MethodScope methodScope) {
921-
super.cleanupGraph(methodScope);
920+
protected void cleanupGraph(MethodScope rootMethodScope) {
921+
super.cleanupGraph(rootMethodScope);
922922

923923
for (FrameState frameState : graph.getNodes(FrameState.TYPE)) {
924924
if (frameState.bci == BytecodeFrame.UNWIND_BCI) {
@@ -928,7 +928,7 @@ protected void cleanupGraph(MethodScope methodScope) {
928928
* anything because the usages of the frameState are not available yet. So we need
929929
* to call it again.
930930
*/
931-
PEMethodScope peMethodScope = (PEMethodScope) methodScope;
931+
PEMethodScope peMethodScope = (PEMethodScope) rootMethodScope;
932932
Invoke invoke = peMethodScope.invokeData != null ? peMethodScope.invokeData.invoke : null;
933933
InliningUtil.handleMissingAfterExceptionFrameState(frameState, invoke, null, true);
934934

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/CachingPEGraphDecoder.java‎ renamed to ‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/CachingPEGraphDecoder.java‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@
2222
* or visit www.oracle.com if you need additional information or have any
2323
* questions.
2424
*/
25-
package jdk.graal.compiler.replacements;
26-
27-
import static jdk.graal.compiler.core.common.NativeImageSupport.inRuntimeCode;
25+
package jdk.graal.compiler.truffle;
2826

2927
import java.util.concurrent.ConcurrentHashMap;
3028
import java.util.function.Supplier;
@@ -55,14 +53,15 @@
5553
import jdk.graal.compiler.phases.common.CanonicalizerPhase;
5654
import jdk.graal.compiler.phases.common.DominatorBasedGlobalValueNumberingPhase;
5755
import jdk.graal.compiler.phases.util.Providers;
56+
import jdk.graal.compiler.replacements.PEGraphDecoder;
5857
import jdk.vm.ci.code.Architecture;
5958
import jdk.vm.ci.meta.ResolvedJavaMethod;
6059

6160
/**
6261
* A graph decoder that provides all necessary encoded graphs on-the-fly (by parsing the methods and
6362
* encoding the graphs).
6463
*/
65-
public class CachingPEGraphDecoder extends PEGraphDecoder {
64+
public final class CachingPEGraphDecoder extends PEGraphDecoder {
6665

6766
private static final TimerKey BuildGraphTimer = DebugContext.timer("PartialEvaluation-GraphBuilding");
6867

@@ -118,14 +117,9 @@ public CachingPEGraphDecoder(Architecture architecture,
118117
}
119118

120119
@SuppressWarnings("try")
121-
private EncodedGraph createGraph(ResolvedJavaMethod method, BytecodeProvider intrinsicBytecodeProvider) {
120+
private EncodedGraph createGraph(ResolvedJavaMethod method) {
122121
CanonicalizerPhase canonicalizer = CanonicalizerPhase.create();
123-
StructuredGraph graphToEncode;
124-
if (graph.isSubstitution() && inRuntimeCode()) {
125-
throw GraalError.shouldNotReachHere("dead path"); // ExcludeFromJacocoGeneratedReport
126-
} else {
127-
graphToEncode = buildGraph(method, intrinsicBytecodeProvider, canonicalizer);
128-
}
122+
StructuredGraph graphToEncode = buildGraph(method, canonicalizer);
129123

130124
/*
131125
* ConvertDeoptimizeToGuardPhase reduces the number of merges in the graph, so that fewer
@@ -145,7 +139,7 @@ private EncodedGraph createGraph(ResolvedJavaMethod method, BytecodeProvider int
145139
}
146140

147141
@SuppressWarnings("try")
148-
private StructuredGraph buildGraph(ResolvedJavaMethod method, BytecodeProvider intrinsicBytecodeProvider, CanonicalizerPhase canonicalizer) {
142+
private StructuredGraph buildGraph(ResolvedJavaMethod method, CanonicalizerPhase canonicalizer) {
149143
StructuredGraph graphToEncode;
150144
/*
151145
* Always parse graphs without assumptions, this is required when graphs are cached across
@@ -163,9 +157,6 @@ private StructuredGraph buildGraph(ResolvedJavaMethod method, BytecodeProvider i
163157
build();
164158
// @formatter:on
165159
try (DebugContext.Scope scope = debug.scope("buildGraph", graphToEncode); DebugCloseable a = BuildGraphTimer.start(debug)) {
166-
if (intrinsicBytecodeProvider != null) {
167-
throw GraalError.shouldNotReachHere("isn't this dead?"); // ExcludeFromJacocoGeneratedReport
168-
}
169160
graphBuilderPhaseInstance.apply(graphToEncode);
170161
canonicalizer.apply(graphToEncode, graphCacheProviders);
171162
if (postParsingPhase != null) {
@@ -177,14 +168,14 @@ private StructuredGraph buildGraph(ResolvedJavaMethod method, BytecodeProvider i
177168
return graphToEncode;
178169
}
179170

180-
@SuppressWarnings({"unused", "try"})
181-
private EncodedGraph lookupOrCreatePersistentEncodedGraph(ResolvedJavaMethod method, BytecodeProvider intrinsicBytecodeProvider) {
171+
@SuppressWarnings("try")
172+
private EncodedGraph lookupOrCreatePersistentEncodedGraph(ResolvedJavaMethod method) {
182173
EncodedGraph result = persistentGraphCache.get(method);
183174
if (result == null && method.hasBytecodes()) {
184175
try (AutoCloseable scope = createPersistentCachedGraphScope.get()) {
185176
// Encoded graphs creation must be wrapped by "scopes" provided by
186177
// createCachedGraphScope.
187-
result = createGraph(method, intrinsicBytecodeProvider);
178+
result = createGraph(method);
188179
} catch (Throwable ex) {
189180
throw debug.handle(ex);
190181
}
@@ -195,17 +186,26 @@ private EncodedGraph lookupOrCreatePersistentEncodedGraph(ResolvedJavaMethod met
195186

196187
@Override
197188
protected EncodedGraph lookupEncodedGraph(ResolvedJavaMethod method, BytecodeProvider intrinsicBytecodeProvider) {
189+
if (intrinsicBytecodeProvider != null) {
190+
/*
191+
* intrinsicBytecodeProvider is read from
192+
* InlineInvokePlugin.InlineInfo#getIntrinsicBytecodeProvider which is always null
193+
* because the only source is PartialEvaluator#asInlineInfo
194+
*/
195+
throw GraalError.shouldNotReachHere("dead path"); // ExcludeFromJacocoGeneratedReport
196+
}
197+
198198
EncodedGraph result = localGraphCache.get(method);
199199
if (result != null) {
200200
return result;
201201
}
202202

203-
result = lookupOrCreatePersistentEncodedGraph(method, intrinsicBytecodeProvider);
203+
result = lookupOrCreatePersistentEncodedGraph(method);
204204
// Cached graph from previous compilation may not have source positions, re-parse and
205205
// store in compilation-local cache.
206206
if (result != null && !result.trackNodeSourcePosition() && graph.trackNodeSourcePosition()) {
207207
assert method.hasBytecodes();
208-
result = createGraph(method, intrinsicBytecodeProvider);
208+
result = createGraph(method);
209209
assert result.trackNodeSourcePosition();
210210
}
211211

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/truffle/PartialEvaluator.java‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
import jdk.graal.compiler.phases.OptimisticOptimizations;
6363
import jdk.graal.compiler.phases.contract.NodeCostUtil;
6464
import jdk.graal.compiler.phases.util.Providers;
65-
import jdk.graal.compiler.replacements.CachingPEGraphDecoder;
6665
import jdk.graal.compiler.replacements.InlineDuringParsingPlugin;
6766
import jdk.graal.compiler.replacements.PEGraphDecoder;
6867
import jdk.graal.compiler.replacements.ReplacementsImpl;
@@ -367,22 +366,14 @@ private final class PELoopExplosionPlugin implements LoopExplosionPlugin {
367366
@Override
368367
public LoopExplosionKind loopExplosionKind(ResolvedJavaMethod method) {
369368
TruffleCompilerRuntime.LoopExplosionKind explosionKind = getMethodInfo(method).loopExplosion();
370-
switch (explosionKind) {
371-
case NONE:
372-
return LoopExplosionKind.NONE;
373-
case FULL_EXPLODE:
374-
return LoopExplosionKind.FULL_EXPLODE;
375-
case FULL_EXPLODE_UNTIL_RETURN:
376-
return LoopExplosionKind.FULL_EXPLODE_UNTIL_RETURN;
377-
case FULL_UNROLL:
378-
return LoopExplosionKind.FULL_UNROLL;
379-
case MERGE_EXPLODE:
380-
return LoopExplosionKind.MERGE_EXPLODE;
381-
case FULL_UNROLL_UNTIL_RETURN:
382-
return LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN;
383-
default:
384-
throw new IllegalStateException("Unsupported TruffleCompilerRuntime.LoopExplosionKind: " + String.valueOf(explosionKind));
385-
}
369+
return switch (explosionKind) {
370+
case NONE -> LoopExplosionKind.NONE;
371+
case FULL_EXPLODE -> LoopExplosionKind.FULL_EXPLODE;
372+
case FULL_EXPLODE_UNTIL_RETURN -> LoopExplosionKind.FULL_EXPLODE_UNTIL_RETURN;
373+
case FULL_UNROLL -> LoopExplosionKind.FULL_UNROLL;
374+
case MERGE_EXPLODE -> LoopExplosionKind.MERGE_EXPLODE;
375+
case FULL_UNROLL_UNTIL_RETURN -> LoopExplosionKind.FULL_UNROLL_UNTIL_RETURN;
376+
};
386377
}
387378
}
388379

@@ -422,13 +413,12 @@ private GraphBuilderConfiguration getGraphBuilderConfigurationCopy(boolean force
422413
protected abstract GraphBuilderPhase.Instance createGraphBuilderPhaseInstance(CoreProviders providers, GraphBuilderConfiguration graphBuilderConfig, OptimisticOptimizations optimisticOpts);
423414

424415
@SuppressWarnings("try")
425-
public void doGraphPE(TruffleTierContext context, InlineInvokePlugin inlineInvokePlugin, EconomicMap<ResolvedJavaMethod, EncodedGraph> graphCache) {
426-
InlineInvokePlugin[] inlineInvokePlugins = new InlineInvokePlugin[]{
427-
inlineInvokePlugin
428-
};
416+
public final void doGraphPE(TruffleTierContext context, InlineInvokePlugin inlineInvokePlugin, EconomicMap<ResolvedJavaMethod, EncodedGraph> graphCache) {
429417
PEGraphDecoder decoder = createGraphDecoder(context,
430418
context.isFirstTier() ? firstTierDecodingPlugins : lastTierDecodingPlugins,
431-
inlineInvokePlugins,
419+
new InlineInvokePlugin[]{
420+
inlineInvokePlugin
421+
},
432422
new InterceptReceiverPlugin(context.compilable),
433423
nodePlugins,
434424
new TruffleSourceLanguagePositionProvider(context.task),

0 commit comments

Comments
 (0)