diff --git a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReassociationTest.java b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReassociationTest.java index a8291cf091d4..043766a01efd 100644 --- a/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReassociationTest.java +++ b/compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/ReassociationTest.java @@ -25,6 +25,11 @@ */ package jdk.graal.compiler.core.test; +import java.util.List; +import java.util.Random; + +import org.junit.Test; + import jdk.graal.compiler.api.directives.GraalDirectives; import jdk.graal.compiler.graph.Node; import jdk.graal.compiler.graph.iterators.FilteredNodeIterable; @@ -32,10 +37,6 @@ import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions; import jdk.graal.compiler.nodes.calc.BinaryArithmeticNode; import jdk.graal.compiler.phases.common.ReassociationPhase; -import org.junit.Test; - -import java.util.List; -import java.util.Random; public class ReassociationTest extends GraalCompilerTest { @@ -429,7 +430,7 @@ public static int testLoopSnippet2() { public static int refLoopSnippet2() { for (int i = 0; i < LENGTH; i++) { - arr[i] = i * i * 8; + arr[i] = i * 8 * i; GraalDirectives.controlFlowAnchor(); } return arr[100]; @@ -446,8 +447,8 @@ private void testReassociateInvariant(String testMethod, String refMethod) { } private void testFinalGraph(String testMethod, String refMethod) { - StructuredGraph expected = getFinalGraph(testMethod); - StructuredGraph actual = getFinalGraph(refMethod); + StructuredGraph actual = getFinalGraph(testMethod); + StructuredGraph expected = getFinalGraph(refMethod); assertEquals(expected, actual); } diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/Loop.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/Loop.java index 2c595a47d232..6e943302d6da 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/Loop.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/Loop.java @@ -251,18 +251,43 @@ public boolean apply(Node n) { } } + /** + * Reassociates loop invariants by pushing loop variant operands further down the operand tree. + * + *
+ * inv2 var inv1 inv2 + * \ / \ / + * inv1 + => var + + * \ / \ / + * + + + *+ * + * Also ensures that loop phis are pushed down the furthest (i.e., used as late as possible) to + * avoid long dependency chains on register level when calculating backedge values: + * + *
+ * inv phi inv var + * \ / \ / + * var + => phi + + * \ / \ / + * + + + *+ */ public boolean reassociateInvariants() { int count = 0; StructuredGraph graph = loopBegin().graph(); InvariantPredicate invariant = new InvariantPredicate(); NodeBitMap newLoopNodes = graph.createNodeBitMap(); + var phis = loopBegin().phis(); for (BinaryArithmeticNode> binary : whole().nodes().filter(BinaryArithmeticNode.class)) { if (!binary.mayReassociate()) { continue; } - ValueNode result = BinaryArithmeticNode.reassociateMatchedValues(binary, invariant, binary.getX(), binary.getY(), NodeView.DEFAULT); + // pushing down loop variants will associate loop invariants at the "top" + ValueNode result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, n -> !invariant.apply(n), NodeView.DEFAULT); if (result == binary) { - result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, invariant, NodeView.DEFAULT); + // use loop phis as late as possible to shorten the register dependency chains + result = BinaryArithmeticNode.reassociateUnmatchedValues(binary, n -> n instanceof PhiNode phi && phis.contains(phi), NodeView.DEFAULT); } if (result != binary) { if (!result.isAlive()) { diff --git a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ReassociationPhase.java b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ReassociationPhase.java index e6937c7c79ad..90062f0e83dd 100644 --- a/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ReassociationPhase.java +++ b/compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/ReassociationPhase.java @@ -79,17 +79,25 @@ protected void run(StructuredGraph graph, CoreProviders context) { canonicalizer.applyIncremental(graph, context, changedNodes.getNodes()); } - //@formatter:off /** - * Re-associate loop invariant so that invariant parts of the expression can move outside of the - * loop. + * Re-associate loop invariant so that invariant parts of the expression can move out of the + * loop. For example: * - * For example: - * for (int i = 0; i < LENGTH; i++) { for (int i = 0; i < LENGTH; i++) { - * arr[i] = (i * inv1) * inv2; => arr[i] = i * (inv1 * inv2); - * } } + *
+ * for (int i = 0; i < LENGTH; i++) { + * arr[i] = (i * inv1) * inv2; + * } + *+ * + * is transformed to + * + *
+ * + * for (int i = 0; i < LENGTH; i++) { + * arr[i] = i * (inv1 * inv2); + * } + **/ - //@formatter:on @SuppressWarnings("try") private static void reassociateInvariants(StructuredGraph graph, CoreProviders context) { DebugContext debug = graph.getDebug(); @@ -101,7 +109,11 @@ private static void reassociateInvariants(StructuredGraph graph, CoreProviders c // bound. while (changed && iterations < 32) { changed = false; - for (Loop loop : loopsData.loops()) { + /* + * Process inner loops last to ensure loop phis are used as late as possible. This + * minimizes dependency chains in the backedge value calculation on register level. + */ + for (Loop loop : loopsData.outerFirst()) { changed |= loop.reassociateInvariants(); } loopsData.deleteUnusedNodes();