[GR-48365] Be more conservative when merging PiNodes and ValueAnchorN…

…odes

PullRequest: graal/15653

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ea/EscapeAnalysisTest.java

@@ -408,7 +408,7 @@ public boolean testInstanceOfSnippet() {
 
     @SuppressWarnings("unused")
     public static void testNewNodeSnippet() {
-        new ValueAnchorNode(null);
+        new ValueAnchorNode();
     }
 
     /**

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/ConditionAnchorNode.java

@@ -86,7 +86,7 @@ public Node canonical(CanonicalizerTool tool, Node forValue) {
             // An anchor that still has usages must still exist since it's possible the condition is
             // true for control flow reasons so the Pi stamp is also only valid for those reason.
             if (c.getValue() == negated || hasUsages()) {
-                return new ValueAnchorNode(null);
+                return new ValueAnchorNode();
             } else {
                 return null;
             }
@@ -100,7 +100,7 @@ public Node canonical(CanonicalizerTool tool, Node forValue) {
     @Override
     public void lower(LoweringTool tool) {
         if (graph().getGuardsStage() == GraphState.GuardsStage.FIXED_DEOPTS) {
-            ValueAnchorNode newAnchor = graph().add(new ValueAnchorNode(null));
+            ValueAnchorNode newAnchor = graph().add(new ValueAnchorNode());
             graph().replaceFixedWithFixed(this, newAnchor);
         }
     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/FixedGuardNode.java

@@ -103,7 +103,7 @@ public void simplify(SimplifierTool tool) {
                  * are required to be anchored. After guard lowering it might be possible to replace
                  * the ValueAnchorNode with the Begin of the current block.
                  */
-                graph().replaceFixedWithFixed(this, graph().add(new ValueAnchorNode(null)));
+                graph().replaceFixedWithFixed(this, graph().add(new ValueAnchorNode()));
             } else {
                 graph().removeFixed(this);
             }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/PiNode.java

@@ -227,7 +227,7 @@ public void virtualize(VirtualizerTool tool) {
         }
     }
 
