Skip to content

Commit d023440

Browse files
committed
[JDK-8365217] Relax fixed guard check in FixReadsPhase.
PullRequest: graal/21823
2 parents afaec4b + b42bd0c commit d023440

File tree

2 files changed

+42
-11
lines changed

2 files changed

+42
-11
lines changed

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,27 @@
2828
import static jdk.graal.compiler.nodeinfo.NodeCycles.CYCLES_0;
2929
import static jdk.graal.compiler.nodeinfo.NodeSize.SIZE_0;
3030

31+
import java.util.ArrayList;
32+
3133
import org.graalvm.collections.EconomicSet;
34+
3235
import jdk.graal.compiler.core.common.type.StampFactory;
3336
import jdk.graal.compiler.graph.Node;
3437
import jdk.graal.compiler.graph.NodeBitMap;
3538
import jdk.graal.compiler.graph.NodeClass;
3639
import jdk.graal.compiler.graph.NodeInputList;
37-
import jdk.graal.compiler.nodes.spi.Canonicalizable;
38-
import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
39-
import jdk.graal.compiler.nodes.spi.Simplifiable;
40-
import jdk.graal.compiler.nodes.spi.SimplifierTool;
4140
import jdk.graal.compiler.nodeinfo.NodeInfo;
4241
import jdk.graal.compiler.nodes.StructuredGraph;
4342
import jdk.graal.compiler.nodes.ValueNode;
4443
import jdk.graal.compiler.nodes.calc.FloatingNode;
44+
import jdk.graal.compiler.nodes.spi.Canonicalizable;
45+
import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
4546
import jdk.graal.compiler.nodes.spi.LIRLowerable;
4647
import jdk.graal.compiler.nodes.spi.NodeLIRBuilderTool;
48+
import jdk.graal.compiler.nodes.spi.Simplifiable;
49+
import jdk.graal.compiler.nodes.spi.SimplifierTool;
4750
import jdk.graal.compiler.nodes.util.GraphUtil;
4851

49-
import java.util.ArrayList;
50-
5152
@NodeInfo(allowedUsageTypes = Guard, cycles = CYCLES_0, size = SIZE_0)
5253
public final class MultiGuardNode extends FloatingNode implements GuardingNode, LIRLowerable, Simplifiable, Canonicalizable, Node.ValueNumberable {
5354
public static final NodeClass<MultiGuardNode> TYPE = NodeClass.create(MultiGuardNode.class);
@@ -167,4 +168,8 @@ public static GuardingNode addGuard(GuardingNode first, GuardingNode second) {
167168
return combine(first, second);
168169
}
169170
}
171+
172+
public NodeInputList<ValueNode> getGuards() {
173+
return guards;
174+
}
170175
}

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

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
*/
2525
package jdk.graal.compiler.phases.common;
2626

27+
import java.util.ArrayDeque;
28+
import java.util.Deque;
2729
import java.util.ListIterator;
2830
import java.util.Optional;
2931

@@ -67,6 +69,7 @@
6769
import jdk.graal.compiler.nodes.StructuredGraph.ScheduleResult;
6870
import jdk.graal.compiler.nodes.UnaryOpLogicNode;
6971
import jdk.graal.compiler.nodes.ValueNode;
72+
import jdk.graal.compiler.nodes.ValueNodeInterface;
7073
import jdk.graal.compiler.nodes.ValuePhiNode;
7174
import jdk.graal.compiler.nodes.calc.BinaryNode;
7275
import jdk.graal.compiler.nodes.calc.ConditionalNode;
@@ -76,6 +79,7 @@
7679
import jdk.graal.compiler.nodes.cfg.HIRBlock;
7780
import jdk.graal.compiler.nodes.extended.GuardingNode;
7881
import jdk.graal.compiler.nodes.extended.IntegerSwitchNode;
82+
import jdk.graal.compiler.nodes.extended.MultiGuardNode;
7983
import jdk.graal.compiler.nodes.memory.FixedAccessNode;
8084
import jdk.graal.compiler.nodes.memory.FloatingAccessNode;
8185
import jdk.graal.compiler.nodes.memory.FloatingReadNode;
@@ -706,17 +710,39 @@ public static boolean verifyPiRemovalInvariants(StructuredGraph graph) {
706710
// floating guarded nodes without a guard are "universally" true meaning they
707711
// can be executed everywhere
708712
final GuardingNode guard = ((FloatingGuardedNode) n).getGuard();
709-
final boolean isUniversallyTrue = guard == null;
710-
if (!isUniversallyTrue) {
711-
assert guard instanceof FixedNode : Assertions.errorMessage(
712-
"Should not have floating guarded nodes without fixed guards left after removing pis, they could float uncontrolled now", n);
713-
}
713+
assert verifyOnlyFixedGuards(guard);
714714
}
715715
}
716716
}
717717
return true;
718718
}
719719

720+
private static boolean verifyOnlyFixedGuards(ValueNodeInterface guardingRoot) {
721+
final boolean isUniversallyTrue = guardingRoot == null;
722+
if (isUniversallyTrue) {
723+
return true;
724+
}
725+
final StructuredGraph graph = guardingRoot.asNode().graph();
726+
NodeBitMap visited = graph.createNodeBitMap();
727+
Deque<ValueNodeInterface> toVisit = new ArrayDeque<>();
728+
toVisit.add(guardingRoot);
729+
730+
while (!toVisit.isEmpty()) {
731+
ValueNodeInterface currentGuard = toVisit.pop();
732+
if (visited.isMarked(currentGuard.asNode())) {
733+
continue;
734+
}
735+
visited.mark(currentGuard.asNode());
736+
if (currentGuard instanceof MultiGuardNode mg) {
737+
toVisit.addAll(mg.getGuards());
738+
} else {
739+
assert currentGuard instanceof FixedNode : Assertions.errorMessage(
740+
"Should not have floating guarded nodes without fixed guards left after removing pis, they could float uncontrolled now", currentGuard);
741+
}
742+
}
743+
return true;
744+
}
745+
720746
@Override
721747
public void updateGraphState(GraphState graphState) {
722748
super.updateGraphState(graphState);

0 commit comments

Comments
 (0)