[GR-44459] Track extra metadata about DeoptEntry/DeoptProxyAnchor nodes.

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);

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

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) {

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

@@ -104,6 +104,14 @@ public abstract class TypeFlow<T> {
      */
     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<TypeFlow, TypeState> 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,15 +608,19 @@ 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);
             }
         }
 
         for (TypeFlow<?> observer : getObservers()) {
-            observer.onObservedUpdate(bb);
+            if (observer.isValid()) {
+                observer.onObservedUpdate(bb);
+            } else {
+                removeObserver(observer);
+            }
         }
     }
 

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;
+    }
 }

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();
 }

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<DeoptProxyAnchorNode> 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<? extends DeoptProxyAnchorNode> c) {
-        super(c, StampFactory.forVoid());
+        this.proxifiedInvokeBci = proxifiedInvokeBci;
     }
 
     @Override
     public void generate(NodeLIRBuilderTool gen) {
         /* No-op */
     }
+
+    @Override
+    public int getProxifiedInvokeBci() {
+        return proxifiedInvokeBci;
+    }
 }

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<Va
              * exception handlers and directly unwind.
              */
             int bci = invoke.stateAfter().bci;
-            appendWithUnwind(new DeoptEntryNode(), bci);
+            appendWithUnwind(DeoptEntryNode.create(invoke.bci()), bci);
         }
 
         ValueNode result = invoke;

substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java

@@ -68,6 +68,7 @@
 import org.graalvm.compiler.nodes.graphbuilderconf.InlineInvokePlugin;
 import org.graalvm.compiler.nodes.graphbuilderconf.IntrinsicContext;
 import org.graalvm.compiler.nodes.graphbuilderconf.NodePlugin;
+import org.graalvm.compiler.nodes.java.ExceptionObjectNode;
 import org.graalvm.compiler.options.Option;
 import org.graalvm.compiler.options.OptionValues;
 import org.graalvm.compiler.phases.OptimisticOptimizations;
@@ -102,6 +103,8 @@
 import com.oracle.svm.core.ParsingReason;
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.graal.nodes.DeoptEntryNode;
+import com.oracle.svm.core.graal.nodes.DeoptEntrySupport;
+import com.oracle.svm.core.graal.nodes.DeoptProxyAnchorNode;
 import com.oracle.svm.core.graal.nodes.InlinedInvokeArgumentsNode;
 import com.oracle.svm.core.graal.stackvalue.StackValueNode;
 import com.oracle.svm.core.option.HostedOptionKey;
@@ -127,6 +130,7 @@
 import com.oracle.svm.hosted.phases.InlineBeforeAnalysisPolicyUtils.AlwaysInlineScope;
 import com.oracle.svm.hosted.phases.StrengthenStampsPhase;
 
+import jdk.vm.ci.code.BytecodeFrame;
 import jdk.vm.ci.code.BytecodePosition;
 import jdk.vm.ci.meta.JavaKind;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
@@ -139,12 +143,8 @@
 public class ParseOnceRuntimeCompilationFeature extends RuntimeCompilationFeature implements Feature, RuntimeCompilationSupport {
 
     public static class Options {
-        /*
-         * Note this phase is currently overly aggressive and can illegally remove proxies. This
-         * will be fixed in GR-44459.
-         */
         @Option(help = "Remove Deopt(Entries,Anchors,Proxies) determined to be unneeded after the runtime compiled graphs have been finalized.")//
-        public static final HostedOptionKey<Boolean> RemoveUnneededDeoptSupport = new HostedOptionKey<>(false);
+        public static final HostedOptionKey<Boolean> RemoveUnneededDeoptSupport = new HostedOptionKey<>(true);
 
         @Option(help = "Perform InlineBeforeAnalysis on runtime compiled methods")//
         public static final HostedOptionKey<Boolean> 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<StateSplit, Boolean> decisionCache = EconomicMap.create();
+            EconomicMap<StateSplit, RemovalDecision> 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<StateSplit, Boolean> decisionCache) {
-            Boolean cached = decisionCache.get(node);
+        RemovalDecision getDecision(StateSplit node, EconomicMap<StateSplit, RemovalDecision> 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";
         }
     }
 }