Skip to content

Commit

Permalink
[GR-62401] Recompute CFG in peeling during mid tier peeling with floa…
Browse files Browse the repository at this point in the history 
…ting guards.

PullRequest: graal/20145
  • Loading branch information
@davleopo
davleopo committed Feb 27, 2025
2 parents 9c1cee2 + 2c73b4c commit 1bf3f76
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 23 deletions.
87 changes: 73 additions & 14 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopPeelingPhase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,16 @@
*/
package jdk.graal.compiler.loop.phases;

import java.util.ArrayList;
import java.util.Optional;

import jdk.graal.compiler.debug.Assertions;
import jdk.graal.compiler.debug.DebugContext;
import jdk.graal.compiler.graph.Graph.Mark;
import jdk.graal.compiler.graph.Graph.NodeEventScope;
import jdk.graal.compiler.nodes.GraphState;
import jdk.graal.compiler.nodes.GraphState.StageFlag;
import jdk.graal.compiler.nodes.LoopBeginNode;
import jdk.graal.compiler.nodes.StructuredGraph;
import jdk.graal.compiler.nodes.loop.Loop;
import jdk.graal.compiler.nodes.loop.LoopPolicies;
Expand All @@ -38,13 +43,17 @@
import jdk.graal.compiler.options.OptionKey;
import jdk.graal.compiler.options.OptionType;
import jdk.graal.compiler.phases.common.CanonicalizerPhase;
import jdk.graal.compiler.phases.common.util.EconomicSetNodeEventListener;
import jdk.graal.compiler.phases.util.GraphOrder;

