Skip to content

Commit

Permalink
[GR-43432] [GR-42824] Fixups for ParseOnceJIT Runtime Graphs.
Browse files Browse the repository at this point in the history 
PullRequest: graal/13814
  • Loading branch information
@teshull
teshull committed Feb 22, 2023
2 parents 5544c4f + 7eb7c5e commit 65c4670
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 32 deletions.
2 changes: 1 addition & 1 deletion ...g.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/PEGraphDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ protected boolean tryInvocationPlugin(PEMethodScope methodScope, LoopScope loopS
}
}

private InvocationPlugin getInvocationPlugin(ResolvedJavaMethod targetMethod) {
protected InvocationPlugin getInvocationPlugin(ResolvedJavaMethod targetMethod) {
Object invocationPlugin = invocationPluginCache.computeIfAbsent(targetMethod, method -> {
Object plugin = invocationPlugins.lookupInvocation(targetMethod, options);
if (plugin == null) {
Expand Down
2 changes: 1 addition & 1 deletion ...tevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodTypeFlow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public synchronized boolean updateFlowsGraph(PointsToAnalysis bb, MethodFlowsGra
throwSealedError();
}

forceReparseOnCreation = forceReparse | forceReparseOnCreation;
forceReparseOnCreation = forceReparse || forceReparseOnCreation;

assert !(newGraphKind == MethodFlowsGraph.GraphKind.STUB && graphKind == MethodFlowsGraph.GraphKind.FULL) : "creating less strict graph";
MethodFlowsGraph.GraphKind originalGraphKind = graphKind;
Expand Down
9 changes: 9 additions & 0 deletions ...graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysisGraphDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.graalvm.compiler.nodes.StructuredGraph;
import org.graalvm.compiler.nodes.ValueNode;
import org.graalvm.compiler.nodes.graphbuilderconf.InlineInvokePlugin;
import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugin;
import org.graalvm.compiler.nodes.util.GraphUtil;
import org.graalvm.compiler.replacements.PEGraphDecoder;

Expand Down Expand Up @@ -98,6 +99,14 @@ protected InlineBeforeAnalysisGraphDecoder(BigBang bb, InlineBeforeAnalysisPolic
}
}

@Override
protected InvocationPlugin getInvocationPlugin(ResolvedJavaMethod targetMethod) {
if (policy.tryInvocationPlugins()) {
return super.getInvocationPlugin(targetMethod);
}
return null;
}

@Override
protected PEMethodScope createMethodScope(StructuredGraph targetGraph, PEMethodScope caller, LoopScope callerLoopScope, EncodedGraph encodedGraph, ResolvedJavaMethod method, InvokeData invokeData,
int inliningDepth, ValueNode[] arguments) {
Expand Down
8 changes: 8 additions & 0 deletions ...racle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysisPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ protected boolean shouldInlineInvoke(GraphBuilderContext b, ResolvedJavaMethod m
return false;
}

protected boolean tryInvocationPlugins() {
/*
* If an invocation plugin was unable to be used during bytecode parsing, then it will be
* retried during graph decoding. In the default case this should not happen.
*/
return false;
}

protected S createRootScope() {
return null;
}
Expand Down
6 changes: 1 addition & 5 deletions .../com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/SubstrateCompilationDirectives.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,7 @@ public boolean isDeoptEntry(MultiMethod method, int bci, boolean duringCall, boo
return true;
}

Map<Long, DeoptSourceFrameInfo> bciMap = deoptEntries.get(toAnalysisMethod((ResolvedJavaMethod) method));
assert bciMap != null : "can only query for deopt entries for methods registered as deopt targets";

long encodedBci = FrameInfoEncoder.encodeBci(bci, duringCall, rethrowException);
return bciMap.containsKey(encodedBci);
return isRegisteredDeoptEntry(method, bci, duringCall, rethrowException);
}

public boolean isRegisteredDeoptEntry(MultiMethod method, int bci, boolean duringCall, boolean rethrowException) {
Expand Down
57 changes: 32 additions & 25 deletions ...rc/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor;
import com.oracle.graal.pointsto.meta.HostedProviders;
import com.oracle.svm.common.meta.MultiMethod;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.deopt.DeoptTest;
import com.oracle.svm.core.graal.nodes.DeoptEntryBeginNode;
import com.oracle.svm.core.graal.nodes.DeoptEntryNode;
Expand All @@ -61,8 +60,7 @@
import com.oracle.svm.core.heap.Pod.RuntimeSupport.PodFactory;
import com.oracle.svm.core.util.VMError;
import com.oracle.svm.hosted.annotation.CustomSubstitutionMethod;
import com.oracle.svm.hosted.code.CompilationInfo;
import com.oracle.svm.hosted.meta.HostedMethod;
import com.oracle.svm.hosted.code.SubstrateCompilationDirectives;
import com.oracle.svm.hosted.nodes.DeoptProxyNode;
import com.oracle.svm.hosted.phases.HostedGraphKit;

Expand Down Expand Up @@ -93,13 +91,26 @@ public ResolvedJavaMethod resolve(ResolvedJavaMethod method) {
}

final class PodFactorySubstitutionMethod extends CustomSubstitutionMethod {

private static class DeoptInfoProvider {
final MultiMethod method;

DeoptInfoProvider(MultiMethod method) {
this.method = method;
}

boolean isDeoptEntry(int bci, boolean duringCall, boolean rethrowException) {
return SubstrateCompilationDirectives.singleton().isDeoptEntry(method, bci, duringCall, rethrowException);
}
}

PodFactorySubstitutionMethod(ResolvedJavaMethod original) {
super(original);
}

@Override
public boolean allowRuntimeCompilation() {
return !SubstrateOptions.parseOnce();
return true;
}

@Override
Expand All @@ -114,13 +125,9 @@ public StructuredGraph buildGraph(DebugContext debug, ResolvedJavaMethod method,
boolean trackNodeSourcePosition = (purpose == Purpose.ANALYSIS);

HostedGraphKit kit = new HostedGraphKit(debug, providers, method, trackNodeSourcePosition);
CompilationInfo deoptTargetInfo = null;
DeoptInfoProvider deoptInfo = null;
if (MultiMethod.isDeoptTarget(method)) {
if (method instanceof HostedMethod) {
deoptTargetInfo = ((HostedMethod) method).compilationInfo;
} else {
throw VMError.unimplemented("Pod support currently does not work with ParseOnceJIT");
}
deoptInfo = new DeoptInfoProvider((MultiMethod) method);
}

ResolvedJavaType factoryType = method.getDeclaringClass();
Expand All @@ -134,12 +141,12 @@ public StructuredGraph buildGraph(DebugContext debug, ResolvedJavaMethod method,
* allocated instance in a local and load it after each step during which a deopt can occur.
*/
int instanceLocal = kit.getFrameState().localsSize() - 1; // reserved when generating class
int nextDeoptIndex = startMethod(kit, deoptTargetInfo, 0);
int nextDeoptIndex = startMethod(kit, deoptInfo, 0);
instantiatePod(kit, providers, factoryType, podConcreteType, instanceLocal);
if (isAnnotationPresent(DeoptTest.class)) {
kit.append(new TestDeoptimizeNode());
}
nextDeoptIndex = invokeConstructor(kit, method, deoptTargetInfo, nextDeoptIndex, targetCtor, instanceLocal);
nextDeoptIndex = invokeConstructor(kit, method, deoptInfo, nextDeoptIndex, targetCtor, instanceLocal);

kit.createReturn(kit.loadLocal(instanceLocal, JavaKind.Object), JavaKind.Object);
return kit.finalizeGraph();
Expand All @@ -160,17 +167,17 @@ private ResolvedJavaMethod findMatchingConstructor(ResolvedJavaMethod method, Re
throw new GraalError("Matching constructor not found: %s", getSignature());
}

private static boolean shouldInsertDeoptEntry(CompilationInfo deoptTargetInfo, int bci, boolean duringCall, boolean rethrowException) {
if (deoptTargetInfo != null) {
return deoptTargetInfo.isDeoptEntry(bci, duringCall, rethrowException);
private static boolean shouldInsertDeoptEntry(DeoptInfoProvider deoptInfo, int bci, boolean duringCall, boolean rethrowException) {
if (deoptInfo != null) {
return deoptInfo.isDeoptEntry(bci, duringCall, rethrowException);
}
return false;
}

private static int startMethod(HostedGraphKit kit, CompilationInfo deoptTargetInfo, int nextDeoptIndex) {
if (deoptTargetInfo != null) {
private static int startMethod(HostedGraphKit kit, DeoptInfoProvider deoptInfo, int nextDeoptIndex) {
if (deoptInfo != null) {
FrameState initialState = kit.getGraph().start().stateAfter();
if (shouldInsertDeoptEntry(deoptTargetInfo, initialState.bci, false, false)) {
if (shouldInsertDeoptEntry(deoptInfo, initialState.bci, false, false)) {
return appendDeoptWithExceptionUnwind(kit, initialState, initialState.bci, nextDeoptIndex);
}
}
Expand All @@ -192,32 +199,32 @@ private static ValueNode loadNonNullField(HostedGraphKit kit, ValueNode object,
return kit.append(PiNode.create(kit.createLoadField(object, field), StampFactory.objectNonNull()));
}

private static int invokeConstructor(HostedGraphKit kit, ResolvedJavaMethod method, CompilationInfo deoptTargetInfo, int nextDeoptIndex, ResolvedJavaMethod targetCtor, int instanceLocal) {
private static int invokeConstructor(HostedGraphKit kit, ResolvedJavaMethod method, DeoptInfoProvider deoptInfo, int nextDeoptIndex, ResolvedJavaMethod targetCtor, int instanceLocal) {
ValueNode instance = kit.loadLocal(instanceLocal, JavaKind.Object);
ValueNode[] originalArgs = kit.loadArguments(method.toParameterTypes()).toArray(ValueNode.EMPTY_ARRAY);
ValueNode[] invokeArgs = Arrays.copyOf(originalArgs, originalArgs.length);
invokeArgs[0] = instance;
return invokeWithDeoptAndExceptionUnwind(kit, deoptTargetInfo, nextDeoptIndex, targetCtor, InvokeKind.Special, invokeArgs);
return invokeWithDeoptAndExceptionUnwind(kit, deoptInfo, nextDeoptIndex, targetCtor, InvokeKind.Special, invokeArgs);
}

/** @see com.oracle.svm.hosted.phases.SharedGraphBuilderPhase */
private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, CompilationInfo deoptTargetInfo, int initialNextDeoptIndex, ResolvedJavaMethod target, InvokeKind invokeKind,
private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptInfoProvider deoptInfo, int initialNextDeoptIndex, ResolvedJavaMethod target, InvokeKind invokeKind,
ValueNode... args) {
int bci = kit.bci();
InvokeWithExceptionNode invoke = kit.startInvokeWithException(target, invokeKind, kit.getFrameState(), bci, args);
invoke.setNodeSourcePosition(NodeSourcePosition.placeholder(kit.getGraph().method(), bci));
kit.exceptionPart();
ExceptionObjectNode exception = kit.exceptionObject();

if (deoptTargetInfo == null) {
if (deoptInfo == null) {
kit.append(new UnwindNode(exception));
kit.endInvokeWithException();
return initialNextDeoptIndex;
}

int nextDeoptIndex = initialNextDeoptIndex;

if (shouldInsertDeoptEntry(deoptTargetInfo, bci, false, true)) {
if (shouldInsertDeoptEntry(deoptInfo, bci, false, true)) {
// Exception during invoke

var exceptionDeopt = kit.add(new DeoptEntryNode());
Expand All @@ -237,13 +244,13 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, Compila
kit.append(new UnwindNode(exception));
}

if (shouldInsertDeoptEntry(deoptTargetInfo, invoke.stateAfter().bci, false, false)) {
if (shouldInsertDeoptEntry(deoptInfo, invoke.stateAfter().bci, false, false)) {
// Deopt entry after invoke without exception

kit.noExceptionPart();
nextDeoptIndex = appendDeoptWithExceptionUnwind(kit, invoke.stateAfter(), invoke.stateAfter().bci, nextDeoptIndex);
} else {
VMError.guarantee(!shouldInsertDeoptEntry(deoptTargetInfo, bci, true, false), "need to add support for inserting DeoptProxyAnchorNode");
VMError.guarantee(!shouldInsertDeoptEntry(deoptInfo, bci, true, false), "need to add support for inserting DeoptProxyAnchorNode");
}
kit.endInvokeWithException();

Expand Down
9 changes: 9 additions & 0 deletions ...om.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/InlineBeforeAnalysisPolicyImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@ protected boolean shouldInlineInvoke(GraphBuilderContext b, ResolvedJavaMethod m
return inliningAllowed(hostVM, b, method);
}

@Override
protected boolean tryInvocationPlugins() {
/*
* We conditionally allow the invocation plugin to be triggered during graph decoding to see
* what happens.
*/
return true;
}

public static boolean inliningAllowed(SVMHost hostVM, GraphBuilderContext b, ResolvedJavaMethod method) {
AnalysisMethod caller = (AnalysisMethod) b.getMethod();
AnalysisMethod callee = (AnalysisMethod) method;
Expand Down

0 comments on commit 65c4670

Please sign in to comment.