Skip to content

[GR-67492] Optimizations for schedule phase #11615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 15, 2025
Merged

[GR-67492] Optimizations for schedule phase #11615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c98ca5
Make the latest schedule sorting within one block more efficient by
thomaswue Jul 12, 2025
75aeb90
Small fix in schedule phase when checking for existence of floating g…
thomaswue Jul 12, 2025
1299101
Fix assertions with missing message.
thomaswue Jul 12, 2025
8716739
Micro-optimize raw edges iterator.
thomaswue Jul 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/NodeClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,8 +953,7 @@ public boolean hasNext() {

private Node forward() {
while (mask != 0) {
Node next = getInput();
mask = advanceInput();
Node next = getAndAdvanceInput();
if (next != null) {
return next;
}
Expand All @@ -978,47 +977,45 @@ public Node next() {
}
}

public final long advanceInput() {
int state = (int) mask & 0x03;
private Node getAndAdvanceInput() {
long state = mask & 0x03;
Node result;
if (state == 0) {
// Skip normal field.
return mask >>> NEXT_EDGE;
result = Edges.getNodeUnsafe(node, mask & 0xFC);
mask = mask >>> NEXT_EDGE;
} else if (state == 1) {
// We are iterating a node list.
NodeList<?> nodeList = Edges.getNodeListUnsafe(node, mask & 0xFC);
result = nodeList.nodes[nodeList.size() - 1 - (int) ((mask >>> NEXT_EDGE) & 0xFFFF)];
if ((mask & 0xFFFF00) != 0) {
// Node list count is non-zero, decrease by 1.
return mask - 0x100;
mask = mask - 0x100;
} else {
// Node list is finished => go to next input.
return mask >>> 24;
mask = mask >>> 24;
}
} else {
// Need to expand node list.
// Node list needs to expand first.
result = null;
NodeList<?> nodeList = Edges.getNodeListUnsafe(node, mask & 0xFC);
if (nodeList != null) {
int size = nodeList.size();
if (size != 0) {
// Set pointer to upper most index of node list.
return ((mask >>> NEXT_EDGE) << 24) | (mask & 0xFD) | ((long) (size - 1) << NEXT_EDGE);
int size;
if (nodeList != null && ((size = nodeList.size()) != 0)) {
// Set pointer to upper most index of node list.
mask = ((mask >>> NEXT_EDGE) << 24) | (mask & 0xFD) | ((long) (size - 1) << NEXT_EDGE);
result = nodeList.nodes[size - 1 - (int) ((mask >>> NEXT_EDGE) & 0xFFFF)];
if ((mask & 0xFFFF00) != 0) {
// Node list count is non-zero, decrease by 1.
mask = mask - 0x100;
} else {
// Node list is finished => go to next input.
mask = mask >>> 24;
}
} else {
// Node list is empty or null => skip.
mask = mask >>> NEXT_EDGE;
}
// Node list is empty or null => skip.
return mask >>> NEXT_EDGE;
}
}

public Node getInput() {
int state = (int) mask & 0x03;
if (state == 0) {
return Edges.getNodeUnsafe(node, mask & 0xFC);
} else if (state == 1) {
// We are iterating a node list.
NodeList<?> nodeList = Edges.getNodeListUnsafe(node, mask & 0xFC);
return nodeList.nodes[nodeList.size() - 1 - (int) ((mask >>> NEXT_EDGE) & 0xFFFF)];
} else {
// Node list needs to expand first.
return null;
}
return result;
}

@Override
Expand Down
14 changes: 14 additions & 0 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/graph/NodeStack.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ private void grow() {
values = newValues;
}

/**
* Reverse the order of the top n elements of this stack.
*/
public void reverseTopElements(int n) {
if (n > 1) {
for (int i = 0; i < (n >> 1); ++i) {
Node a = values[tos - 1 - i];
Node b = values[tos - n + i];
values[tos - 1 - i] = b;
values[tos - n + i] = a;
}
}
}

public Node get(int index) {
return values[index];
}
Expand Down
99 changes: 44 additions & 55 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/schedule/SchedulePhase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,14 @@ private static void fillKillSet(LocationSet killed, List<Node> subList) {

private static void sortNodesLatestWithinBlock(ControlFlowGraph cfg, BlockMap<List<Node>> earliestBlockToNodesMap, BlockMap<List<Node>> latestBlockToNodesMap, NodeMap<HIRBlock> currentNodeMap,
BlockMap<ArrayList<FloatingReadNode>> watchListMap, NodeBitMap visited, boolean supportsImplicitNullChecks) {
NodeStack nodeStack = new NodeStack();
for (HIRBlock b : cfg.getBlocks()) {
sortNodesLatestWithinBlock(b, earliestBlockToNodesMap, latestBlockToNodesMap, currentNodeMap, watchListMap, visited, supportsImplicitNullChecks);
sortNodesLatestWithinBlock(nodeStack, b, earliestBlockToNodesMap, latestBlockToNodesMap, currentNodeMap, watchListMap, visited, supportsImplicitNullChecks);
}
}

private static void sortNodesLatestWithinBlock(HIRBlock b, BlockMap<List<Node>> earliestBlockToNodesMap, BlockMap<List<Node>> latestBlockToNodesMap, NodeMap<HIRBlock> nodeMap,
private static void sortNodesLatestWithinBlock(NodeStack nodeStack, HIRBlock b, BlockMap<List<Node>> earliestBlockToNodesMap, BlockMap<List<Node>> latestBlockToNodesMap,
NodeMap<HIRBlock> nodeMap,
BlockMap<ArrayList<FloatingReadNode>> watchListMap, NodeBitMap unprocessed, boolean supportsImplicitNullChecks) {
List<Node> earliestSorting = earliestBlockToNodesMap.get(b);
ArrayList<Node> result = new ArrayList<>(earliestSorting.size());
Expand All @@ -499,7 +501,7 @@ private static void sortNodesLatestWithinBlock(HIRBlock b, BlockMap<List<Node>>
// if multiple proxies reference the same value, schedule the value of a
// proxy once
if (value != null && nodeMap.get(value) == b && unprocessed.isMarked(value)) {
sortIntoList(value, b, result, nodeMap, unprocessed, null);
sortIntoList(nodeStack, value, b, result, nodeMap, unprocessed);
}
}
}
Expand All @@ -509,18 +511,19 @@ private static void sortNodesLatestWithinBlock(HIRBlock b, BlockMap<List<Node>>
// Only if the end node is either a control split or an end node, we need to force
// it to be the last node in the schedule.
fixedEndNode = endNode;
unprocessed.clear(fixedEndNode);
}
for (Node n : earliestSorting) {
if (n != fixedEndNode) {
if (n instanceof FixedNode) {
assert nodeMap.get(n) == b : Assertions.errorMessageContext("n", n, "b", b);
checkWatchList(b, nodeMap, unprocessed, result, watchList, n);
sortIntoList(n, b, result, nodeMap, unprocessed, null);
checkWatchList(nodeStack, b, nodeMap, unprocessed, result, watchList, n);
sortIntoList(nodeStack, n, b, result, nodeMap, unprocessed);
} else if (nodeMap.get(n) == b && n instanceof FloatingReadNode) {
FloatingReadNode floatingReadNode = (FloatingReadNode) n;
if (isImplicitNullOpportunity(floatingReadNode, b, supportsImplicitNullChecks)) {
// Schedule at the beginning of the block.
sortIntoList(floatingReadNode, b, result, nodeMap, unprocessed, null);
sortIntoList(nodeStack, floatingReadNode, b, result, nodeMap, unprocessed);
} else {
LocationIdentity location = floatingReadNode.getLocationIdentity();
if (b.canKill(location)) {
Expand All @@ -539,38 +542,41 @@ private static void sortNodesLatestWithinBlock(HIRBlock b, BlockMap<List<Node>>
assert nodeMap.get(n) == b : n;
assert !(n instanceof FixedNode) : n;
if (unprocessed.isMarked(n)) {
sortIntoList(n, b, result, nodeMap, unprocessed, fixedEndNode);
sortIntoList(nodeStack, n, b, result, nodeMap, unprocessed);
}
}

if (endNode != null && unprocessed.isMarked(endNode)) {
sortIntoList(endNode, b, result, nodeMap, unprocessed, null);
if (fixedEndNode != null) {
result.add(fixedEndNode);
} else if (endNode != null && unprocessed.isMarked(endNode)) {
sortIntoList(nodeStack, endNode, b, result, nodeMap, unprocessed);
}

latestBlockToNodesMap.put(b, result);
}

private static void checkWatchList(HIRBlock b, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed, ArrayList<Node> result, ArrayList<FloatingReadNode> watchList, Node n) {
private static void checkWatchList(NodeStack nodeStack, HIRBlock b, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed, ArrayList<Node> result, ArrayList<FloatingReadNode> watchList, Node n) {
if (watchList != null && !watchList.isEmpty()) {
// Check if this node kills a node in the watch list.
if (MemoryKill.isSingleMemoryKill(n)) {
LocationIdentity identity = ((SingleMemoryKill) n).getKilledLocationIdentity();
checkWatchList(watchList, identity, b, result, nodeMap, unprocessed);
checkWatchList(nodeStack, watchList, identity, b, result, nodeMap, unprocessed);
} else if (MemoryKill.isMultiMemoryKill(n)) {
for (LocationIdentity identity : ((MultiMemoryKill) n).getKilledLocationIdentities()) {
checkWatchList(watchList, identity, b, result, nodeMap, unprocessed);
checkWatchList(nodeStack, watchList, identity, b, result, nodeMap, unprocessed);
}
}
}
}

private static void checkWatchList(ArrayList<FloatingReadNode> watchList, LocationIdentity identity, HIRBlock b, ArrayList<Node> result, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed) {
private static void checkWatchList(NodeStack nodeStack, ArrayList<FloatingReadNode> watchList, LocationIdentity identity, HIRBlock b, ArrayList<Node> result, NodeMap<HIRBlock> nodeMap,
NodeBitMap unprocessed) {
if (identity.isImmutable()) {
// Nothing to do. This can happen for an initialization write.
} else if (identity.isAny()) {
for (FloatingReadNode r : watchList) {
if (unprocessed.isMarked(r)) {
sortIntoList(r, b, result, nodeMap, unprocessed, null);
sortIntoList(nodeStack, r, b, result, nodeMap, unprocessed);
}
}
watchList.clear();
Expand All @@ -582,7 +588,7 @@ private static void checkWatchList(ArrayList<FloatingReadNode> watchList, Locati
assert locationIdentity.isMutable();
if (unprocessed.isMarked(r)) {
if (identity.overlaps(locationIdentity)) {
sortIntoList(r, b, result, nodeMap, unprocessed, null);
sortIntoList(nodeStack, r, b, result, nodeMap, unprocessed);
} else {
++index;
continue;
Expand All @@ -595,60 +601,54 @@ private static void checkWatchList(ArrayList<FloatingReadNode> watchList, Locati
}
}

private static void sortIntoList(Node n, HIRBlock b, ArrayList<Node> result, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed, Node excludeNode) {
private static void sortIntoList(NodeStack stack, Node n, HIRBlock b, ArrayList<Node> result, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed) {
assert unprocessed.isMarked(n) : Assertions.errorMessage(n);
assert nodeMap.get(n) == b : Assertions.errorMessage(n);

if (n instanceof PhiNode) {
return;
}
assert stack.isEmpty() : "Node stack must be pre-allocated, but empty.";
assert !(n instanceof PhiNode) : "Phi nodes will never be sorted into the list.";
assert !(n instanceof ProxyNode) : "Proxy nodes will never be sorted into the list.";

unprocessed.clear(n);

/*
* Schedule all unprocessed transitive inputs. This uses an explicit stack instead of
* recursion to avoid overflowing the call stack.
*/
NodeStack stack = new NodeStack();
ArrayList<Node> tempList = new ArrayList<>();
stack.push(n);
pushUnprocessedInputs(n, b, nodeMap, unprocessed, stack);
while (!stack.isEmpty()) {
Node top = stack.peek();
pushUnprocessedInputs(top, b, nodeMap, unprocessed, excludeNode, stack, tempList);
if (stack.peek() == top) {
if (top != n) {
if (unprocessed.isMarked(top) && !(top instanceof ProxyNode)) {
result.add(top);
}
int added = pushUnprocessedInputs(top, b, nodeMap, unprocessed, stack);
if (added == 0) {
if (unprocessed.isMarked(top)) {
result.add(top);
unprocessed.clear(top);
}
stack.pop();
}
}
result.add(n);
}

if (n instanceof ProxyNode) {
// Skip proxy nodes.
} else {
result.add(n);
private static int pushUnprocessedInputs(Node n, HIRBlock b, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed, NodeStack stack) {
int pushCount = 0;
for (Node input : n.inputs()) {
if (nodeMap.get(input) == b && unprocessed.isMarked(input)) {
assert !(input instanceof PhiNode) : "Phi nodes will always be already unmarked in the bitmap.";
assert !(input instanceof ProxyNode) : "Proxy nodes will always be already unmarked in the bitmap.";
stack.push(input);
pushCount++;
}
}
}

private static void pushUnprocessedInputs(Node n, HIRBlock b, NodeMap<HIRBlock> nodeMap, NodeBitMap unprocessed, Node excludeNode, NodeStack stack, ArrayList<Node> tempList) {
tempList.clear();
n.inputs().snapshotTo(tempList);
/*
* Nodes on top of the stack are scheduled first. Pushing inputs left to right would
* therefore mean scheduling them right to left. We observe the best performance when
* scheduling inputs left to right, therefore we push them in reverse order. We could
* explore more elaborate scheduling policies, like scheduling for reduced register
* pressure using Sethi-Ullman numbering (GR-34624).
*/
for (int i = tempList.size() - 1; i >= 0; i--) {
Node input = tempList.get(i);
if (nodeMap.get(input) == b && unprocessed.isMarked(input) && input != excludeNode && !(input instanceof PhiNode)) {
stack.push(input);
}
}
stack.reverseTopElements(pushCount);
return pushCount;
}

protected void calcLatestBlock(HIRBlock earliestBlock, SchedulingStrategy strategy, Node currentNode, NodeMap<HIRBlock> currentNodeMap, LocationIdentity constrainingLocation,
Expand Down Expand Up @@ -843,18 +843,9 @@ public void add(Node node) {
* Number of nodes in this micro block.
*/
public int getNodeCount() {
assert getActualNodeCount() == nodeCount : getActualNodeCount() + " != " + nodeCount;
return nodeCount;
}

private int getActualNodeCount() {
int count = 0;
for (NodeEntry e = head; e != null; e = e.next) {
count++;
}
return count;
}

/**
* The id of the micro block, with a block always associated with a lower id than its
* successors.
Expand Down Expand Up @@ -940,7 +931,7 @@ private void scheduleEarliestIterative(BlockMap<List<Node>> blockToNodes, NodeMa
}
}

if (graph.getGuardsStage().allowsFloatingGuards() && graph.getNodes(GuardNode.TYPE).isNotEmpty()) {
if (graph.getGuardsStage().allowsFloatingGuards() && graph.hasNode(GuardNode.TYPE)) {
// Now process guards.
if (GuardPriorities.getValue(graph.getOptions()) && withGuardOrder) {
EnumMap<GuardPriority, List<GuardNode>> guardsByPriority = new EnumMap<>(GuardPriority.class);
Expand All @@ -955,8 +946,6 @@ private void scheduleEarliestIterative(BlockMap<List<Node>> blockToNodes, NodeMa
} else {
processNodes(visited, entries, stack, startBlock, graph.getNodes(GuardNode.TYPE));
}
} else {
assert graph.getNodes(GuardNode.TYPE).isEmpty();
}

// Now process inputs of fixed nodes.
Expand Down
Loading