[GR-44458] [GR-42162] Add infrastructure support for inlinebeforeanalysis in parseoncejit.

PullRequest: graal/14353

Author: teshull

compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/replacements/PEGraphDecoder.java

@@ -214,6 +214,10 @@ public boolean isInlinedMethod() {
             return caller != null;
         }
 
+        public ValueNode[] getArguments() {
+            return arguments;
+        }
+
         /**
          * Gets the call stack representing this method scope and its callers.
          */
@@ -1208,6 +1212,8 @@ protected LoopScope doInline(PEMethodScope methodScope, LoopScope loopScope, Inv
             }
         }
 
+        predecessor = afterMethodScopeCreation(inlineScope, predecessor);
+
         LoopScope inlineLoopScope = createInitialLoopScope(inlineScope, predecessor);
 
         /*
@@ -1229,6 +1235,10 @@ protected LoopScope doInline(PEMethodScope methodScope, LoopScope loopScope, Inv
         return inlineLoopScope;
     }
 
+    protected FixedWithNextNode afterMethodScopeCreation(@SuppressWarnings("unused") PEMethodScope inlineScope, FixedWithNextNode predecessor) {
+        return predecessor;
+    }
+
     @Override
     protected void afterMethodScope(MethodScope methodScope) {
         /*
@@ -1349,7 +1359,7 @@ protected void finishInlining(MethodScope is) {
         }
 
         /*
-         * Usage the handles that we have on the return value and the exception to update the
+         * Use the handles that we have on the return value and the exception to update the
          * orderId->Node table.
          */
         registerNode(loopScope, invokeData.invokeOrderId, returnValue, true, true);

compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/truffle/compiler/substitutions/TruffleGraphBuilderPlugins.java