public class LoopPeelingPhase extends LoopPhase<LoopPolicies> {

public static class Options {
// @formatter:off
@Option(help = "Allow iterative peeling of loops up to this many times (each time the peeling phase runs).", type = OptionType.Debug)
public static final OptionKey<Integer> IterativePeelingLimit = new OptionKey<>(2);
@Option(help = "Run the canonicalizer incrementally between iterative peelings to improve the heuristics.", type = OptionType.Debug)
public static final OptionKey<Boolean> IncrementalCanonDuringPeel = new OptionKey<>(true);
// @formatter:on
}

Expand Down Expand Up @@ -79,29 +88,79 @@ private static boolean stateAllowsPeeling(GraphState graphState) {
@Override
@SuppressWarnings({"try", "deprecation"})
protected void run(StructuredGraph graph, CoreProviders context) {
DebugContext debug = graph.getDebug();
if (graph.hasLoops()) {
LoopsData data = context.getLoopsDataProvider().getLoopsData(graph);
boolean shouldPeelAlot = LoopPolicies.Options.PeelALot.getValue(graph.getOptions());
int shouldPeelOnly = LoopPolicies.Options.PeelOnlyLoopWithNodeID.getValue(graph.getOptions());
try (DebugContext.Scope s = debug.scope("peeling", data.getCFG())) {
final boolean shouldPeelAlot = LoopPolicies.Options.PeelALot.getValue(graph.getOptions());
final int shouldPeelOnly = LoopPolicies.Options.PeelOnlyLoopWithNodeID.getValue(graph.getOptions());
final boolean incrementalCanon = Options.IncrementalCanonDuringPeel.getValue(graph.getOptions());
/*
* When guards are floating we need to update the CFG on every peeling operation because
* answering the question which floating guards are inside a loop potentially need cfg
* blocks for anchors which can change. See
* "NOTE: Guard Handling: Referenced by loop phases" in LoopFragment.java .
*/
final boolean floatingGuardsNeedCFGUpdates = graph.getGuardsStage().allowsFloatingGuards();

/*
* Given that we potentially have to recompute the loops data between peeling iterations we
* design peeling the following way: We iterate all loops and decide for them if we want to
* peel them. If so, we peel them and compute the cfg in between if necessary, then redo the
* same logic again.
*/

// we use a list to preserve the outer first order
ArrayList<LoopBeginNode> toPeel = new ArrayList<>();
EconomicSetNodeEventListener ec = new EconomicSetNodeEventListener();
for (int iteration = 0; iteration < Options.IterativePeelingLimit.getValue(graph.getOptions()); iteration++) {
try (NodeEventScope s = graph.trackNodeEvents(ec)) {
LoopsData data = context.getLoopsDataProvider().getLoopsData(graph);
// record the shape of the graph before peeling
Mark before = graph.getMark();
toPeel.clear();
for (Loop loop : data.outerFirst()) {
if (canPeel(loop)) {
for (int iteration = 0; iteration < Options.IterativePeelingLimit.getValue(graph.getOptions()); iteration++) {
if ((shouldPeelAlot || getPolicies().shouldPeel(loop, data.getCFG(), context, iteration)) &&
(shouldPeelOnly == -1 || shouldPeelOnly == loop.loopBegin().getId())) {
LoopTransformations.peel(loop);
if (!canPeel(loop)) {
continue;
}
final boolean peelOnlyIDDisabledOrMatches = shouldPeelOnly == -1 || shouldPeelOnly == loop.loopBegin().getId();
if (!peelOnlyIDDisabledOrMatches) {
continue;
}
if (shouldPeelAlot || getPolicies().shouldPeel(loop, data.getCFG(), context, iteration)) {
toPeel.add(loop.loopBegin());
}
}
if (!before.isCurrent()) {
/*
* Peeling heuristics may create overflow guards of loops and run counted loop
* detection. If that happens and we added additional nodes we need to reset the
* loop fragments because additional code can be reachable in the loop body.
*/
for (Loop l : data.loops()) {
l.invalidateFragmentsAndIVs();
}
}
for (LoopBeginNode marked : toPeel) {
for (Loop loop : data.loops()) {
if (loop.loopBegin() == marked) {
graph.getDebug().dump(DebugContext.VERY_DETAILED_LEVEL, graph, "Before peeling loop %s", loop);
LoopTransformations.peel(loop);
if (floatingGuardsNeedCFGUpdates) {
data = context.getLoopsDataProvider().getLoopsData(graph);
} else {
loop.invalidateFragmentsAndIVs();
data.getCFG().updateCachedLocalLoopFrequency(loop.loopBegin(), f -> f.decrementFrequency(1.0));
debug.dump(DebugContext.VERBOSE_LEVEL, graph, "After peeling loop %s", loop);
}
graph.getDebug().dump(DebugContext.VERY_DETAILED_LEVEL, graph, "After peeling loop %s", loop);
if (Assertions.detailedAssertionsEnabled(graph.getOptions())) {
assert GraphOrder.assertSchedulableGraph(graph);
}
}
}
}
data.deleteUnusedNodes();
} catch (Throwable t) {
throw debug.handle(t);
}
if (incrementalCanon) {
canonicalizer.applyIncremental(graph, context, ec.getNodes());
}
ec.getNodes().clear();
}
}

Expand Down
39 changes: 30 additions & 9 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/loop/LoopFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,23 +449,44 @@ private static void markFloating(WorkQueue workList, Loop loop, Node start, Node
workList.pop();
boolean isLoopNode = currentEntry.isLoopNode;
Node current = currentEntry.n;
if (!isLoopNode && current instanceof GuardNode && !current.hasUsages()) {
GuardNode guard = (GuardNode) current;
// NOTE: Guard Handling: Referenced by loop phases.
if (!isLoopNode && current instanceof GuardNode guard && !current.hasUsages()) {
if (isLoopNode(guard.getCondition(), loopNodes, nonLoopNodes) != TriState.FALSE) {
ValueNode anchor = guard.getAnchor().asNode();
TriState isAnchorInLoop = isLoopNode(anchor, loopNodes, nonLoopNodes);
if (isAnchorInLoop != TriState.FALSE) {
if (!(anchor instanceof LoopExitNode && ((LoopExitNode) anchor).loopBegin() == loopBeginNode)) {
// It is undecidable whether the node is in the loop or not. This is
// not an issue for getting counted loop information,
// but causes issues when using the information for actual loop
// transformations. This is why a loop transformation must
// not happen while guards are floating.
/*
* Floating guards are treated specially in Graal. They are the only
* floating nodes that survive compilation without usages. If we
* have such floating guards they are normally not considered to be
* part of a loop if they have no usages. If they have usages inside
* the loop they will be naturally pulled inside the loop. If
* however only the condition of the guard is inside the loop we
* could schedule it outside the loop. It is necessary to schedule
* floating guards early however for correctness. Thus, we include
* guards as aggressively inside loops as possible since we "assume"
* they are scheduled later inside a loop because that would
* resemble their EARLY location.
*/
isLoopNode = true;
}
} else if (cfg.blockFor(anchor).strictlyDominates(cfg.blockFor(loopBeginNode))) {
// The anchor is above the loop. The no-usage guard can potentially be
// scheduled inside the loop.
/*
* The anchor dominates the loop, it could be the guard would be
* scheduled inside the loop. Be pessimistic an also always include it
* in the loop.
*/
isLoopNode = true;
}
}
if (!isLoopNode) {
/*
* If anchor or condition are inside a loop the guard should be in. We
* schedule guards as early as possible for correctness.
*/
if (isLoopNode(guard.getAnchor().asNode(), loopNodes, nonLoopNodes) == TriState.TRUE ||
isLoopNode(guard.getCondition(), loopNodes, nonLoopNodes) == TriState.TRUE) {
isLoopNode = true;
}
}
Expand Down

0 comments on commit 1bf3f76

Please sign in to comment.