-    public static ValueNode canonical(ValueNode object, Stamp piStamp, GuardingNode guard, PiNode self) {
+    public static ValueNode canonical(ValueNode object, Stamp piStamp, GuardingNode guard, ValueNode self) {
         // Use most up to date stamp.
         Stamp computedStamp = piStamp.improveWith(object.stamp(NodeView.DEFAULT));
 
@@ -257,15 +257,16 @@ public static ValueNode canonical(ValueNode object, Stamp piStamp, GuardingNode
                     }
                     if (otherPi.object() == self || otherPi.object() == object) {
                         // Check if other pi's stamp is more precise
-                        Stamp joinedStamp = piStamp.improveWith(otherPi.piStamp());
-                        if (joinedStamp.equals(piStamp)) {
+                        Stamp joinedPiStamp = piStamp.improveWith(otherPi.piStamp());
+                        if (joinedPiStamp.equals(piStamp)) {
                             // Stamp did not get better, nothing to do.
-                        } else if (otherPi.object() == object && joinedStamp.equals(otherPi.piStamp())) {
+                        } else if (otherPi.object() == object && joinedPiStamp.equals(otherPi.piStamp())) {
                             // We can be replaced with the other pi.
                             return otherPi;
-                        } else {
-                            // Create a new pi node with the more precise joined stamp.
-                            return new PiNode(object, joinedStamp, guard.asNode());
+                        } else if (self != null && self.hasExactlyOneUsage() && otherPi.object == self) {
+                            if (joinedPiStamp.equals(otherPi.piStamp)) {
+                                return object;
+                            }
                         }
                     }
                 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/extended/ValueAnchorNode.java

@@ -30,106 +30,45 @@
 import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_0;
 
 import jdk.graal.compiler.core.common.type.StampFactory;
+import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeClass;
 import jdk.graal.compiler.graph.spi.NodeWithIdentity;
 import jdk.graal.compiler.nodeinfo.NodeInfo;
-import jdk.graal.compiler.nodes.virtual.VirtualObjectNode;
-import jdk.graal.compiler.nodes.AbstractBeginNode;
-import jdk.graal.compiler.nodes.FixedNode;
 import jdk.graal.compiler.nodes.FixedWithNextNode;
-import jdk.graal.compiler.nodes.ValueNode;
 import jdk.graal.compiler.nodes.debug.ControlFlowAnchored;
-import jdk.graal.compiler.nodes.memory.FixedAccessNode;
+import jdk.graal.compiler.nodes.spi.Canonicalizable;
+import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
 import jdk.graal.compiler.nodes.spi.LIRLowerable;
 import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool;
-import jdk.graal.compiler.nodes.spi.Simplifiable;
-import jdk.graal.compiler.nodes.spi.SimplifierTool;
-import jdk.graal.compiler.nodes.spi.Virtualizable;
-import jdk.graal.compiler.nodes.spi.VirtualizerTool;
-import jdk.graal.compiler.nodes.util.GraphUtil;
 
 /**
- * This node can be used for two different kinds of anchoring for non-CFG (floating) nodes: It can
- * 1) keep one node above a certain point in the graph by specifying that node as the
- * {@link #anchored} node; or 2) it can keep nodes below a certain point in the graph by using this
- * node as an Anchor or Guard input.
+ * This node can be used to keep nodes below a certain point in the graph by using this node as an
+ * Anchor or Guard input.
  *
  * This node must not move in the CFG, because that would change the anchor point. So optimizations
  * like de-duplication are not allowed and this node implements {@link NodeWithIdentity}. However,
  * duplication is allowed, i.e., it is on purpose that this node does not implement
  * {@link ControlFlowAnchored}.
  */
 @NodeInfo(allowedUsageTypes = {Anchor, Guard}, cycles = CYCLES_0, size = SIZE_0)
-public final class ValueAnchorNode extends FixedWithNextNode implements LIRLowerable, Simplifiable, Virtualizable, AnchoringNode, GuardingNode, NodeWithIdentity {
+public final class ValueAnchorNode extends FixedWithNextNode implements LIRLowerable, Canonicalizable, AnchoringNode, GuardingNode, NodeWithIdentity {
 
     public static final NodeClass<ValueAnchorNode> TYPE = NodeClass.create(ValueAnchorNode.class);
-    @OptionalInput(Guard) ValueNode anchored;
 
-    public ValueAnchorNode(ValueNode value) {
+    public ValueAnchorNode() {
         super(TYPE, StampFactory.forVoid());
-        this.anchored = value;
     }
 
     @Override
     public void generate(NodeLIRBuilderTool gen) {
         // Nothing to emit, since this node is used for structural purposes only.
     }
 
-    public ValueNode getAnchoredNode() {
-        return anchored;
-    }
-
     @Override
-    public void simplify(SimplifierTool tool) {
-        while (next() instanceof ValueAnchorNode) {
-            ValueAnchorNode nextAnchor = (ValueAnchorNode) next();
-            if (nextAnchor.anchored == anchored || nextAnchor.anchored == null) {
-                // two anchors for the same anchored -> coalesce
-                // nothing anchored on the next anchor -> coalesce
-                nextAnchor.replaceAtUsages(this);
-                GraphUtil.removeFixedWithUnusedInputs(nextAnchor);
-                /*
-                 * Combining two anchors can allow the combining of two PiNode that are now anchored
-                 * at the same place.
-                 */
-                tool.addToWorkList(usages());
-            } else {
-                break;
-            }
-        }
-        if (tool.allUsagesAvailable() && hasNoUsages() && next() instanceof FixedAccessNode) {
-            FixedAccessNode currentNext = (FixedAccessNode) next();
-            if (currentNext.getGuard() == anchored) {
-                GraphUtil.removeFixedWithUnusedInputs(this);
-                return;
-            }
-        }
-
-        if (anchored != null && (anchored.isConstant() || anchored instanceof FixedNode)) {
-            // anchoring fixed nodes and constants is useless
-            removeAnchoredNode();
-        }
-
-        if (anchored == null && hasNoUsages()) {
-            // anchor is not necessary any more => remove.
-            GraphUtil.removeFixedWithUnusedInputs(this);
+    public Node canonical(CanonicalizerTool tool) {
+        if (tool.allUsagesAvailable() && hasNoUsages()) {
+            return null;
         }
-    }
-
-    @Override
-    public void virtualize(VirtualizerTool tool) {
-        if (anchored == null || anchored instanceof AbstractBeginNode) {
-            tool.delete();
-        } else {
-            ValueNode alias = tool.getAlias(anchored);
-            if (alias instanceof VirtualObjectNode) {
-                tool.delete();
-            }
-        }
-    }
-
-    public void removeAnchoredNode() {
-        this.updateUsages(anchored, null);
-        this.anchored = null;
+        return this;
     }
 }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/LoopFragmentInside.java

@@ -320,7 +320,7 @@ protected CompareNode placeNewSegmentAndCleanup(LoopEx loop, EconomicMap<Node, N
                      * anchor point here ensuring nothing can flow above the original iteration.
                      */
                     if (!(lastCodeNode instanceof GuardingNode) || !(lastCodeNode instanceof AnchoringNode)) {
-                        ValueAnchorNode newAnchoringPointAfterPrevIteration = graph.add(new ValueAnchorNode(null));
+                        ValueAnchorNode newAnchoringPointAfterPrevIteration = graph.add(new ValueAnchorNode());
                         graph.addAfterFixed(lastCodeNode, newAnchoringPointAfterPrevIteration);
                         lastCodeNode = newAnchoringPointAfterPrevIteration;
                     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ConditionalEliminationPhase.java

@@ -425,7 +425,7 @@ protected void processConditionAnchor(ConditionAnchorNode node) {
                     GraphUtil.unlinkFixedNode(node);
                     GraphUtil.killWithUnusedFloatingInputs(node);
                 } else {
-                    ValueAnchorNode valueAnchor = node.graph().add(new ValueAnchorNode(null));
+                    ValueAnchorNode valueAnchor = node.graph().add(new ValueAnchorNode());
                     node.replaceAtUsages(valueAnchor);
                     node.graph().replaceFixedWithFixed(node, valueAnchor);
                 }
@@ -437,7 +437,11 @@ protected void processConditionAnchor(ConditionAnchorNode node) {
         protected void processGuard(GuardNode node) {
             if (!tryProveGuardCondition(node, node.getCondition(), (guard, result, guardedValueStamp, newInput) -> {
                 if (result != node.isNegated()) {
+                    ValueNode condition = node.getCondition();
                     node.replaceAndDelete(guard.asNode());
+                    if (condition.hasNoUsages()) {
+                        GraphUtil.killWithUnusedFloatingInputs(condition);
+                    }
                     if (guard instanceof DeoptimizingGuard && !((DeoptimizingGuard) guard).isNegated()) {
                         rebuildPiNodes((DeoptimizingGuard) guard);
                     }

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/inlining/InliningUtil.java

@@ -697,7 +697,7 @@ private static Pair<ValueNode, FixedNode> replaceInvokeAtUsages(ValueNode invoke
                 StructuredGraph graph = origReturn.graph();
                 if (!(anchorCandidate instanceof AbstractBeginNode)) {
                     // Add anchor for pi after the original candidate
-                    ValueAnchorNode anchor = graph.add(new ValueAnchorNode(null));
+                    ValueAnchorNode anchor = graph.add(new ValueAnchorNode());
                     if (anchorCandidate.predecessor() == null) {
                         anchor.setNext(anchorCandidate);
                     } else {

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/nodes/MethodHandleNode.java

@@ -35,25 +35,21 @@
 import jdk.graal.compiler.core.common.type.StampPair;
 import jdk.graal.compiler.core.common.type.TypeReference;
 import jdk.graal.compiler.debug.GraalError;
-import jdk.graal.compiler.graph.Node;
 import jdk.graal.compiler.graph.NodeClass;
 import jdk.graal.compiler.nodeinfo.NodeInfo;
 import jdk.graal.compiler.nodes.CallTargetNode;
 import jdk.graal.compiler.nodes.CallTargetNode.InvokeKind;
 import jdk.graal.compiler.nodes.FixedGuardNode;
 import jdk.graal.compiler.nodes.FixedNode;
 import jdk.graal.compiler.nodes.FixedWithNextNode;
-import jdk.graal.compiler.nodes.GuardNode;
 import jdk.graal.compiler.nodes.Invoke;
 import jdk.graal.compiler.nodes.InvokeNode;
 import jdk.graal.compiler.nodes.LogicNode;
 import jdk.graal.compiler.nodes.NodeView;
 import jdk.graal.compiler.nodes.PiNode;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.ValueNode;
-import jdk.graal.compiler.nodes.extended.AnchoringNode;
 import jdk.graal.compiler.nodes.extended.GuardingNode;
-import jdk.graal.compiler.nodes.extended.ValueAnchorNode;
 import jdk.graal.compiler.nodes.java.InstanceOfNode;
 import jdk.graal.compiler.nodes.java.MethodCallTargetNode;
 import jdk.graal.compiler.nodes.spi.Simplifiable;
@@ -138,14 +134,6 @@ public GraphAdder(StructuredGraph graph) {
          */
         public abstract <T extends ValueNode> T add(T node);
 
-        /**
-         * @return an {@link AnchoringNode} if floating guards should be created, otherwise
-         *         {@link FixedGuardNode}s will be used.
-         */
-        public AnchoringNode getGuardAnchor() {
-            return null;
-        }
-
         public Assumptions getAssumptions() {
             return graph.getAssumptions();
         }
@@ -331,19 +319,10 @@ private static void maybeCastArgument(GraphAdder adder, ValueNode[] arguments, i
                     assert !inst.isAlive();
                     if (!inst.isTautology()) {
                         inst = adder.add(inst);
-                        AnchoringNode guardAnchor = adder.getGuardAnchor();
                         DeoptimizationReason reason = DeoptimizationReason.ClassCastException;
                         DeoptimizationAction action = DeoptimizationAction.InvalidateRecompile;
                         Speculation speculation = SpeculationLog.NO_SPECULATION;
-                        GuardingNode guard;
-                        if (guardAnchor == null) {
-                            FixedGuardNode fixedGuard = adder.add(new FixedGuardNode(inst, reason, action, speculation, false));
-                            guard = fixedGuard;
-                        } else {
-                            GuardNode newGuard = adder.add(new GuardNode(inst, guardAnchor, reason, action, false, speculation, null));
-                            adder.add(new ValueAnchorNode(newGuard));
-                            guard = newGuard;
-                        }
+                        GuardingNode guard = adder.add(new FixedGuardNode(inst, reason, action, speculation, false));
                         ValueNode valueNode = adder.add(PiNode.create(argument, StampFactory.object(targetType), guard.asNode()));
                         arguments[index] = valueNode;
                     }

substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java

@@ -402,7 +402,7 @@ public void simplify(Node n, SimplifierTool tool) {
                          * anchor the PiNode at the BeginNode of the preceding block, because at
                          * that point the condition is not proven yet.
                          */
-                        ValueAnchorNode anchor = graph.add(new ValueAnchorNode(null));
+                        ValueAnchorNode anchor = graph.add(new ValueAnchorNode());
                         graph.addAfterFixed(survivingBegin, anchor);
                         survivingBegin.replaceAtUsages(anchor, InputType.Guard, InputType.Anchor);
                     }
@@ -726,7 +726,7 @@ private ValueNode insertPi(ValueNode input, Object newStampOrConstant, FixedWith
                 return null;
             }
 
-            ValueAnchorNode anchor = graph.add(new ValueAnchorNode(null));
+            ValueAnchorNode anchor = graph.add(new ValueAnchorNode());
             graph.addAfterFixed(anchorPoint, anchor);
             return graph.unique(new PiNode(input, piStamp, anchor));
         }