--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/ConditionalEliminationPhase.java Thu Oct 17 20:27:44 2019 +0100
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/ConditionalEliminationPhase.java Thu Oct 17 20:53:35 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -56,6 +56,7 @@
import org.graalvm.compiler.nodes.AbstractMergeNode;
import org.graalvm.compiler.nodes.BinaryOpLogicNode;
import org.graalvm.compiler.nodes.ConditionAnchorNode;
+import org.graalvm.compiler.nodes.DeoptimizeNode;
import org.graalvm.compiler.nodes.DeoptimizingGuard;
import org.graalvm.compiler.nodes.EndNode;
import org.graalvm.compiler.nodes.FixedGuardNode;
@@ -90,20 +91,20 @@
import org.graalvm.compiler.nodes.extended.ValueAnchorNode;
import org.graalvm.compiler.nodes.java.InstanceOfNode;
import org.graalvm.compiler.nodes.java.TypeSwitchNode;
+import org.graalvm.compiler.nodes.spi.CoreProviders;
import org.graalvm.compiler.nodes.spi.NodeWithState;
import org.graalvm.compiler.nodes.spi.StampInverter;
import org.graalvm.compiler.nodes.util.GraphUtil;
import org.graalvm.compiler.phases.BasePhase;
import org.graalvm.compiler.phases.schedule.SchedulePhase;
import org.graalvm.compiler.phases.schedule.SchedulePhase.SchedulingStrategy;
-import org.graalvm.compiler.phases.tiers.PhaseContext;
import jdk.vm.ci.meta.DeoptimizationAction;
import jdk.vm.ci.meta.JavaConstant;
import jdk.vm.ci.meta.SpeculationLog.Speculation;
import jdk.vm.ci.meta.TriState;
-public class ConditionalEliminationPhase extends BasePhase<PhaseContext> {
+public class ConditionalEliminationPhase extends BasePhase<CoreProviders> {
private static final CounterKey counterStampsRegistered = DebugContext.counter("StampsRegistered");
private static final CounterKey counterStampsFound = DebugContext.counter("StampsFound");
@@ -123,7 +124,7 @@
@Override
@SuppressWarnings("try")
- protected void run(StructuredGraph graph, PhaseContext context) {
+ protected void run(StructuredGraph graph, CoreProviders context) {
try (DebugContext.Scope s = graph.getDebug().scope("DominatorConditionalElimination")) {
BlockMap<List<Node>> blockToNodes = null;
NodeMap<Block> nodeToBlock = null;
@@ -154,7 +155,7 @@
}
protected ControlFlowGraph.RecursiveVisitor<?> createVisitor(StructuredGraph graph, @SuppressWarnings("unused") ControlFlowGraph cfg, BlockMap<List<Node>> blockToNodes,
- NodeMap<Block> nodeToBlock, PhaseContext context) {
+ NodeMap<Block> nodeToBlock, CoreProviders context) {
return new Instance(graph, blockToNodes, nodeToBlock, context);
}
@@ -174,14 +175,9 @@
AbstractBeginNode beginNode = b.getBeginNode();
if (beginNode instanceof AbstractMergeNode && anchorBlock != b) {
AbstractMergeNode mergeNode = (AbstractMergeNode) beginNode;
- for (GuardNode guard : mergeNode.guards().snapshot()) {
- try (DebugCloseable closeable = guard.withNodeSourcePosition()) {
- GuardNode newlyCreatedGuard = new GuardNode(guard.getCondition(), anchorBlock.getBeginNode(), guard.getReason(), guard.getAction(), guard.isNegated(), guard.getSpeculation(),
- guard.getNoDeoptSuccessorPosition());
- GuardNode newGuard = mergeNode.graph().unique(newlyCreatedGuard);
- guard.replaceAndDelete(newGuard);
- }
- }
+ mergeNode.replaceAtUsages(InputType.Anchor, anchorBlock.getBeginNode());
+ mergeNode.replaceAtUsages(InputType.Guard, anchorBlock.getBeginNode());
+ assert mergeNode.anchored().isEmpty();
}
FixedNode endNode = b.getEndNode();
@@ -302,7 +298,7 @@
*/
private Deque<DeoptimizingGuard> pendingTests;
- public Instance(StructuredGraph graph, BlockMap<List<Node>> blockToNodes, NodeMap<Block> nodeToBlock, PhaseContext context) {
+ public Instance(StructuredGraph graph, BlockMap<List<Node>> blockToNodes, NodeMap<Block> nodeToBlock, CoreProviders context) {
this.graph = graph;
this.debug = graph.getDebug();
this.blockToNodes = blockToNodes;
@@ -313,7 +309,7 @@
this.conditions = new ArrayDeque<>();
tool = GraphUtil.getDefaultSimplifier(context.getMetaAccess(), context.getConstantReflection(), context.getConstantFieldProvider(), false, graph.getAssumptions(), graph.getOptions(),
context.getLowerer());
- mergeMaps = EconomicMap.create();
+ mergeMaps = EconomicMap.create(Equivalence.IDENTITY);
}
protected void processConditionAnchor(ConditionAnchorNode node) {
@@ -335,19 +331,26 @@
if (!tryProveGuardCondition(node, node.getCondition(), (guard, result, guardedValueStamp, newInput) -> {
if (result != node.isNegated()) {
node.replaceAndDelete(guard.asNode());
+ if (guard instanceof DeoptimizingGuard && !((DeoptimizingGuard) guard).isNegated()) {
+ rebuildPiNodes((DeoptimizingGuard) guard);
+ }
} else {
- /*
- * Don't kill this branch immediately because `killCFG` can have complex
- * implications in the presence of loops: it might replace or delete nodes in
- * other branches or even above the kill point. Instead of killing immediately,
- * just leave the graph in a state that is easy to simplify by a subsequent
- * canonicalizer phase.
- */
- FixedGuardNode deopt = new FixedGuardNode(LogicConstantNode.forBoolean(result, node.graph()), node.getReason(), node.getAction(), node.getSpeculation(), node.isNegated(),
- node.getNodeSourcePosition());
AbstractBeginNode beginNode = (AbstractBeginNode) node.getAnchor();
- graph.addAfterFixed(beginNode, node.graph().add(deopt));
+ if (beginNode.next() instanceof DeoptimizeNode) {
+ // This branch is already dead.
+ } else {
+ /*
+ * Don't kill this branch immediately because `killCFG` can have complex
+ * implications in the presence of loops: it might replace or delete nodes
+ * in other branches or even above the kill point. Instead of killing
+ * immediately, just leave the graph in a state that is easy to simplify by
+ * a subsequent canonicalizer phase.
+ */
+ FixedGuardNode deopt = new FixedGuardNode(LogicConstantNode.forBoolean(result, node.graph()), node.getReason(), node.getAction(), node.getSpeculation(), node.isNegated(),
+ node.getNodeSourcePosition());
+ graph.addAfterFixed(beginNode, node.graph().add(deopt));
+ }
}
return true;
})) {
@@ -361,41 +364,14 @@
node.replaceAtUsages(guard.asNode());
GraphUtil.unlinkFixedNode(node);
GraphUtil.killWithUnusedFloatingInputs(node);
+ if (guard instanceof DeoptimizingGuard && !((DeoptimizingGuard) guard).isNegated()) {
+ rebuildPiNodes((DeoptimizingGuard) guard);
+ }
} else {
node.setCondition(LogicConstantNode.forBoolean(result, node.graph()), node.isNegated());
// Don't kill this branch immediately, see `processGuard`.
}
- if (guard instanceof DeoptimizingGuard && !node.isNegated() && !((DeoptimizingGuard) guard).isNegated()) {
- LogicNode newCondition = ((DeoptimizingGuard) guard.asNode()).getCondition();
- if (newCondition instanceof InstanceOfNode) {
- InstanceOfNode inst = (InstanceOfNode) newCondition;
- ValueNode originalValue = GraphUtil.skipPi(inst.getValue());
- PiNode pi = null;
- // Ensure that any Pi that's weaker than what the instanceof proves is
- // replaced by one derived from the instanceof itself.
- for (PiNode existing : guard.asNode().usages().filter(PiNode.class).snapshot()) {
- if (!existing.isAlive()) {
- continue;
- }
- if (originalValue != GraphUtil.skipPi(existing.object())) {
- // Somehow these are unrelated values so leave it alone
- continue;
- }
- // If the pi has a weaker stamp or the same stamp but a different input
- // then replace it.
- boolean strongerStamp = !existing.piStamp().join(inst.getCheckedStamp()).equals(inst.getCheckedStamp());
- boolean differentStamp = !existing.piStamp().equals(inst.getCheckedStamp());
- boolean differentObject = existing.object() != inst.getValue();
- if (!strongerStamp && (differentStamp || differentObject)) {
- if (pi == null) {
- pi = graph.unique(new PiNode(inst.getValue(), inst.getCheckedStamp(), (ValueNode) guard));
- }
- existing.replaceAndDelete(pi);
- }
- }
- }
- }
debug.log("Kill fixed guard %s", node);
return true;
})) {
@@ -403,6 +379,59 @@
}
}
+ private void rebuildPiNodes(DeoptimizingGuard guard) {
+ LogicNode newCondition = guard.getCondition();
+ if (newCondition instanceof InstanceOfNode) {
+ InstanceOfNode inst = (InstanceOfNode) newCondition;
+ ValueNode originalValue = GraphUtil.skipPi(inst.getValue());
+ PiNode pi = null;
+ // Ensure that any Pi that's weaker than what the instanceof proves is
+ // replaced by one derived from the instanceof itself.
+ for (PiNode existing : guard.asNode().usages().filter(PiNode.class).snapshot()) {
+ if (!existing.isAlive()) {
+ continue;
+ }
+ if (originalValue != GraphUtil.skipPi(existing.object())) {
+ // Somehow these are unrelated values so leave it alone
+ continue;
+ }
+ // If the pi has a weaker stamp or the same stamp but a different input
+ // then replace it.
+ boolean strongerStamp = !existing.piStamp().join(inst.getCheckedStamp()).equals(inst.getCheckedStamp());
+ boolean differentCheckedStamp = !existing.piStamp().equals(inst.getCheckedStamp());
+ boolean differentObject = existing.object() != inst.getValue();
+ if (!strongerStamp && (differentCheckedStamp || differentObject)) {
+ if (pi == null) {
+ pi = graph.unique(new PiNode(inst.getValue(), inst.getCheckedStamp(), (ValueNode) guard));
+ }
+ if (!pi.stamp(NodeView.DEFAULT).join(existing.stamp(NodeView.DEFAULT)).equals(pi.stamp(NodeView.DEFAULT))) {
+ /*
+ * With a code sequence like null check, type check, null check of type
+ * checked value, CE will use the first null check to prove the second
+ * null check so the graph ends up a Pi guarded by the first null check
+ * but consuming the output Pi from the type check check. In this case
+ * we should still canonicalize the checked stamp for consistency.
+ */
+ if (differentCheckedStamp) {
+ PiNode alternatePi = graph.unique(new PiNode(existing.object(), inst.getCheckedStamp(), (ValueNode) guard));
+ /*
+ * If the resulting stamp is as good or better then do the
+ * replacement. However when interface types are involved it's
+ * possible that improving the checked stamp merges types which
+ * appear unrelated so there's we must skip the replacement.
+ */
+ if (alternatePi.stamp(NodeView.DEFAULT).join(existing.stamp(NodeView.DEFAULT)).equals(alternatePi.stamp(NodeView.DEFAULT))) {
+ existing.replaceAndDelete(alternatePi);
+ }
+ }
+ continue;
+ }
+ existing.replaceAndDelete(pi);
+ }
+ }
+ }
+ }
+
protected void processIf(IfNode node) {
tryProveCondition(node.condition(), (guard, result, guardedValueStamp, newInput) -> {
node.setCondition(LogicConstantNode.forBoolean(result, node.graph()));
@@ -587,7 +616,7 @@
Stamp newStamp = infoElement.getStamp();
if (phi.stamp(NodeView.DEFAULT).tryImproveWith(newStamp) != null) {
if (mergeMap == null) {
- mergeMap = EconomicMap.create();
+ mergeMap = EconomicMap.create(Equivalence.IDENTITY);
mergeMaps.put(merge, mergeMap);
}