Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GR-42162] Add support for tracking inlining during inlinebeforeanalysis. #7163

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ public GraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Providers pro
// Set up a default value that everything constructed by GraphKit will use.
graph.withNodeSourcePosition(NodeSourcePosition.substitution(stubMethod));
}
graph.recordMethod(stubMethod);
this.wordTypes = wordTypes;
this.graphBuilderPlugins = graphBuilderPlugins;
this.lastFixedNode = graph.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,14 @@ public StructuredGraph.AllowAssumptions allowAssumptions(AnalysisMethod method)
return StructuredGraph.AllowAssumptions.NO;
}

/**
* @return Whether which methods were inlined should be recorded.
*/
@SuppressWarnings("unused")
public boolean recordInlinedMethods(AnalysisMethod method) {
return false;
}

public void initializeProviders(HostedProviders newProviders) {
AnalysisError.guarantee(providers == null, "can only initialize providers once");
providers = newProviders;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,7 @@ public StructuredGraph decodeAnalyzedGraph(DebugContext debug, Iterable<EncodedN
}

var allowAssumptions = getUniverse().hostVM().allowAssumptions(this);
// Note we never record inlined methods. This is correct even for runtime compiled methods
StructuredGraph result = new StructuredGraph.Builder(debug.getOptions(), debug, allowAssumptions).method(this).recordInlinedMethods(false).trackNodeSourcePosition(
analyzedGraph.trackNodeSourcePosition()).build();
GraphDecoder decoder = new GraphDecoder(AnalysisParsedGraph.HOST_ARCHITECTURE, result);
Expand All @@ -960,6 +961,10 @@ public void setAnalyzedGraph(EncodedGraph analyzedGraph) {
this.analyzedGraph = analyzedGraph;
}

public EncodedGraph getAnalyzedGraph() {
return analyzedGraph;
}

@Override
public MultiMethodKey getMultiMethodKey() {
return multiMethodKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public static StructuredGraph decodeGraph(BigBang bb, AnalysisMethod method, Ana

StructuredGraph result = new StructuredGraph.Builder(bb.getOptions(), debug, bb.getHostVM().allowAssumptions(method))
.method(method)
.recordInlinedMethods(false)
.recordInlinedMethods(bb.getHostVM().recordInlinedMethods(method))
.trackNodeSourcePosition(analysisParsedGraph.getEncodedGraph().trackNodeSourcePosition())
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@
*/
package com.oracle.graal.pointsto.phases;

import static com.oracle.graal.pointsto.phases.InlineBeforeAnalysisGraphDecoder.InlineBeforeAnalysisMethodScope.recordInlined;

import java.util.ArrayDeque;
import java.util.Deque;
import java.util.concurrent.ConcurrentHashMap;

import org.graalvm.collections.EconomicSet;
import org.graalvm.compiler.bytecode.BytecodeProvider;
import org.graalvm.compiler.debug.GraalError;
import org.graalvm.compiler.graph.Node;
Expand Down Expand Up @@ -64,6 +67,12 @@ public class InlineBeforeAnalysisMethodScope extends PEMethodScope {

private boolean inliningAborted;

/*
* We temporarily track all graphs actually encoded (i.e., not aborted) so that all
* recording can be performed afterwards.
*/
private final EconomicSet<EncodedGraph> encodedGraphs;

InlineBeforeAnalysisMethodScope(StructuredGraph targetGraph, PEMethodScope caller, LoopScope callerLoopScope, EncodedGraph encodedGraph, ResolvedJavaMethod method,
InvokeData invokeData, int inliningDepth, ValueNode[] arguments) {
super(targetGraph, caller, callerLoopScope, encodedGraph, method, invokeData, inliningDepth, arguments);
Expand All @@ -83,6 +92,16 @@ public class InlineBeforeAnalysisMethodScope extends PEMethodScope {
graph.getDebug().logv(" ".repeat(inliningDepth) + "openCalleeScope for " + method.format("%H.%n(%p)") + ": " + policyScope);
}
}
encodedGraphs = EconomicSet.create();
}

static void recordInlined(InlineBeforeAnalysisMethodScope callerScope, InlineBeforeAnalysisMethodScope calleeScope) {
/*
* Update caller's encoded graphs
*/
var callerEncodedGraphs = callerScope.encodedGraphs;
callerEncodedGraphs.addAll(calleeScope.encodedGraphs);
callerEncodedGraphs.add(calleeScope.encodedGraph);
}
}

Expand Down Expand Up @@ -111,6 +130,18 @@ protected InvocationPlugin getInvocationPlugin(ResolvedJavaMethod targetMethod)
return null;
}

@Override
protected void cleanupGraph(MethodScope ms) {
super.cleanupGraph(ms);

// at the very end we record all inlining
var methodScope = cast(ms);
methodScope.encodedGraphs.add(methodScope.encodedGraph);
for (var encodedGraph : methodScope.encodedGraphs) {
super.recordGraphElements(encodedGraph);
}
}

@Override
protected PEMethodScope createMethodScope(StructuredGraph targetGraph, PEMethodScope caller, LoopScope callerLoopScope, EncodedGraph encodedGraph, ResolvedJavaMethod method, InvokeData invokeData,
int inliningDepth, ValueNode[] arguments) {
Expand Down Expand Up @@ -195,6 +226,14 @@ protected LoopScope processNextNode(MethodScope ms, LoopScope loopScope) {
return super.processNextNode(methodScope, loopScope);
}

@Override
protected void recordGraphElements(EncodedGraph encodedGraph) {
/*
* We temporarily delay recording graph elements, as at this point it is possible inlining
* will be aborted.
*/
}

@Override
protected void finishInlining(MethodScope is) {
InlineBeforeAnalysisMethodScope inlineScope = cast(is);
Expand Down Expand Up @@ -235,6 +274,8 @@ protected void finishInlining(MethodScope is) {
callerScope.policyScope.commitCalleeScope(inlineScope.policyScope);
}

recordInlined(callerScope, inlineScope);

NodeSourcePosition callerBytecodePosition = callerScope.getCallerNodeSourcePosition();
Object reason = callerBytecodePosition != null ? callerBytecodePosition : callerScope.method;
reason = reason == null ? graph.method() : reason;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ private static boolean trackNodeSourcePosition(boolean forceTrackNodeSourcePosit

@SuppressWarnings("this-escape")
public SubstrateGraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Providers providers, WordTypes wordTypes,
GraphBuilderConfiguration.Plugins graphBuilderPlugins, CompilationIdentifier compilationId, boolean forceTrackNodeSourcePosition) {
super(debug, stubMethod, providers, wordTypes, graphBuilderPlugins, compilationId, null, trackNodeSourcePosition(forceTrackNodeSourcePosition), false);
GraphBuilderConfiguration.Plugins graphBuilderPlugins, CompilationIdentifier compilationId, boolean forceTrackNodeSourcePosition, boolean recordInlinedMethods) {
super(debug, stubMethod, providers, wordTypes, graphBuilderPlugins, compilationId, null, trackNodeSourcePosition(forceTrackNodeSourcePosition), recordInlinedMethods);
assert wordTypes != null : "Support for Word types is mandatory";
frameState = new FrameStateBuilder(this, stubMethod, graph);
frameState.disableKindVerification();
Expand All @@ -113,6 +113,11 @@ public SubstrateGraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Prov
graph.start().setStateAfter(frameState.create(bci(), graph.start()));
}

public SubstrateGraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Providers providers, WordTypes wordTypes,
GraphBuilderConfiguration.Plugins graphBuilderPlugins, CompilationIdentifier compilationId, boolean forceTrackNodeSourcePosition) {
this(debug, stubMethod, providers, wordTypes, graphBuilderPlugins, compilationId, forceTrackNodeSourcePosition, false);
}

@Override
protected MethodCallTargetNode createMethodCallTarget(InvokeKind invokeKind, ResolvedJavaMethod targetMethod, ValueNode[] args, StampPair returnStamp, int bci) {
return new SubstrateMethodCallTargetNode(invokeKind, targetMethod, args, returnStamp, null, null, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ public boolean allowAssumptions(AnalysisMethod method) {
return false;
}

@Override
public boolean recordInlinedMethods(AnalysisMethod method) {
return false;
}

@Override
public HostedProviders getHostedProviders(MultiMethod.MultiMethodKey key) {
/* The buildtime providers are always used. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.graalvm.collections.EconomicMap;
import org.graalvm.compiler.core.common.PermanentBailoutException;
Expand Down Expand Up @@ -214,9 +215,11 @@ public int hashCode() {

static final class RuntimeCompiledMethodImpl implements RuntimeCompiledMethod {
final AnalysisMethod method;
final Collection<ResolvedJavaMethod> inlinedMethods;

private RuntimeCompiledMethodImpl(AnalysisMethod method) {
private RuntimeCompiledMethodImpl(AnalysisMethod method, Collection<ResolvedJavaMethod> inlinedMethods) {
this.method = method;
this.inlinedMethods = inlinedMethods;
}

@Override
Expand All @@ -226,10 +229,7 @@ public AnalysisMethod getMethod() {

@Override
public Collection<ResolvedJavaMethod> getInlinedMethods() {
/*
* Currently no inlining is performed when ParseOnceJIT is enabled.
*/
return List.of();
return inlinedMethods;
}

@Override
Expand Down Expand Up @@ -399,8 +399,15 @@ public void afterAnalysis(AfterAnalysisAccess access) {
for (var method : impl.getUniverse().getMethods()) {
var rMethod = method.getMultiMethod(RUNTIME_COMPILED_METHOD);
if (rMethod != null && rMethod.isReachable() && !invalidForRuntimeCompilation.containsKey(rMethod)) {
boolean added = runtimeCompilations.add(new RuntimeCompiledMethodImpl(method));
assert !added || runtimeCompiledMethodCallTree.containsKey(method);
var runtimeInlinedMethods = rMethod.getAnalyzedGraph().getInlinedMethods();
var inlinedMethods = runtimeInlinedMethods.stream().map(inlinedMethod -> {
ResolvedJavaMethod orig = ((AnalysisMethod) inlinedMethod).getMultiMethod(ORIGINAL_METHOD);
assert orig != null;
return orig;
}).collect(Collectors.toUnmodifiableSet());
boolean added = runtimeCompilations.add(new RuntimeCompiledMethodImpl(method, inlinedMethods));
assert added;
assert runtimeCompiledMethodCallTree.containsKey(method);
}
}

Expand Down Expand Up @@ -763,6 +770,11 @@ public boolean allowAssumptions(AnalysisMethod method) {
return method.getMultiMethodKey() == RUNTIME_COMPILED_METHOD;
}

@Override
public boolean recordInlinedMethods(AnalysisMethod method) {
return method.getMultiMethodKey() == RUNTIME_COMPILED_METHOD;
}

@Override
public Object parseGraph(BigBang bb, DebugContext debug, AnalysisMethod method) {
// want to have a couple more checks here that are in DeoptimizationUtils
Expand Down Expand Up @@ -811,9 +823,7 @@ private Object parseRuntimeCompiledMethod(BigBang bb, DebugContext debug, Analys
if (parsed) {
// enable this logging to get log output in compilation passes
try (Indent indent2 = debug.logAndIndent("parse graph phases")) {
RuntimeGraphBuilderPhase
.createRuntimeGraphBuilderPhase(bb, analysisProviders, graphBuilderConfig, optimisticOpts)
.apply(graph);
RuntimeGraphBuilderPhase.createRuntimeGraphBuilderPhase(bb, analysisProviders, graphBuilderConfig, optimisticOpts).apply(graph);
} catch (PermanentBailoutException ex) {
bb.getUnsupportedFeatures().addMessage(method.format("%H.%n(%p)"), method, ex.getLocalizedMessage(), null, ex);
recordFailed(method);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,14 @@ public StructuredGraph.AllowAssumptions allowAssumptions(AnalysisMethod method)
return super.allowAssumptions(method);
}

@Override
public boolean recordInlinedMethods(AnalysisMethod method) {
if (parsingSupport != null) {
return parsingSupport.recordInlinedMethods(method);
}
return super.recordInlinedMethods(method);
}

@Override
public HostedProviders getProviders(MultiMethod.MultiMethodKey key) {
if (parsingSupport != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public interface SVMParsingSupport {

boolean allowAssumptions(AnalysisMethod method);

boolean recordInlinedMethods(AnalysisMethod method);

HostedProviders getHostedProviders(MultiMethod.MultiMethodKey key);

void initializeInlineBeforeAnalysisPolicy(SVMHost svmHost, InlineBeforeAnalysisPolicyUtils inliningUtils);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.oracle.svm.core.graal.replacements.SubstrateGraphKit;
import com.oracle.svm.core.nodes.SubstrateMethodCallTargetNode;
import com.oracle.svm.core.util.VMError;
import com.oracle.svm.hosted.code.SubstrateCompilationDirectives;
import com.oracle.svm.hosted.meta.HostedMethod;

import jdk.vm.ci.meta.JavaKind;
Expand All @@ -65,7 +66,8 @@
public class HostedGraphKit extends SubstrateGraphKit {

public HostedGraphKit(DebugContext debug, HostedProviders providers, ResolvedJavaMethod method, GraphProvider.Purpose purpose) {
super(debug, method, providers, providers.getWordTypes(), providers.getGraphBuilderPlugins(), new SubstrateCompilationIdentifier(), purpose == GraphProvider.Purpose.ANALYSIS);
super(debug, method, providers, providers.getWordTypes(), providers.getGraphBuilderPlugins(), new SubstrateCompilationIdentifier(), purpose == GraphProvider.Purpose.ANALYSIS,
SubstrateCompilationDirectives.isRuntimeCompiledMethod(method));
}

@Override
Expand Down