@@ -173,8 +173,16 @@ private static void registerTruffleSafepointPlugins(InvocationPlugins plugins, K
         r.register(new RequiredInvocationPlugin("poll", com.oracle.truffle.api.nodes.Node.class) {
             @Override
             public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode arg) {
-                if (arg.isConstant()) {
-                    assert TruffleSafepointInsertionPhase.allowsSafepoints(b.getGraph()) : "TruffleSafepoint.poll only expected to be removed in Truffle compilations.";
+                if (!TruffleSafepointInsertionPhase.allowsSafepoints(b.getGraph())) {
+                    if (!canDelayIntrinsification) {
+                        /*
+                         * TruffleSafepoint.poll only expected to be removed in Truffle
+                         * compilations.
+                         */
+                        throw failPEConstant(b, arg);
+                    }
+                    return false;
+                } else if (arg.isConstant()) {
                     return true;
                 } else if (canDelayIntrinsification) {
                     return false;

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AbstractAnalysisEngine.java

@@ -37,6 +37,7 @@
 import org.graalvm.compiler.debug.Indent;
 import org.graalvm.compiler.graph.Node;
 import org.graalvm.compiler.nodes.DeoptBciSupplier;
+import org.graalvm.compiler.nodes.StateSplit;
 import org.graalvm.compiler.nodes.ValueNode;
 import org.graalvm.compiler.options.OptionValues;
 import org.graalvm.compiler.printer.GraalDebugHandlersFactory;
@@ -363,6 +364,33 @@ public static BytecodePosition syntheticSourcePosition(Node node, ResolvedJavaMe
         if (node instanceof DeoptBciSupplier) {
             bci = ((DeoptBciSupplier) node).bci();
         }
+
+        /*
+         * If the node is a state split, then we can read the framestate to get a better guess of
+         * the node's position
+         */
+        if (node instanceof StateSplit stateSplit) {
+            var frameState = stateSplit.stateAfter();
+            if (frameState != null) {
+                if (frameState.outerFrameState() != null) {
+                    /*
+                     * If the outer framestate is not null, then inlinebeforeanalysis has inlined this call. We
+                     * store the position of the original call to prevent recursive flows.
+                     */
+                    var current = frameState;
+                    while (current.outerFrameState() != null) {
+                        current = current.outerFrameState();
+                    }
+                    assert method.equals(current.getMethod());
+                    bci = current.bci;
+                } else if (bci == BytecodeFrame.UNKNOWN_BCI) {
+                    /*
+                     * If there is a single framestate, then use its bci if nothing better is available
+                     */
+                    bci = frameState.bci;
+                }
+            }
+        }
         return new BytecodePosition(null, method, bci);
     }
 

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/AnalysisPolicy.java

@@ -184,6 +184,9 @@ public abstract AbstractSpecialInvokeTypeFlow createSpecialInvokeTypeFlow(Byteco
     public abstract AbstractStaticInvokeTypeFlow createStaticInvokeTypeFlow(BytecodePosition invokeLocation, AnalysisType receiverType, PointsToAnalysisMethod targetMethod,
                     TypeFlow<?>[] actualParameters, ActualReturnTypeFlow actualReturn, MultiMethod.MultiMethodKey callerMultiMethodKey);
 
+    public abstract InvokeTypeFlow createDeoptInvokeTypeFlow(BytecodePosition invokeLocation, AnalysisType receiverType, PointsToAnalysisMethod targetMethod,
+                    TypeFlow<?>[] actualParameters, ActualReturnTypeFlow actualReturn, MultiMethod.MultiMethodKey callerMultiMethodKey);
+
     public abstract MethodFlowsGraphInfo staticRootMethodGraph(PointsToAnalysis bb, PointsToAnalysisMethod method);
 
     public abstract AnalysisContext allocationContext(PointsToAnalysis bb, MethodFlowsGraph callerGraph);

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/api/HostVM.java

@@ -34,6 +34,7 @@
 import java.util.Optional;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.BiConsumer;
+import java.util.function.Function;
 
 import org.graalvm.compiler.core.common.spi.ForeignCallDescriptor;
 import org.graalvm.compiler.core.common.spi.ForeignCallsProvider;
@@ -222,9 +223,9 @@ public boolean hasNeverInlineDirective(ResolvedJavaMethod method) {
         return true;
     }
 
-    public InlineBeforeAnalysisGraphDecoder<?> createInlineBeforeAnalysisGraphDecoder(BigBang bb, AnalysisMethod method, StructuredGraph resultGraph) {
+    public InlineBeforeAnalysisGraphDecoder createInlineBeforeAnalysisGraphDecoder(BigBang bb, AnalysisMethod method, StructuredGraph resultGraph) {
         /* No inlining by the static analysis unless explicitly overwritten by the VM. */
-        return new InlineBeforeAnalysisGraphDecoder<>(bb, InlineBeforeAnalysisPolicy.NO_INLINING, resultGraph, bb.getProviders(method), null);
+        return new InlineBeforeAnalysisGraphDecoder(bb, InlineBeforeAnalysisPolicy.NO_INLINING, resultGraph, bb.getProviders(method), null);
     }
 
     @SuppressWarnings("unused")
@@ -316,7 +317,7 @@ public interface MultiMethodAnalysisPolicy {
          *         called multiple times with the same parameters; hence, values returned by this
          *         method are allowed to be cached
          */
-        <T extends AnalysisMethod> Collection<T> determineCallees(BigBang bb, T implementation, T target, MultiMethod.MultiMethodKey callerMultiMethodKey, InvokeTypeFlow parsingReason);
+        <T extends AnalysisMethod> Collection<T> determineCallees(BigBang bb, T implementation, T target, MultiMethod.MultiMethodKey callerMultiMethodKey, InvokeTypeFlow invokeFlow);
 
         /**
          * Decides whether the caller's flows should be linked to callee's parameters flows.
@@ -346,7 +347,7 @@ public interface MultiMethodAnalysisPolicy {
     protected static final MultiMethodAnalysisPolicy DEFAULT_MULTIMETHOD_ANALYSIS_POLICY = new MultiMethodAnalysisPolicy() {
 
         @Override
-        public <T extends AnalysisMethod> Collection<T> determineCallees(BigBang bb, T implementation, T target, MultiMethod.MultiMethodKey callerMultiMethodKey, InvokeTypeFlow parsingReason) {
+        public <T extends AnalysisMethod> Collection<T> determineCallees(BigBang bb, T implementation, T target, MultiMethod.MultiMethodKey callerMultiMethodKey, InvokeTypeFlow invokeFlow) {
             return List.of(implementation);
         }
 
@@ -382,4 +383,11 @@ public boolean ignoreInstanceOfTypeDisallowed() {
     public boolean isUnknownValueField(@SuppressWarnings("unused") AnalysisField field) {
         return false;
     }
+
+    /**
+     * Returns the function Strengthen Graphs should use to improve types based on analysis results.
+     */
+    public Function<AnalysisType, ResolvedJavaType> getStrengthenGraphsToTargetFunction(@SuppressWarnings("unused") MultiMethod.MultiMethodKey key) {
+        return (t) -> t;
+    }
 }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/DirectInvokeTypeFlow.java

@@ -33,7 +33,6 @@
 import com.oracle.graal.pointsto.meta.AnalysisType;
 import com.oracle.graal.pointsto.meta.PointsToAnalysisMethod;
 import com.oracle.graal.pointsto.util.LightImmutableCollection;
-import com.oracle.svm.common.meta.MultiMethod;
 import com.oracle.svm.common.meta.MultiMethod.MultiMethodKey;
 
 import jdk.vm.ci.code.BytecodePosition;
@@ -81,7 +80,7 @@ public final Collection<AnalysisMethod> getAllCallees() {
              * callee to be set, but for it not to be linked.
              */
             Collection<AnalysisMethod> result = LightImmutableCollection.toCollection(this, CALLEES_ACCESSOR);
-            assert result.stream().filter(MultiMethod::isOriginalMethod).allMatch(AnalysisMethod::isImplementationInvoked);
+            assert result.stream().filter(m -> m.isOriginalMethod()).allMatch(AnalysisMethod::isImplementationInvoked);
             return result;
         }
         return Collections.emptyList();

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/InvokeTypeFlow.java

@@ -148,6 +148,7 @@ public TypeFlow<?> getActualReturn() {
     }
 
     public void setActualReturn(PointsToAnalysis bb, boolean isStatic, ActualReturnTypeFlow actualReturn) {
+        assert this.actualReturn == null;
         this.actualReturn = actualReturn;
         bb.analysisPolicy().linkActualReturn(bb, isStatic, this);
     }
@@ -209,13 +210,20 @@ protected void linkCallee(PointsToAnalysis bb, boolean isStatic, MethodFlowsGrap
                  * a non-state-transfer link. The link only exists for a proper iteration of type
                  * flow graphs, but the state update of 'this' parameters is achieved through direct
                  * state update in VirtualInvokeTypeFlow.update and SpecialInvokeTypeFlow.update by
-                 * calling FormalReceiverTypeFlow.addReceiverState. This happens because the formal
-                 * receiver , i.e., 'this' parameter, state must ONLY reflect those objects of the
-                 * actual receiver that generated the context for the method clone which it belongs
-                 * to. A direct link would instead transfer all the objects of compatible type from
-                 * the actual receiver to the formal receiver.
+                 * calling FormalReceiverTypeFlow.addReceiverState.
+                 * 
+                 * In other words, while the receiver param (actualParameters[0] when !isStatic) is
+                 * linked to the FormalReceiverTypeFlow of the callee, type information is not
+                 * propagated along this edge. This is accomplished by overriding the addState
+                 * method within FormalReceiverTypeFlow.
+                 *
+                 * This action is taken because the formal receiver (i.e., 'this' parameter) state
+                 * must ONLY reflect those objects of the actual receiver that generated the context
+                 * for the method clone which it belongs to. A direct link would instead transfer
+                 * all the objects of compatible type from the actual receiver to the formal
+                 * receiver.
                  */
-                if (actualParam != null && formalParam != null /* && (i != 0 || isStatic) */) {
+                if (actualParam != null && formalParam != null) {
                     // create the use link:
                     // (formalParam, callerContext) -> (actualParam, calleeContext)
                     // Note: the callerContext is an implicit property of the current InvokeTypeFlow
@@ -237,38 +245,26 @@ protected void linkCallee(PointsToAnalysis bb, boolean isStatic, MethodFlowsGrap
     }
 
     public void linkReturn(PointsToAnalysis bb, boolean isStatic, MethodFlowsGraphInfo calleeFlows) {
-        if (bb.getHostVM().getMultiMethodAnalysisPolicy().performReturnLinking(callerMultiMethodKey, calleeFlows.getMethod().getMultiMethodKey())) {
-            if (actualReturn != null) {
-                if (bb.optimizeReturnedParameter()) {
-                    int paramNodeIndex = calleeFlows.getMethod().getTypeFlow().getReturnedParameterIndex();
-                    if (paramNodeIndex != -1) {
-                        if (isStatic || paramNodeIndex != 0) {
-                            TypeFlow<?> actualParam = actualParameters[paramNodeIndex];
-                            actualParam.addUse(bb, actualReturn);
-                        } else {
-                            /*
-                             * The callee returns `this`. The formal-receiver state is updated in
-                             * InvokeTypeFlow#updateReceiver() for each linked callee and every time
-                             * the formal-receiver is updated then the same update state is
-                             * propagated to the actual-return. One may think that we could simply
-                             * add a direct use link from the formal-receiver in the callee to the
-                             * actual-return in the caller to get the state propagation
-                             * automatically. But that would be wrong because then the actual-return
-                             * would get the state from *all* the other places that callee may be
-                             * called from, and that would defeat the purpose of this optimization:
-                             * we want just the receiver state from the caller of current invoke to
-                             * reach the actual-return.
-                             */
-                        }
+        if (actualReturn != null && bb.getHostVM().getMultiMethodAnalysisPolicy().performReturnLinking(callerMultiMethodKey, calleeFlows.getMethod().getMultiMethodKey())) {
+            if (bb.optimizeReturnedParameter()) {
+                int paramNodeIndex = calleeFlows.getMethod().getTypeFlow().getReturnedParameterIndex();
+                if (paramNodeIndex != -1) {
+                    if (isStatic || paramNodeIndex != 0) {
+                        TypeFlow<?> actualParam = actualParameters[paramNodeIndex];
+                        actualParam.addUse(bb, actualReturn);
                     } else {
                         /*
-                         * The callee may have a return type, hence the actualReturn is non-null,
-                         * but it might throw an exception instead of returning, hence the formal
-                         * return is null.
+                         * The callee returns `this`. The formal-receiver state is updated in
+                         * InvokeTypeFlow#updateReceiver() for each linked callee and every time the
+                         * formal-receiver is updated then the same update state is propagated to
+                         * the actual-return. One may think that we could simply add a direct use
+                         * link from the formal-receiver in the callee to the actual-return in the
+                         * caller to get the state propagation automatically. But that would be
+                         * wrong because then the actual-return would get the state from *all* the
+                         * other places that callee may be called from, and that would defeat the
+                         * purpose of this optimization: we want just the receiver state from the
+                         * caller of current invoke to reach the actual-return.
                          */
-                        if (calleeFlows.getReturnFlow() != null) {
-                            calleeFlows.getReturnFlow().addUse(bb, actualReturn);
-                        }
                     }
                 } else {
                     /*
@@ -280,6 +276,14 @@ public void linkReturn(PointsToAnalysis bb, boolean isStatic, MethodFlowsGraphIn
                         calleeFlows.getReturnFlow().addUse(bb, actualReturn);
                     }
                 }
+            } else {
+                /*
+                 * The callee may have a return type, hence the actualReturn is non-null, but it
+                 * might throw an exception instead of returning, hence the formal return is null.
+                 */
+                if (calleeFlows.getReturnFlow() != null) {
+                    calleeFlows.getReturnFlow().addUse(bb, actualReturn);
+                }
             }
         }
     }