diff --git a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/java/BciBlockMapping.java b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/java/BciBlockMapping.java index fd19bc6f6551..b86f2eb9595f 100644 --- a/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/java/BciBlockMapping.java +++ b/compiler/src/jdk.internal.vm.compiler/src/org/graalvm/compiler/java/BciBlockMapping.java @@ -624,6 +624,12 @@ public boolean isOutOfBounds() { } } + /** + * Represents an entry into an exception handler routine. This block is needed to ensure there + * is a place to put checks that determine whether the handler should be entered (i.e., the + * exception kind is compatible) or execution should continue to check the next handler (or + * unwind). + */ public static class ExceptionDispatchBlock extends BciBlock { public final ExceptionHandler handler; public final int deoptBci; @@ -1351,7 +1357,7 @@ private void addSuccessor(int predBci, BciBlock sux) { } /** - * Logic for adding an the "normal" invoke successor link. + * Logic for adding the "normal" invoke successor link. */ protected void addInvokeNormalSuccessor(int invokeBci, BciBlock sux) { addSuccessor(invokeBci, sux); diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java index 061c57751930..1100614a3233 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java @@ -487,7 +487,9 @@ public void run(DebugContext ignored) { PointsToStats.registerTypeFlowQueuedUpdate(PointsToAnalysis.this, operation); operation.inQueue = false; - operation.update(PointsToAnalysis.this); + if (operation.isValid()) { + operation.update(PointsToAnalysis.this); + } } @Override diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodFlowsGraph.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodFlowsGraph.java index a371d36e792e..a7757d4a36d8 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodFlowsGraph.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/MethodFlowsGraph.java @@ -468,6 +468,15 @@ public String toString() { */ final void removeInternalFlows(PointsToAnalysis bb) { + // Invalidate internal flows which will be cleared + flowsIterator().forEachRemaining(typeFlow -> { + // param and return flows will not be cleared + boolean skipInvalidate = typeFlow instanceof FormalParamTypeFlow || typeFlow instanceof FormalReturnTypeFlow; + if (!skipInvalidate) { + typeFlow.invalidate(); + } + }); + // Clear out the parameter uses and observers for (var param : getParameters()) { if (param != null) { diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java index d990ee21ee3c..fe41f3c9ae70 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/TypeFlow.java @@ -104,6 +104,14 @@ public abstract class TypeFlow { */ private volatile boolean isSaturated; + /** + * A TypeFlow is invalidated when the flowsgraph it belongs to is updated due to + * {@link MethodTypeFlow#updateFlowsGraph}. Once a flow is invalided it no longer needs to be + * updated and its links can be removed. Note delaying the removal of invalid flows does not + * affect correctness, so they can be removed lazily. + */ + private boolean isValid = true; + @SuppressWarnings("rawtypes")// private static final AtomicReferenceFieldUpdater STATE_UPDATER = AtomicReferenceFieldUpdater.newUpdater(TypeFlow.class, TypeState.class, "state"); @@ -258,6 +266,20 @@ public int getSlot() { return this.slot; } + /** + * Return true is the flow is valid and should be updated. + */ + public boolean isValid() { + return isValid; + } + + /** + * Invalidating the typeflow will cause the flow to be lazily removed in the future. + */ + public void invalidate() { + isValid = false; + } + /** * Return true if this flow is saturated. When an observer becomes saturated it doesn't * immediately remove itself from all its inputs. The inputs lazily remove it on next update. @@ -392,6 +414,9 @@ protected boolean doAddUse(PointsToAnalysis bb, TypeFlow use) { if (use.equals(this)) { return false; } + if (!use.isValid()) { + return false; + } /* Input is always tracked. */ registerInput(bb, use); if (use.isSaturated()) { @@ -473,6 +498,10 @@ private boolean doAddObserver(PointsToAnalysis bb, TypeFlow observer) { if (observer.equals(this)) { return false; } + if (!observer.isValid()) { + return false; + } + registerObservee(bb, observer); return ConcurrentLightHashSet.addElement(this, OBSERVERS_UPDATER, observer); } @@ -579,7 +608,7 @@ public static AnalysisType filterUncheckedInterface(AnalysisType type) { public void update(PointsToAnalysis bb) { TypeState curState = getState(); for (TypeFlow use : getUses()) { - if (use.isSaturated()) { + if (!use.isValid() || use.isSaturated()) { removeUse(use); } else { use.addState(bb, curState); @@ -587,7 +616,11 @@ public void update(PointsToAnalysis bb) { } for (TypeFlow observer : getObservers()) { - observer.onObservedUpdate(bb); + if (observer.isValid()) { + observer.onObservedUpdate(bb); + } else { + removeObserver(observer); + } } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntryNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntryNode.java index 28a68807da34..b7abcf7d8664 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntryNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntryNode.java @@ -43,6 +43,8 @@ import com.oracle.svm.core.graal.lir.DeoptEntryOp; +import jdk.vm.ci.code.BytecodeFrame; + /** * A landing-pad for deoptimization. This node is generated in deoptimization target methods for all * deoptimization entry points. @@ -53,8 +55,20 @@ public final class DeoptEntryNode extends WithExceptionNode implements DeoptEntr @OptionalInput(InputType.State) protected FrameState stateAfter; - public DeoptEntryNode() { + private final int proxifiedInvokeBci; + + protected DeoptEntryNode(int proxifiedInvokeBci) { super(TYPE, StampFactory.forVoid()); + this.proxifiedInvokeBci = proxifiedInvokeBci; + } + + public static DeoptEntryNode create(int proxifiedInvokeBci) { + assert proxifiedInvokeBci != BytecodeFrame.UNKNOWN_BCI; + return new DeoptEntryNode(proxifiedInvokeBci); + } + + public static DeoptEntryNode create() { + return new DeoptEntryNode(BytecodeFrame.UNKNOWN_BCI); } @Override @@ -99,4 +113,9 @@ public void setStateAfter(FrameState x) { public boolean hasSideEffect() { return true; } + + @Override + public int getProxifiedInvokeBci() { + return proxifiedInvokeBci; + } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntrySupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntrySupport.java index 0055cae3c30e..c8e55daa4a2d 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntrySupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntrySupport.java @@ -30,4 +30,12 @@ import org.graalvm.compiler.nodes.spi.LIRLowerable; public interface DeoptEntrySupport extends LIRLowerable, ControlFlowAnchored, FixedNodeInterface, StateSplit { + + /** + * Returns the invoke bci this deopt is serving as a proxy anchor for or + * {@link jdk.vm.ci.code.BytecodeFrame#UNKNOWN_BCI} if it is not serving as a proxy anchor for + * an invoke. Note this will be different than the entry's stateafter bci since this node will + * be after the invoke it is "proxifying". + */ + int getProxifiedInvokeBci(); } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptProxyAnchorNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptProxyAnchorNode.java index 789f3854eb5b..57ade97e07fd 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptProxyAnchorNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptProxyAnchorNode.java @@ -41,16 +41,22 @@ public class DeoptProxyAnchorNode extends AbstractStateSplit implements DeoptEntrySupport { public static final NodeClass TYPE = NodeClass.create(DeoptProxyAnchorNode.class); - public DeoptProxyAnchorNode() { - this(TYPE); - } + private final int proxifiedInvokeBci; + + public DeoptProxyAnchorNode(int proxifiedInvokeBci) { + super(TYPE, StampFactory.forVoid()); + assert proxifiedInvokeBci >= 0 : "DeoptProxyAnchorNode should be proxing an invoke"; - protected DeoptProxyAnchorNode(NodeClass c) { - super(c, StampFactory.forVoid()); + this.proxifiedInvokeBci = proxifiedInvokeBci; } @Override public void generate(NodeLIRBuilderTool gen) { /* No-op */ } + + @Override + public int getProxifiedInvokeBci() { + return proxifiedInvokeBci; + } } 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 index a6d24213dbdc..5a9473748364 100644 --- 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 @@ -249,7 +249,7 @@ public ValueNode createCFunctionCallWithCapture(ValueNode targetAddress, List RemoveUnneededDeoptSupport = new HostedOptionKey<>(false); + public static final HostedOptionKey RemoveUnneededDeoptSupport = new HostedOptionKey<>(true); @Option(help = "Perform InlineBeforeAnalysis on runtime compiled methods")// public static final HostedOptionKey RuntimeCompilationInlineBeforeAnalysis = new HostedOptionKey<>(true); @@ -1226,58 +1226,103 @@ public boolean insertPlaceholderParamAndReturnFlows(MultiMethod.MultiMethodKey m } /** - * Removes Deoptimizations Entrypoints which are deemed to be unnecessary after the runtime - * compilation methods are optimized. + * Removes {@link DeoptEntryNode}s, {@link DeoptProxyAnchorNode}s, and {@link DeoptProxyNode}s + * which are determined to be unnecessary after the runtime compilation methods are optimized. */ static class RemoveUnneededDeoptSupport extends Phase { + enum RemovalDecision { + KEEP, + PROXIFY, + REMOVE + } @Override protected void run(StructuredGraph graph) { - EconomicMap decisionCache = EconomicMap.create(); + EconomicMap decisionCache = EconomicMap.create(); // First go through and delete all unneeded proxies for (DeoptProxyNode proxyNode : graph.getNodes(DeoptProxyNode.TYPE).snapshot()) { ValueNode proxyPoint = proxyNode.getProxyPoint(); if (proxyPoint instanceof StateSplit) { - if (proxyPoint instanceof DeoptEntryNode && shouldRemove((StateSplit) proxyPoint, decisionCache)) { + if (getDecision((StateSplit) proxyPoint, decisionCache) == RemovalDecision.REMOVE) { proxyNode.replaceAtAllUsages(proxyNode.getOriginalNode(), true); proxyNode.safeDelete(); } } } - // Next remove all unneeded DeoptEntryNodes + // Next, remove all unneeded DeoptEntryNodes for (DeoptEntryNode deoptEntry : graph.getNodes().filter(DeoptEntryNode.class).snapshot()) { - if (shouldRemove(deoptEntry, decisionCache)) { - deoptEntry.killExceptionEdge(); - graph.removeSplit(deoptEntry, deoptEntry.getPrimarySuccessor()); + switch (getDecision(deoptEntry, decisionCache)) { + case REMOVE -> { + deoptEntry.killExceptionEdge(); + graph.removeSplit(deoptEntry, deoptEntry.getPrimarySuccessor()); + } + case PROXIFY -> { + deoptEntry.killExceptionEdge(); + DeoptProxyAnchorNode newAnchor = graph.add(new DeoptProxyAnchorNode(deoptEntry.getProxifiedInvokeBci())); + newAnchor.setStateAfter(deoptEntry.stateAfter()); + graph.replaceSplitWithFixed(deoptEntry, newAnchor, deoptEntry.getPrimarySuccessor()); + } + } + } + + // Finally, remove all unneeded DeoptProxyAnchorNodes + for (DeoptProxyAnchorNode proxyAnchor : graph.getNodes().filter(DeoptProxyAnchorNode.class).snapshot()) { + if (getDecision(proxyAnchor, decisionCache) == RemovalDecision.REMOVE) { + graph.removeFixed(proxyAnchor); } } } - boolean shouldRemove(StateSplit node, EconomicMap decisionCache) { - Boolean cached = decisionCache.get(node); + RemovalDecision getDecision(StateSplit node, EconomicMap decisionCache) { + RemovalDecision cached = decisionCache.get(node); if (cached != null) { return cached; } + DeoptEntrySupport proxyNode; + if (node instanceof ExceptionObjectNode exceptionObject) { + /* + * For the exception edge of a DeoptEntryNode, we insert the proxies on the + * exception object. + */ + proxyNode = (DeoptEntrySupport) exceptionObject.predecessor(); + } else { + proxyNode = (DeoptEntrySupport) node; + } + + RemovalDecision decision = RemovalDecision.REMOVE; var directive = SubstrateCompilationDirectives.singleton(); - FrameState state = node.stateAfter(); + FrameState state = proxyNode.stateAfter(); HostedMethod method = (HostedMethod) state.getMethod(); + if (proxyNode instanceof DeoptEntryNode) { + if (directive.isDeoptEntry(method, state.bci, state.duringCall(), state.rethrowException())) { + // must keep all deopt entries which are still guarding nodes + decision = RemovalDecision.KEEP; + } + } - boolean result = true; - if (directive.isRegisteredDeoptTarget(method)) { - result = !directive.isDeoptEntry(method, state.bci, state.duringCall(), state.rethrowException()); + if (decision == RemovalDecision.REMOVE) { + // now check for any implicit deopt entry being protected against + int proxifiedInvokeBci = proxyNode.getProxifiedInvokeBci(); + if (proxifiedInvokeBci != BytecodeFrame.UNKNOWN_BCI && directive.isDeoptEntry(method, proxifiedInvokeBci, true, false)) { + // must keep still keep a proxy for nodes which are "proxifying" an invoke + decision = proxyNode instanceof DeoptEntryNode ? RemovalDecision.PROXIFY : RemovalDecision.KEEP; + } } // cache the decision - decisionCache.put(node, result); - return result; + decisionCache.put(node, decision); + if (proxyNode != node) { + decisionCache.put(proxyNode, decision); + } + return decision; } @Override public CharSequence getName() { - return "RemoveDeoptEntries"; + return "RemoveUnneededDeoptSupport"; } } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java index 67bfba4ea2e7..a60f30eb92ec 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java @@ -174,7 +174,7 @@ private static int startMethod(HostedGraphKit kit, DeoptInfoProvider deoptInfo, if (deoptInfo != null) { FrameState initialState = kit.getGraph().start().stateAfter(); if (shouldInsertDeoptEntry(deoptInfo, initialState.bci, false, false)) { - return appendDeoptWithExceptionUnwind(kit, initialState, initialState.bci, nextDeoptIndex); + return appendDeoptWithExceptionUnwind(kit, initialState, initialState.bci, nextDeoptIndex, DeoptEntryNode.create()); } } return nextDeoptIndex; @@ -223,7 +223,7 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn if (shouldInsertDeoptEntry(deoptInfo, bci, false, true)) { // Exception during invoke - var exceptionDeopt = kit.add(new DeoptEntryNode()); + var exceptionDeopt = kit.add(DeoptEntryNode.create(invoke.bci())); exceptionDeopt.setStateAfter(exception.stateAfter().duplicate()); var exceptionDeoptBegin = kit.add(new DeoptEntryBeginNode()); int exceptionDeoptIndex = nextDeoptIndex++; @@ -246,10 +246,10 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn kit.noExceptionPart(); if (needDeoptEntry) { // Deopt entry after invoke without exception - nextDeoptIndex = appendDeoptWithExceptionUnwind(kit, invoke.stateAfter(), invoke.stateAfter().bci, nextDeoptIndex); + nextDeoptIndex = appendDeoptWithExceptionUnwind(kit, invoke.stateAfter(), invoke.stateAfter().bci, nextDeoptIndex, DeoptEntryNode.create(invoke.bci())); } else { // Only a proxy is needed - nextDeoptIndex = appendDeoptProxyAnchorNode(kit, invoke.stateAfter(), nextDeoptIndex); + nextDeoptIndex = appendDeoptProxyAnchorNode(kit, invoke.stateAfter(), nextDeoptIndex, invoke.bci()); } } kit.endInvokeWithException(); @@ -258,8 +258,8 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn } /** @see com.oracle.svm.hosted.phases.SharedGraphBuilderPhase */ - private static int appendDeoptWithExceptionUnwind(HostedGraphKit kit, FrameState state, int exceptionBci, int nextDeoptIndex) { - var entry = kit.add(new DeoptEntryNode()); + private static int appendDeoptWithExceptionUnwind(HostedGraphKit kit, FrameState state, int exceptionBci, int nextDeoptIndex, DeoptEntryNode deoptEntryNode) { + var entry = kit.add(deoptEntryNode); entry.setStateAfter(state.duplicate()); var begin = kit.append(new DeoptEntryBeginNode()); ((FixedWithNextNode) begin.predecessor()).setNext(entry); @@ -280,8 +280,8 @@ private static int appendDeoptWithExceptionUnwind(HostedGraphKit kit, FrameState } /** @see com.oracle.svm.hosted.phases.SharedGraphBuilderPhase */ - private static int appendDeoptProxyAnchorNode(HostedGraphKit kit, FrameState state, int nextDeoptIndex) { - var anchor = kit.append(new DeoptProxyAnchorNode()); + private static int appendDeoptProxyAnchorNode(HostedGraphKit kit, FrameState state, int nextDeoptIndex, int invokeBci) { + var anchor = kit.append(new DeoptProxyAnchorNode(invokeBci)); anchor.setStateAfter(state.duplicate()); // Ensure later nodes see values from potential deoptimization diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/DeoptimizationTargetBciBlockMapping.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/DeoptimizationTargetBciBlockMapping.java index 5659c45b89bf..4c1aa64c7329 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/DeoptimizationTargetBciBlockMapping.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/DeoptimizationTargetBciBlockMapping.java @@ -37,14 +37,40 @@ import com.oracle.svm.common.meta.MultiMethod; import com.oracle.svm.core.code.FrameInfoEncoder; +import com.oracle.svm.core.graal.nodes.DeoptEntryNode; +import com.oracle.svm.core.graal.nodes.DeoptProxyAnchorNode; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.code.SubstrateCompilationDirectives; +import jdk.vm.ci.code.BytecodeFrame; import jdk.vm.ci.meta.ResolvedJavaMethod; /** * To guarantee DeoptEntryNodes and DeoptProxyNodes are inserted at the correct positions, the bci * block mapping creation must be augmented to identify these insertion points. + * + * Deoptimizations can occur at three different places: + *
    + *
  1. At the start of any bci (!duringCall && !rethrowException).
    2. + *
  2. During a call (duringCall && !rethrowException).
    3. + *
  3. Exceptional flow out of a bci (!duringCall && rethrowException)
    4. + *
+ * + * For case 1, a {@link DeoptEntryNode} must be inserted before this bci and also an exception flow + * control must be modeled. + * + * For case 2, a {@link DeoptProxyAnchorNode} must be inserted in both the normal and exception + * successor to guarantee the callsite cannot be optimized around. However, if a + * {@link DeoptEntryNode} within the given path immediately follows the call, then it is not + * necessary to insert a {@link DeoptProxyAnchorNode}, as the optimization is already prevented. + * + * For case 3, a {@link DeoptEntryNode} must be inserted within the exceptional control path before + * the logic determining which handler should be entered. + * + * Since deoptimizations can throw exceptions, at each "normal" (i.e., case 1) DeoptEntryNode + * insertion point we also must model its exceptional control flow path. It is not possible for a + * DeoptEntryNode for case 3 to throw an exception, as it is in a transient state which does + * correspond to a position within the Java code. */ final class DeoptimizationTargetBciBlockMapping extends BciBlockMapping { @@ -66,12 +92,13 @@ private DeoptimizationTargetBciBlockMapping(Bytecode code, DebugContext debug) { */ interface DeoptEntryInsertionPoint { - /* The deopt entry position this block represents. */ - int deoptEntryBci(); - - boolean duringCall(); + /* + * The bci of the invoke this insertion point is serving as a proxy anchor for, or + * BytecodeFrame.UNKNOWN_BCI if it is not "proxifying" an invoke. + */ + int proxifiedInvokeBci(); - boolean rethrowException(); + boolean isExceptionDispatch(); /* * The bci for the stateAfter value for the node to be inserted at this block. Note that for @@ -79,10 +106,20 @@ interface DeoptEntryInsertionPoint { */ int frameStateBci(); - /* Whether this block represents a DeoptEntryNode or DeoptProxyAnchorNode. */ - boolean isProxy(); - BciBlock asBlock(); + + /** + * Whether this insertion point is protecting an invoke. + */ + default boolean proxifysInvoke() { + assert proxifiedInvokeBci() >= 0 || proxifiedInvokeBci() == BytecodeFrame.UNKNOWN_BCI; + return proxifiedInvokeBci() != BytecodeFrame.UNKNOWN_BCI; + } + + /** + * Whether this is a proxy ({@link DeoptProxyAnchorNode}) or a full {@link DeoptEntryNode}. + */ + boolean isProxy(); } /** @@ -90,20 +127,23 @@ interface DeoptEntryInsertionPoint { */ static final class DeoptBciBlock extends BciBlock implements DeoptEntryInsertionPoint { public final int deoptEntryBci; - public final boolean isProxy; - private DeoptBciBlock(int startBci, int deoptEntryBci, boolean isProxy) { + public int proxifiedInvokeBci; + + private DeoptBciBlock(int startBci, int deoptEntryBci, int proxifiedInvokeBci) { super(startBci, startBci); this.deoptEntryBci = deoptEntryBci; - this.isProxy = isProxy; + this.proxifiedInvokeBci = proxifiedInvokeBci; } static DeoptBciBlock createDeoptEntry(int bci) { - return new DeoptBciBlock(bci, bci, false); + assert bci >= 0; + return new DeoptBciBlock(bci, bci, BytecodeFrame.UNKNOWN_BCI); } - static DeoptBciBlock createDeoptProxy(int successorBci, int deoptBci) { - return new DeoptBciBlock(successorBci, deoptBci, true); + static DeoptBciBlock createDeoptProxy(int successorBci, int proxifiedInvokeBci) { + assert proxifiedInvokeBci >= 0; + return new DeoptBciBlock(successorBci, BytecodeFrame.UNKNOWN_BCI, proxifiedInvokeBci); } @Override @@ -117,31 +157,29 @@ public boolean isInstructionBlock() { } @Override - public int deoptEntryBci() { - return deoptEntryBci; + public int proxifiedInvokeBci() { + return proxifiedInvokeBci; } - @Override - public int frameStateBci() { - return getStartBci(); + public void setProxifiedInvokeBci(int bci) { + assert proxifiedInvokeBci == BytecodeFrame.UNKNOWN_BCI; + this.proxifiedInvokeBci = bci; } - /* - * Proxies correspond to the frameState (duringCall && !rethrowException) - */ @Override - public boolean duringCall() { - return isProxy; + public boolean isProxy() { + assert deoptEntryBci >= 0 || deoptEntryBci == BytecodeFrame.UNKNOWN_BCI; + return deoptEntryBci == BytecodeFrame.UNKNOWN_BCI; } @Override - public boolean rethrowException() { - return false; + public int frameStateBci() { + return getStartBci(); } @Override - public boolean isProxy() { - return isProxy; + public boolean isExceptionDispatch() { + return false; } @Override @@ -159,51 +197,45 @@ public String toString() { * Represents a DeoptEntryInsertionPoint whose successor is an ExceptionDispatchBlock. */ static class DeoptExceptionDispatchBlock extends ExceptionDispatchBlock implements DeoptEntryInsertionPoint { - public final boolean isProxy; - DeoptExceptionDispatchBlock(int bci, boolean isProxy) { + final boolean isDeoptEntry; + final boolean isInvokeProxy; + + DeoptExceptionDispatchBlock(int bci, boolean isDeoptEntry, boolean isInvokeProxy) { super(bci); - this.isProxy = isProxy; + this.isDeoptEntry = isDeoptEntry; + this.isInvokeProxy = isInvokeProxy; } - DeoptExceptionDispatchBlock(ExceptionDispatchBlock dispatch, int bci, boolean isProxy) { + DeoptExceptionDispatchBlock(ExceptionDispatchBlock dispatch, int bci, boolean isDeoptEntry, boolean isInvokeProxy) { super(dispatch.handler, bci); - this.isProxy = isProxy; + this.isDeoptEntry = isDeoptEntry; + this.isInvokeProxy = isInvokeProxy; } /* - * For DeoptExceptionDispatchBlocks the deoptEntryBci and frameStateBci are the same. + * For DeoptExceptionDispatchBlocks the deoptEntryBci, invokeBci, and frameStateBci are the + * same (when present). */ - @Override - public int deoptEntryBci() { - return deoptBci; - } @Override - public int frameStateBci() { - return deoptBci; + public int proxifiedInvokeBci() { + return isInvokeProxy ? deoptBci : BytecodeFrame.UNKNOWN_BCI; } - /* - * Proxies correspond to the frameState (duringCall && !rethrowException) - */ @Override - public boolean duringCall() { - return isProxy; + public boolean isProxy() { + return !isDeoptEntry; } - /* - * If not a proxy, then this block should correspond to the point after an exception object - * is added, but before it is dispatched. - */ @Override - public boolean rethrowException() { - return !isProxy; + public int frameStateBci() { + return deoptBci; } @Override - public boolean isProxy() { - return isProxy; + public boolean isExceptionDispatch() { + return true; } @Override @@ -293,16 +325,23 @@ protected void addInvokeNormalSuccessor(int invokeBci, BciBlock sux) { /* * Because this invoke has an implicit deopt entry, it is necessary to insert proxies * afterwards. - * - * If a DeoptEntryPoint is next, then no additional action needs to be taken, as it - * inserts the needed proxy inputs. Otherwise, a DeoptProxy must be inserted. */ if (!(sux instanceof DeoptBciBlock)) { + /* + * A DeoptProxy must be inserted to guarantee values will be guarded. + */ DeoptBciBlock proxyBlock = DeoptBciBlock.createDeoptProxy(sux.getStartBci(), invokeBci); recordInsertedBlock(proxyBlock); getInstructionBlock(invokeBci).addSuccessor(proxyBlock); proxyBlock.addSuccessor(sux); return; + } else { + /* + * If a DeoptEntryPoint is next, then no additional action needs to be taken, as it + * inserts the needed proxy inputs. We mark this DeoptEntryPoint as also guarding an + * invoke in case we can remove it later. + */ + ((DeoptBciBlock) sux).setProxifiedInvokeBci(invokeBci); } } @@ -311,7 +350,7 @@ protected void addInvokeNormalSuccessor(int invokeBci, BciBlock sux) { /** * If passed DeoptBciBlock is covered by an exception handler, then the proper - * ExceptionDispatchBlock is added as an successor to the block. + * ExceptionDispatchBlock is added as a successor to the block. */ private void addExceptionHandlerEdge(DeoptBciBlock block) { /* Checking whether this deopt is covered by an exception handler. */ @@ -387,7 +426,7 @@ protected BciBlock processNewBciBlock(int bci, BciBlock newBlock) { } /** - * A deopt entry node is must be inserted after an exception object is created if either: + * A deopt entry node must be inserted after an exception object is created if either: *
    *
  • This point is an explicit deopt entry.
    • *
  • This point follows an invoke with an implicit deopt entry. In this situation only a deopt @@ -396,18 +435,18 @@ protected BciBlock processNewBciBlock(int bci, BciBlock newBlock) { */ @Override protected ExceptionDispatchBlock processNewExceptionDispatchBlock(int bci, boolean isInvoke, ExceptionDispatchBlock handler) { - boolean isExplictDeoptEntry = isDeoptEntry(bci, false, true); - if (isExplictDeoptEntry || (isInvoke && isDeoptEntry(bci, true, false))) { - boolean isProxy = !isExplictDeoptEntry; + boolean isDeoptEntry = isDeoptEntry(bci, false, true); + boolean isInvokeProxy = isInvoke && isDeoptEntry(bci, true, false); + if (isDeoptEntry || isInvokeProxy) { DeoptExceptionDispatchBlock block; if (handler == null) { /* * This means that this exception will be dispatched to the unwind block. In this - * case it is still necessary to create deopt entry node. + * case it is still necessary to create a deopt entry node. */ - block = new DeoptExceptionDispatchBlock(bci, isProxy); + block = new DeoptExceptionDispatchBlock(bci, isDeoptEntry, isInvokeProxy); } else { - block = new DeoptExceptionDispatchBlock(handler, bci, isProxy); + block = new DeoptExceptionDispatchBlock(handler, bci, isDeoptEntry, isInvokeProxy); block.addSuccessor(handler); } recordInsertedBlock(block); @@ -421,8 +460,8 @@ protected ExceptionDispatchBlock processNewExceptionDispatchBlock(int bci, boole * *
      *
    • Be on a reachable path (i.e. have a block id >= 0).
      • - *
    • Correspond to a deoptimization entrypoint recorded within method.compilationInfo.
      • - *
    • DeoptProxies can only represent implicit deoptimizations.
      • + *
    • Correspond to a recorded deoptimization entrypoint.
      • + *
    • DeoptProxies must protected an invoke.
      • *
    • Have only one non-proxy DeoptEntryInsertionPoint for each encoded BCI.
      • *
    • Have at most 2 successors.
      • *
    • The successors should not be DeoptEntryInsertionPoints.
      • @@ -434,24 +473,44 @@ protected boolean verify() { Set coveredEncodedBcis = new HashSet<>(); for (DeoptEntryInsertionPoint deopt : insertedBlocks) { BciBlock block = deopt.asBlock(); - int bci = deopt.deoptEntryBci(); - boolean duringCall = deopt.duringCall(); - boolean rethrowException = deopt.rethrowException(); + int bci = deopt.frameStateBci(); + boolean isExceptionDispatch = deopt.isExceptionDispatch(); + boolean isProxy = deopt.isProxy(); + boolean proxifysInvoke = deopt.proxifysInvoke(); + boolean coversInvoke = false; + boolean coversDeoptEntry = false; + + if (deopt.proxifysInvoke()) { + int proxifiedInvokeBci = deopt.proxifiedInvokeBci(); + assert proxifiedInvokeBci >= 0; + coversInvoke = isRegisteredDeoptEntry(proxifiedInvokeBci, true, false); + } + if (!isProxy) { + coversDeoptEntry = isRegisteredDeoptEntry(bci, false, isExceptionDispatch); + } + if (block.getId() < 0) { /* * When using -H:+DeoptimizeAll DeoptInsertionPoints can be within unreachable * paths. */ - assert !isRegisteredDeoptEntry(bci, duringCall, rethrowException); + assert !coversInvoke && !coversDeoptEntry; /* Other information about this block is irrelevant as it is unreachable. */ continue; } - assert isDeoptEntry(bci, duringCall, rethrowException); - assert deopt.isProxy() == duringCall : "deopt proxy nodes always represent implicit deopt entries from invokes."; - if (!deopt.isProxy()) { - assert coveredEncodedBcis.add(FrameInfoEncoder.encodeBci(bci, duringCall, rethrowException)) : "Deoptimization entry points must be unique."; + + if (proxifysInvoke) { + assert coversInvoke; } + + if (isProxy) { + assert proxifysInvoke; + } else { + assert coversDeoptEntry; + assert coveredEncodedBcis.add(FrameInfoEncoder.encodeBci(bci, false, isExceptionDispatch)) : "Deoptimization entry points must be unique."; + } + assert block.getSuccessorCount() <= 2 : "DeoptEntryInsertionPoint must have at most 2 successors"; for (BciBlock sux : block.getSuccessors()) { assert !(sux instanceof DeoptEntryInsertionPoint) : "Successor of DeoptEntryInsertionPoint should not be a DeoptEntryInsertionPoint."; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java index 4c1150d9b44b..b706a79d57fe 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SharedGraphBuilderPhase.java @@ -754,17 +754,24 @@ private void insertDeoptNode(DeoptimizationTargetBciBlockMapping.DeoptEntryInser assert frameState.rethrowException(); } - DeoptEntrySupport deoptNode = graph.add(deopt.isProxy() ? new DeoptProxyAnchorNode() : new DeoptEntryNode()); + int proxifiedInvokeBci = deopt.proxifiedInvokeBci(); + boolean isProxy = deopt.isProxy(); + DeoptEntrySupport deoptNode; + if (isProxy) { + deoptNode = graph.add(new DeoptProxyAnchorNode(proxifiedInvokeBci)); + } else { + boolean proxifysInvoke = deopt.proxifysInvoke(); + deoptNode = graph.add(proxifysInvoke ? DeoptEntryNode.create(proxifiedInvokeBci) : DeoptEntryNode.create()); + } FrameState stateAfter = frameState.create(deopt.frameStateBci(), deoptNode); deoptNode.setStateAfter(stateAfter); if (lastInstr != null) { lastInstr.setNext(deoptNode.asFixedNode()); } - if (deopt.isProxy()) { + if (isProxy) { lastInstr = (DeoptProxyAnchorNode) deoptNode; } else { - assert !deopt.duringCall() : "Implicit deopt entries from invokes cannot have explicit deopt entries."; DeoptEntryNode deoptEntryNode = (DeoptEntryNode) deoptNode; deoptEntryNode.setNext(graph.add(new DeoptEntryBeginNode())); @@ -772,7 +779,7 @@ private void insertDeoptNode(DeoptimizationTargetBciBlockMapping.DeoptEntryInser * DeoptEntries for positions not during an exception dispatch (rethrowException) * also must be linked to their exception target. */ - if (!deopt.rethrowException()) { + if (!deopt.isExceptionDispatch()) { /* * Saving frameState so that different modifications can be made for next() and * exceptionEdge().