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

teshull · teshull · commit ac25ff118863 · 2023-08-10T02:51:16.000Z
PullRequest: graal/15097
diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/GraphKit.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/GraphKit.java
@@ -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();
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java
@@ -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;
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java
@@ -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);
@@ -960,6 +961,10 @@ public void setAnalyzedGraph(EncodedGraph analyzedGraph) {
         this.analyzedGraph = analyzedGraph;
     }
 
+    public EncodedGraph getAnalyzedGraph() {
+        return analyzedGraph;
+    }
+
     @Override
     public MultiMethodKey getMultiMethodKey() {
         return multiMethodKey;
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysis.java
@@ -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();
 
diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysisGraphDecoder.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/phases/InlineBeforeAnalysisGraphDecoder.java
@@ -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;
@@ -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);
@@ -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);
         }
     }
 
@@ -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) {
@@ -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);
@@ -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;
diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java
@@ -107,8 +107,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();
@@ -117,6 +117,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);
diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceDeoptTestFeature.java
@@ -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. */
diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java
@@ -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;
@@ -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
@@ -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
@@ -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);
             }
         }
 
@@ -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
@@ -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);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java
@@ -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) {
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/SVMParsingSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/analysis/SVMParsingSupport.java
@@ -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);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/HostedGraphKit.java
@@ -59,6 +59,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;
@@ -69,7 +70,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

Original file line number	Diff line number	Diff line change
`@@ -119,6 +119,7 @@ public GraphKit(DebugContext debug, ResolvedJavaMethod stubMethod, Providers pro`
`119`	`119`	`// Set up a default value that everything constructed by GraphKit will use.`
`120`	`120`	`graph.withNodeSourcePosition(NodeSourcePosition.substitution(stubMethod));`
`121`	`121`	`}`
	`122`	`+ graph.recordMethod(stubMethod);`
`122`	`123`	`this.wordTypes = wordTypes;`
`123`	`124`	`this.graphBuilderPlugins = graphBuilderPlugins;`
`124`	`125`	`this.lastFixedNode = graph.start();`