[GR-52763] Loop partial unroll: respect floating div stamps.

PullRequest: graal/17309

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopPartialUnrollPhase.java

@@ -28,6 +28,7 @@
 
 import org.graalvm.collections.EconomicMap;
 import org.graalvm.collections.Equivalence;
+
 import jdk.graal.compiler.debug.DebugContext;
 import jdk.graal.compiler.graph.Graph;
 import jdk.graal.compiler.nodes.GraphState;
@@ -42,6 +43,7 @@
 import jdk.graal.compiler.nodes.spi.LoopsDataProvider;
 import jdk.graal.compiler.phases.common.CanonicalizerPhase;
 import jdk.graal.compiler.phases.common.util.EconomicSetNodeEventListener;
+import jdk.graal.compiler.phases.common.util.LoopUtility;
 
 public class LoopPartialUnrollPhase extends LoopPhase<LoopPolicies> {
 
@@ -69,6 +71,7 @@ private void unroll(StructuredGraph graph, CoreProviders context) {
  if (loop.loopBegin().isSimpleLoop()) {
  // First perform the pre/post transformation and do the partial
  // unroll when we come around again.
+ LoopUtility.preserveCounterStampsForDivAfterUnroll(loop);
  LoopTransformations.insertPrePostLoops(loop);
  prePostInserted = true;
  changed = true;

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/phases/common/util/LoopUtility.java

@@ -29,22 +29,28 @@
 import jdk.graal.compiler.core.common.cfg.Loop;
 import jdk.graal.compiler.core.common.type.IntegerStamp;
 import jdk.graal.compiler.core.common.type.Stamp;
+import jdk.graal.compiler.debug.DebugContext;
+import jdk.graal.compiler.debug.GraalError;
 import jdk.graal.compiler.graph.Graph.NodeEvent;
 import jdk.graal.compiler.graph.Graph.NodeEventScope;
 import jdk.graal.compiler.graph.Node;
+import jdk.graal.compiler.nodes.AbstractBeginNode;
 import jdk.graal.compiler.nodes.FixedNode;
 import jdk.graal.compiler.nodes.GraphState.StageFlag;
 import jdk.graal.compiler.nodes.LoopExitNode;
 import jdk.graal.compiler.nodes.NodeView;
+import jdk.graal.compiler.nodes.PiNode;
 import jdk.graal.compiler.nodes.ProxyNode;
 import jdk.graal.compiler.nodes.StructuredGraph;
 import jdk.graal.compiler.nodes.ValueNode;
 import jdk.graal.compiler.nodes.ValuePhiNode;
 import jdk.graal.compiler.nodes.calc.AddNode;
+import jdk.graal.compiler.nodes.calc.FloatingIntegerDivRemNode;
 import jdk.graal.compiler.nodes.calc.IntegerConvertNode;
 import jdk.graal.compiler.nodes.calc.MulNode;
 import jdk.graal.compiler.nodes.cfg.ControlFlowGraph;
 import jdk.graal.compiler.nodes.cfg.HIRBlock;
+import jdk.graal.compiler.nodes.extended.OpaqueValueNode;
 import jdk.graal.compiler.nodes.loop.BasicInductionVariable;
 import jdk.graal.compiler.nodes.loop.InductionVariable;
 import jdk.graal.compiler.nodes.loop.LoopEx;
@@ -166,4 +172,55 @@ public static void stepLoopIVs(StructuredGraph graph, LoopEx loop, ValueNode ite
  phi.setValueAt(0, graph.addOrUniqueWithInputs(steppedInit));
  }
  }
+
+ /**
+ * Ensure that floating div nodes are correct and can be correctly verified after unrolling.
+ *
+ * A loop variant floating div node means the body of the loop guarantees that the div cannot
+ * trap. This guarantee is encoded in the stamps of the div inputs. Whatever iteration space the
+ * loop has, the div will not trap.
+ *
+ * Unrolling a loop does not change the iteration space of a loop nor the values used in the
+ * loop body, it just affects the backedge jump frequency. Thus, any div floating and valid to
+ * be floating before unrolling must be so after unrolling. However, unrolling copies versions
+ * of the loop body which affects stamp computation. The original stamps of loop phis can be set
+ * by various optimizations. After unrolling we may not have enough context information about
+ * the loop to deduce no trap can happen for the values inside the loop. This is a shortcoming
+ * in our stamp system where we do not connect the max trip count of a loop to the inferred
+ * stamp of an arithmetic operation. Thus, we manually inject the original stamps via pi nodes
+ * into the unrolled versions. This ensures the divs verify correctly.
+ */
+ public static void preserveCounterStampsForDivAfterUnroll(LoopEx loop) {
+ for (Node n : loop.inside().nodes()) {
+ if (n instanceof FloatingIntegerDivRemNode<?> idiv) {
+
+ StructuredGraph graph = idiv.graph();
+
+ ValueNode divisor = idiv.getY();
+ IntegerStamp divisorStamp = (IntegerStamp) divisor.stamp(NodeView.DEFAULT);
+ ValueNode dividend = idiv.getX();
+ IntegerStamp dividendStamp = (IntegerStamp) dividend.stamp(NodeView.DEFAULT);
+
+ GraalError.guarantee(!divisorStamp.contains(0), "Divisor stamp must not contain 0 for floating divs - that could trap %s", idiv);
+
+ boolean xInsideLoop = !loop.isOutsideLoop(dividend);
+ boolean yInsideLoop = !loop.isOutsideLoop(divisor);
+
+ if (yInsideLoop) {
+ idiv.setY(piAnchorBeforeLoop(graph, divisor, divisorStamp, loop));
+ }
+ if (xInsideLoop) {
+ idiv.setX(piAnchorBeforeLoop(graph, dividend, dividendStamp, loop));
+ }
+ }
+ }
+ loop.invalidateFragmentsAndIVs();
+ loop.loopBegin().getDebug().dump(DebugContext.VERY_DETAILED_LEVEL, loop.loopBegin().graph(), "After preserving idiv stamps");
+ }
+
+ private static PiNode piAnchorBeforeLoop(StructuredGraph graph, ValueNode v, Stamp s, LoopEx loop) {
+ ValueNode opaqueDivisor = graph.addWithoutUnique(new OpaqueValueNode(v));
+ // just anchor the pi before the loop, that dominates the other input
+ return graph.addWithoutUnique(new PiNode(opaqueDivisor, s, AbstractBeginNode.prevBegin(loop.loopBegin().forwardEnd())));
+ }
 }