Skip to content

Commit 61820b7

Browse files
robcaslozrobcasloz
committed
8259984: IGV: Crash when drawing control flow before GCM
Replace backward traversal in the IGV block formation algorithm by forward traversal guided by node category information. This change addresses the reported assertion failures, places block projection nodes together with their predecessors, and gives a more natural block numbering. Reviewed-by: chagedorn, neliasso
1 parent 7e2c909 commit 61820b7

File tree

1 file changed

+154
-77
lines changed

1 file changed

+154
-77
lines changed

‎src/utils/IdealGraphVisualizer/ServerCompiler/src/com/sun/hotspot/igv/servercompiler/ServerCompilerScheduler.java

+154-77
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1998, 2015, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -45,6 +45,7 @@ private static class Node {
4545
public InputBlock block;
4646
public boolean isBlockProjection;
4747
public boolean isBlockStart;
48+
public boolean isCFG;
4849
}
4950
private InputGraph graph;
5051
private Collection<Node> nodes;
@@ -63,112 +64,122 @@ public int compare(InputEdge o1, InputEdge o2) {
6364

6465
public void buildBlocks() {
6566

67+
// Initialize data structures.
6668
blocks = new Vector<>();
6769
Node root = findRoot();
6870
if (root == null) {
6971
return;
7072
}
7173
Stack<Node> stack = new Stack<>();
7274
Set<Node> visited = new HashSet<>();
75+
Map<InputBlock, Set<Node>> terminators = new HashMap<>();
76+
// Pre-compute control successors of each node, excluding self edges.
77+
Map<Node, Set<Node>> controlSuccs = new HashMap<>();
78+
for (Node n : nodes) {
79+
if (n.isCFG) {
80+
Set<Node> nControlSuccs = new HashSet<>();
81+
for (Node s : n.succs) {
82+
if (s.isCFG && s != n) {
83+
nControlSuccs.add(s);
84+
}
85+
}
86+
controlSuccs.put(n, nControlSuccs);
87+
}
88+
}
7389
stack.add(root);
74-
int blockCount = 0;
90+
// Start from 1 to follow the style of compiler-generated CFGs.
91+
int blockCount = 1;
7592
InputBlock rootBlock = null;
7693

77-
94+
// Traverse the control-flow subgraph forwards, starting from the root.
7895
while (!stack.isEmpty()) {
79-
Node proj = stack.pop();
80-
Node parent = proj;
81-
if (proj.isBlockProjection && proj.preds.size() > 0) {
82-
parent = proj.preds.get(0);
83-
}
84-
85-
if (!visited.contains(parent)) {
86-
visited.add(parent);
87-
InputBlock block = graph.addBlock(Integer.toString(blockCount));
88-
blocks.add(block);
89-
if (parent == root) {
90-
rootBlock = block;
91-
}
92-
blockCount++;
93-
parent.block = block;
94-
if (proj != parent && proj.succs.size() == 1 && proj.succs.contains(root)) {
95-
// Special treatment of Halt-nodes
96-
proj.block = block;
97-
}
98-
99-
Node p = proj;
100-
do {
101-
if (p.preds.size() == 0 || p.preds.get(0) == null) {
102-
p = parent;
103-
break;
104-
}
105-
106-
p = p.preds.get(0);
107-
if (p == proj) {
108-
// Cycle, stop
96+
// Pop a node, mark it as visited, and create a new block.
97+
Node n = stack.pop();
98+
if (visited.contains(n)) {
99+
continue;
100+
}
101+
visited.add(n);
102+
InputBlock block = graph.addBlock(Integer.toString(blockCount));
103+
blocks.add(block);
104+
if (n == root) {
105+
rootBlock = block;
106+
}
107+
blockCount++;
108+
Set<Node> blockTerminators = new HashSet<Node>();
109+
// Move forwards until a terminator node is found, assigning all
110+
// visited nodes to the current block.
111+
while (true) {
112+
// Assign n to current block.
113+
n.block = block;
114+
if (controlSuccs.get(n).size() == 0) {
115+
// No successors: end the block.
116+
blockTerminators.add(n);
117+
break;
118+
} else if (controlSuccs.get(n).size() == 1) {
119+
// One successor: end the block if it is a block start node.
120+
Node s = controlSuccs.get(n).iterator().next();
121+
if (s.isBlockStart) {
122+
// Block start: end the block.
123+
blockTerminators.add(n);
124+
stack.push(s);
109125
break;
126+
} else {
127+
// Not a block start: keep filling the current block.
128+
n = s;
110129
}
111-
112-
if (p.block == null) {
113-
p.block = block;
114-
}
115-
} while (!p.isBlockProjection && !p.isBlockStart);
116-
117-
if (block != rootBlock) {
118-
for (Node n : p.preds) {
119-
if (n != null && n != p) {
120-
if (n.isBlockProjection) {
121-
n = n.preds.get(0);
122-
}
123-
if (n.block != null) {
124-
graph.addBlockEdge(n.block, block);
125-
}
126-
}
127-
}
128-
}
129-
130-
for (Node n : parent.succs) {
131-
if (n != root && n.isBlockProjection) {
132-
for (Node n2 : n.succs) {
133-
134-
if (n2 != parent && n2.block != null && n2.block != rootBlock) {
135-
graph.addBlockEdge(block, n2.block);
130+
} else {
131+
// Multiple successors: end the block.
132+
for (Node s : controlSuccs.get(n)) {
133+
if (s.isBlockProjection && s != root) {
134+
// Assign block projections to the current block,
135+
// and push their successors to the stack. In the
136+
// normal case, we would expect control projections
137+
// to have only one successor, but there are some
138+
// intermediate graphs (e.g. 'Before RemoveUseless')
139+
// where 'IfX' nodes flow both to 'Region' and
140+
// (dead) 'Safepoint' nodes.
141+
s.block = block;
142+
blockTerminators.add(s);
143+
for (Node ps : controlSuccs.get(s)) {
144+
stack.push(ps);
136145
}
137-
}
138-
} else {
139-
if (n != parent && n.block != null && n.block != rootBlock) {
140-
graph.addBlockEdge(block, n.block);
146+
} else {
147+
blockTerminators.add(n);
148+
stack.push(s);
141149
}
142150
}
151+
break;
143152
}
153+
}
154+
terminators.put(block, blockTerminators);
155+
}
144156

145-
int num_preds = p.preds.size();
146-
int bottom = -1;
147-
if (isRegion(p) || isPhi(p)) {
148-
bottom = 0;
149-
}
150-
151-
int pushed = 0;
152-
for (int i = num_preds - 1; i > bottom; i--) {
153-
if (p.preds.get(i) != null && p.preds.get(i) != p) {
154-
stack.push(p.preds.get(i));
155-
pushed++;
157+
// Add block edges based on terminator successors. Note that a block
158+
// might have multiple terminators preceding the same successor block.
159+
for (Map.Entry<InputBlock, Set<Node>> terms : terminators.entrySet()) {
160+
// Unique set of terminator successors.
161+
Set<Node> uniqueSuccs = new HashSet<>();
162+
for (Node t : terms.getValue()) {
163+
for (Node s : controlSuccs.get(t)) {
164+
if (s.block != rootBlock) {
165+
uniqueSuccs.add(s);
156166
}
157167
}
158-
159-
if (pushed == 0 && p == root) {
160-
// TODO: special handling when root backedges are not built yet
161-
}
168+
}
169+
for (Node s : uniqueSuccs) {
170+
graph.addBlockEdge(terms.getKey(), s.block);
162171
}
163172
}
164173

174+
// Fill the blocks.
165175
for (Node n : nodes) {
166176
InputBlock block = n.block;
167177
if (block != null) {
168178
block.addNode(n.inputNode.getId());
169179
}
170180
}
171181

182+
// Compute block index map for dominator computation.
172183
int z = 0;
173184
blockIndex = new HashMap<>(blocks.size());
174185
for (InputBlock b : blocks) {
@@ -203,6 +214,8 @@ public Collection<InputBlock> schedule(InputGraph graph) {
203214

204215
this.graph = graph;
205216
buildUpGraph();
217+
markCFGNodes();
218+
connectOrphansAndWidows();
206219
buildBlocks();
207220
buildDominators();
208221
buildCommonDominators();
@@ -621,4 +634,68 @@ public void buildUpGraph() {
621634
}
622635
}
623636
}
637+
638+
// Mark nodes that form the CFG (same as shown by the 'Show control flow
639+
// only' filter, plus the Root node).
640+
public void markCFGNodes() {
641+
for (Node n : nodes) {
642+
String category = n.inputNode.getProperties().get("category");
643+
assert category != null :
644+
"Node category not found, please use input from a compatible " +
645+
"compiler version";
646+
if (category.equals("control") || category.equals("mixed")) {
647+
// Example: If, IfTrue, CallStaticJava.
648+
n.isCFG = true;
649+
} else if (n.inputNode.getProperties().get("type").equals("bottom")
650+
&& n.preds.size() > 0 &&
651+
n.preds.get(0) != null &&
652+
n.preds.get(0).inputNode.getProperties()
653+
.get("category").equals("control")) {
654+
// Example: Halt, Return, Rethrow.
655+
n.isCFG = true;
656+
} else if (n.isBlockStart || n.isBlockProjection) {
657+
// Example: Root.
658+
n.isCFG = true;
659+
} else {
660+
n.isCFG = false;
661+
}
662+
}
663+
}
664+
665+
// Fix ill-formed graphs with orphan/widow control-flow nodes by adding
666+
// edges from/to the Root node. Such edges are assumed by different parts of
667+
// the scheduling algorithm, but are not always present, e.g. for certain
668+
// 'Safepoint' nodes in the 'Before RemoveUseless' phase.
669+
public void connectOrphansAndWidows() {
670+
Node root = findRoot();
671+
if (root == null) {
672+
return;
673+
}
674+
for (Node n : nodes) {
675+
if (n.isCFG) {
676+
boolean orphan = true;
677+
for (Node p : n.preds) {
678+
if (p != n && p.isCFG) {
679+
orphan = false;
680+
}
681+
}
682+
if (orphan) {
683+
// Add edge from root to this node.
684+
root.succs.add(n);
685+
n.preds.add(0, root);
686+
}
687+
boolean widow = true;
688+
for (Node s : n.succs) {
689+
if (s != n && s.isCFG) {
690+
widow = false;
691+
}
692+
}
693+
if (widow) {
694+
// Add edge from this node to root.
695+
root.preds.add(n);
696+
n.succs.add(root);
697+
}
698+
}
699+
}
700+
}
624701
}

0 commit comments

Comments
 (0)