Skip to content

Commit 2de29a9

Browse files
committed
[GR-59842] Change loop invariant reassociation to use phis last.
PullRequest: graal/19590
2 parents affd971 + ed49f36 commit 2de29a9

File tree

3 files changed

+56
-18
lines changed

3 files changed

+56
-18
lines changed

compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReassociationTest.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,18 @@
2525
*/
2626
package jdk.graal.compiler.core.test;
2727

28+
import java.util.List;
29+
import java.util.Random;
30+
31+
import org.junit.Test;
32+
2833
import jdk.graal.compiler.api.directives.GraalDirectives;
2934
import jdk.graal.compiler.graph.Node;
3035
import jdk.graal.compiler.graph.iterators.FilteredNodeIterable;
3136
import jdk.graal.compiler.nodes.StructuredGraph;
3237
import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
3338
import jdk.graal.compiler.nodes.calc.BinaryArithmeticNode;
3439
import jdk.graal.compiler.phases.common.ReassociationPhase;
35-
import org.junit.Test;
36-
37-
import java.util.List;
38-
import java.util.Random;
3940

4041
public class ReassociationTest extends GraalCompilerTest {
4142

@@ -429,7 +430,7 @@ public static int testLoopSnippet2() {
429430

430431
public static int refLoopSnippet2() {
431432
for (int i = 0; i < LENGTH; i++) {
432-
arr[i] = i * i * 8;
433+
arr[i] = i * 8 * i;
433434
GraalDirectives.controlFlowAnchor();
434435
}
435436
return arr[100];
@@ -446,8 +447,8 @@ private void testReassociateInvariant(String testMethod, String refMethod) {
446447
}
447448

448449
private void testFinalGraph(String testMethod, String refMethod) {
449-
StructuredGraph expected = getFinalGraph(testMethod);
450-
StructuredGraph actual = getFinalGraph(refMethod);
450+
StructuredGraph actual = getFinalGraph(testMethod);
451+
StructuredGraph expected = getFinalGraph(refMethod);
451452
assertEquals(expected, actual);
452453
}
453454

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,18 +251,43 @@ public boolean apply(Node n) {
251251
}
252252
}
253253

254+
/**
255+
* Reassociates loop invariants by pushing loop variant operands further down the operand tree.
256+
*
257+
* <pre>
258+
* inv2 var inv1 inv2
259+
* \ / \ /
260+
* inv1 + => var +
261+
* \ / \ /
262+
* + +
263+
* </pre>
264+
*
265+
* Also ensures that loop phis are pushed down the furthest (i.e., used as late as possible) to
266+
* avoid long dependency chains on register level when calculating backedge values:
267+
*
268+
* <pre>
269+
* inv phi inv var
270+
* \ / \ /
271+
* var + => phi +
272+
* \ / \ /
273+
* + +
274+
* </pre>
275+
*/
254276
public boolean reassociateInvariants() {
255277
int count = 0;
256278
StructuredGraph graph = loopBegin().graph();
257279
InvariantPredicate invariant = new InvariantPredicate();
258280
NodeBitMap newLoopNodes = graph.createNodeBitMap();
281+
var phis = loopBegin().phis();
259282
for (BinaryArithmeticNode<?> binary : whole().nodes().filter(BinaryArithmeticNode.class)) {
260283
if (!binary.mayReassociate()) {
261284
continue;
262285
}
263-
ValueNode result = BinaryArithmeticNode.reassociateMatchedValues(binary, invariant, binary.getX(), binary.getY(), NodeView.DEFAULT);
286+
// pushing down loop variants will associate loop invariants at the "top"
287+
ValueNode result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, n -> !invariant.apply(n), NodeView.DEFAULT);
264288
if (result == binary) {
265-
result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, invariant, NodeView.DEFAULT);
289+
// use loop phis as late as possible to shorten the register dependency chains
290+
result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, n -> n instanceof PhiNode phi && phis.contains(phi), NodeView.DEFAULT);
266291
}
267292
if (result != binary) {
268293
if (!result.isAlive()) {

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,25 @@ protected void run(StructuredGraph graph, CoreProviders context) {
7979
canonicalizer.applyIncremental(graph, context, changedNodes.getNodes());
8080
}
8181

82-
//@formatter:off
8382
/**
84-
* Re-associate loop invariant so that invariant parts of the expression can move outside of the
85-
* loop.
83+
* Re-associate loop invariant so that invariant parts of the expression can move out of the
84+
* loop. For example:
8685
*
87-
* For example:
88-
* for (int i = 0; i < LENGTH; i++) { for (int i = 0; i < LENGTH; i++) {
89-
* arr[i] = (i * inv1) * inv2; => arr[i] = i * (inv1 * inv2);
90-
* } }
86+
* <pre>
87+
* for (int i = 0; i < LENGTH; i++) {
88+
* arr[i] = (i * inv1) * inv2;
89+
* }
90+
* </pre>
91+
*
92+
* is transformed to
93+
*
94+
* <pre>
95+
*
96+
* for (int i = 0; i < LENGTH; i++) {
97+
* arr[i] = i * (inv1 * inv2);
98+
* }
99+
* </pre>
91100
*/
92-
//@formatter:on
93101
@SuppressWarnings("try")
94102
private static void reassociateInvariants(StructuredGraph graph, CoreProviders context) {
95103
DebugContext debug = graph.getDebug();
@@ -101,7 +109,11 @@ private static void reassociateInvariants(StructuredGraph graph, CoreProviders c
101109
// bound.
102110
while (changed && iterations < 32) {
103111
changed = false;
104-
for (Loop loop : loopsData.loops()) {
112+
/*
113+
* Process inner loops last to ensure loop phis are used as late as possible. This
114+
* minimizes dependency chains in the backedge value calculation on register level.
115+
*/
116+
for (Loop loop : loopsData.outerFirst()) {
105117
changed |= loop.reassociateInvariants();
106118
}
107119
loopsData.deleteUnusedNodes();

0 commit comments

Comments
 (0)