Skip to content

Commit 5e91993

Browse files
davleopodavleopo
committed
[GR-59003] [GR-59366] Safepoint refactorings.
PullRequest: graal/19100
2 parents c8d4181 + c0405ac commit 5e91993

File tree

11 files changed

+319
-111
lines changed

11 files changed

+319
-111
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/LoopSafepointStateVerificationTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import jdk.graal.compiler.nodes.LoopExitNode;
4444
import jdk.graal.compiler.nodes.PhiNode;
4545
import jdk.graal.compiler.nodes.StructuredGraph;
46+
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
4647
import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
4748
import jdk.graal.compiler.nodes.loop.Loop;
4849
import jdk.graal.compiler.nodes.util.GraphUtil;
@@ -98,8 +99,8 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
9899
@Override
99100
protected void run(StructuredGraph graph, HighTierContext context) {
100101
for (LoopBeginNode lb : graph.getNodes(LoopBeginNode.TYPE)) {
101-
lb.disableSafepoint();
102-
lb.disableGuestSafepoint();
102+
lb.disableSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
103+
lb.disableGuestSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
103104
}
104105
}
105106

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/core/test/deopt/MonitorDeoptTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import jdk.graal.compiler.nodes.FixedWithNextNode;
4040
import jdk.graal.compiler.nodes.LoopBeginNode;
4141
import jdk.graal.compiler.nodes.StructuredGraph;
42+
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
4243
import jdk.graal.compiler.nodes.StructuredGraph.AllowAssumptions;
4344

4445
public final class MonitorDeoptTest extends GraalCompilerTest {
@@ -180,7 +181,7 @@ private static LoopBeginNode findFirstLoop(StructuredGraph graph) {
180181
*/
181182
private static void removeLoopSafepoint(StructuredGraph graph) {
182183
LoopBeginNode loopBegin = findFirstLoop(graph);
183-
loopBegin.disableSafepoint();
184+
loopBegin.disableSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
184185
}
185186

186187
@Test

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/java/BytecodeParser.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@
334334
import jdk.graal.compiler.nodes.IfNode;
335335
import jdk.graal.compiler.nodes.InliningLog;
336336
import jdk.graal.compiler.nodes.InliningLog.PlaceholderInvokable;
337+
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
337338
import jdk.graal.compiler.nodes.Invokable;
338339
import jdk.graal.compiler.nodes.Invoke;
339340
import jdk.graal.compiler.nodes.InvokeNode;
@@ -3853,8 +3854,8 @@ private LoopBeginNode appendLoopBegin(FixedWithNextNode fixedWithNext, int start
38533854
EndNode preLoopEnd = graph.add(new EndNode());
38543855
LoopBeginNode loopBegin = graph.add(new LoopBeginNode());
38553856
if (disableLoopSafepoint()) {
3856-
loopBegin.disableSafepoint();
3857-
loopBegin.disableGuestSafepoint();
3857+
loopBegin.disableSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
3858+
loopBegin.disableGuestSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
38583859
}
38593860
fixedWithNext.setNext(preLoopEnd);
38603861
// Add the single non-loop predecessor of the loop header.

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopSafepointEliminationPhase.java‎

Lines changed: 157 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@
3232
import jdk.graal.compiler.nodes.FixedNode;
3333
import jdk.graal.compiler.nodes.GraphState;
3434
import jdk.graal.compiler.nodes.Invoke;
35+
import jdk.graal.compiler.nodes.LoopBeginNode.SafepointState;
3536
import jdk.graal.compiler.nodes.LoopEndNode;
3637
import jdk.graal.compiler.nodes.NodeView;
3738
import jdk.graal.compiler.nodes.StructuredGraph;
39+
import jdk.graal.compiler.nodes.cfg.ControlFlowGraph;
3840
import jdk.graal.compiler.nodes.cfg.HIRBlock;
3941
import jdk.graal.compiler.nodes.extended.ForeignCall;
4042
import jdk.graal.compiler.nodes.loop.InductionVariable;
@@ -49,7 +51,6 @@
4951
import jdk.vm.ci.meta.ResolvedJavaMethod;
5052

5153
public class LoopSafepointEliminationPhase extends BasePhase<MidTierContext> {
52-
5354
public static class Options {
5455
//@formatter:off
5556
@Option(help = "Removes safepoints on counted loop ends.", type = OptionType.Expert)
@@ -59,33 +60,7 @@ public static class Options {
5960

6061
private static final long IntegerRangeDistance = NumUtil.unsafeAbs((long) Integer.MAX_VALUE - (long) Integer.MIN_VALUE);
6162

62-
/**
63-
* To be implemented by subclasses to perform additional checks. Returns <code>true</code> if
64-
* the safepoint was also disabled in subclasses and we therefore don't need to continue
65-
* traversing.
66-
*/
67-
@SuppressWarnings("unused")
68-
protected boolean onCallInLoop(LoopEndNode loopEnd, FixedNode currentCallNode) {
69-
return true;
70-
}
71-
72-
/**
73-
* To be implemented by subclasses to compute additional fields.
74-
*/
75-
@SuppressWarnings("unused")
76-
protected void onSafepointDisabledLoopBegin(Loop loop) {
77-
}
78-
79-
/**
80-
* Determines whether guest safepoints should be allowed at all. To be implemented by
81-
* subclasses. The default implementation returns <code>false</code>, leading to guest
82-
* safepoints being disabled for all loops in the graph.
83-
*/
84-
protected boolean allowGuestSafepoints() {
85-
return false;
86-
}
87-
88-
public static boolean loopIsIn32BitRange(Loop loop) {
63+
public static boolean iterationRangeIsIn32Bit(Loop loop) {
8964
if (loop.counted().getStamp().getBits() <= 32) {
9065
return true;
9166
}
@@ -126,12 +101,143 @@ public Optional<NotApplicable> notApplicableTo(GraphState graphState) {
126101
}
127102

128103
@Override
129-
protected final void run(StructuredGraph graph, MidTierContext context) {
130-
LoopsData loops = context.getLoopsDataProvider().getLoopsData(graph);
131-
if (Options.RemoveLoopSafepoints.getValue(graph.getOptions())) {
104+
protected void run(StructuredGraph graph, MidTierContext context) {
105+
new Instance(graph, context).optimizeSafepoints();
106+
}
107+
108+
protected static class Instance {
109+
private final StructuredGraph graph;
110+
private final MidTierContext context;
111+
112+
protected Instance(StructuredGraph graph, MidTierContext context) {
113+
this.graph = graph;
114+
this.context = context;
115+
}
116+
117+
/**
118+
* Disable safepoints on loop ends by body nodes. If there is a node that is a guaranteed
119+
* safepoint (like a call) we can disable the safepoint on the dominated loop end because
120+
* there will be a safepoint poll done already on every iteration of the loop.
121+
*
122+
* So for a pattern
123+
*
124+
* <pre>
125+
* while (header) {
126+
* if (someCondition1) {
127+
* call1();
128+
* continue;
129+
* }
130+
* code();
131+
* if (someCondition2) {
132+
* call2();
133+
* continue;
134+
* }
135+
* restOfBody();
136+
* }
137+
* </pre>
138+
*
139+
* we know that both {@code continue} backedges are dominated by a call. {@code call1} and
140+
* {@code call2} dominate the backedges. We can remove safepoint polls from both of them
141+
* because the call will be a guaranteed safepoint.
142+
*/
143+
private int disableSafepointsByBodyNodes(Loop loop, ControlFlowGraph cfg) {
144+
int loopEndSafepointsDisabled = 0;
145+
for (LoopEndNode loopEnd : loop.loopBegin().loopEnds()) {
146+
HIRBlock b = cfg.blockFor(loopEnd);
147+
blocks: while (b != loop.getCFGLoop().getHeader()) {
148+
assert b != null;
149+
for (FixedNode node : b.getNodes()) {
150+
boolean canDisableSafepoint = canDisableSafepoint(node, context);
151+
boolean disabledInSubclass = onCallInLoop(loopEnd, node);
152+
if (canDisableSafepoint) {
153+
loopEnd.disableSafepoint();
154+
graph.getOptimizationLog().report(LoopSafepointEliminationPhase.class, "SafepointElimination", loop.loopBegin());
155+
loopEndSafepointsDisabled++;
156+
/*
157+
* we can only stop if subclasses also say we can stop iterating blocks
158+
*/
159+
if (disabledInSubclass) {
160+
break blocks;
161+
}
162+
}
163+
}
164+
b = b.getDominator();
165+
}
166+
}
167+
return loopEndSafepointsDisabled;
168+
}
169+
170+
public void optimizeSafepoints() {
171+
final boolean optimisticallyRemoveLoopSafepoints = Options.RemoveLoopSafepoints.getValue(graph.getOptions());
172+
173+
LoopsData loops = context.getLoopsDataProvider().getLoopsData(graph);
132174
loops.detectCountedLoops();
133-
for (Loop loop : loops.countedLoops()) {
134-
if (loop.getCFGLoop().getChildren().isEmpty() && (loop.loopBegin().isPreLoop() || loop.loopBegin().isPostLoop() || loopIsIn32BitRange(loop) || loop.loopBegin().isStripMinedInner())) {
175+
176+
for (Loop loop : loops.loops()) {
177+
if (!allowGuestSafepoints()) {
178+
loop.loopBegin().disableGuestSafepoint(SafepointState.MUST_NEVER_SAFEPOINT);
179+
}
180+
int loopEndSafepointsDisabled = disableSafepointsByBodyNodes(loop, loops.getCFG());
181+
final boolean allLoopEndSafepointsDisabled = loopEndSafepointsDisabled == loop.loopBegin().getLoopEndCount();
182+
if (!allLoopEndSafepointsDisabled && optimisticallyRemoveLoopSafepoints) {
183+
if (optimizeSafepointsForCountedLoop(loop)) {
184+
/*
185+
* We removed all loop end safepoints if we do it optimistically for the
186+
* entire loop.
187+
*/
188+
loopEndSafepointsDisabled = loop.loopBegin().getLoopEndCount();
189+
}
190+
}
191+
final boolean allLoopEndSafepointsEnabled = loopEndSafepointsDisabled == 0;
192+
if (allLoopEndSafepointsEnabled) {
193+
/*
194+
* Only if ALL paths through the loop are guaranteed to safepoint we can drop
195+
* the exit safepoint. If there is any path left that does not safepoint we
196+
* could be only executing that path and then we need an exit safepoint.
197+
*/
198+
loop.loopBegin().disableLoopExitSafepoint(SafepointState.OPTIMIZER_DISABLED);
199+
}
200+
201+
}
202+
loops.deleteUnusedNodes();
203+
}
204+
205+
/**
206+
* Determines whether guest safepoints should be allowed at all. To be implemented by
207+
* subclasses. The default implementation returns <code>false</code>, leading to guest
208+
* safepoints being disabled for all loops in the graph.
209+
*/
210+
protected boolean allowGuestSafepoints() {
211+
return false;
212+
}
213+
214+
/**
215+
* To be implemented by subclasses to perform additional checks. Returns <code>true</code>
216+
* if the safepoint was also disabled in subclasses and we therefore don't need to continue
217+
* traversing.
218+
*/
219+
@SuppressWarnings("unused")
220+
protected boolean onCallInLoop(LoopEndNode loopEnd, FixedNode currentCallNode) {
221+
return true;
222+
}
223+
224+
/**
225+
* To be implemented by subclasses to compute additional fields.
226+
*/
227+
@SuppressWarnings("unused")
228+
protected void onSafepointDisabledLoopBegin(Loop loop) {
229+
}
230+
231+
/**
232+
* Tries to optimize away safepoints for the given counted loop completely. We have not been
233+
* able to remove safepoints from the loop ends of the given loop yet. However, the
234+
* optimizer may believe this loop is short running enough to remove safepoints.
235+
*/
236+
private boolean optimizeSafepointsForCountedLoop(Loop loop) {
237+
if (loop.isCounted()) {
238+
if (loop.getCFGLoop().getChildren().isEmpty() &&
239+
(loop.loopBegin().isPreLoop() || loop.loopBegin().isPostLoop() || loopIsIn32BitRange(loop) ||
240+
loop.loopBegin().isStripMinedInner())) {
135241
boolean hasSafepoint = false;
136242
for (LoopEndNode loopEnd : loop.loopBegin().loopEnds()) {
137243
hasSafepoint |= loopEnd.canSafepoint();
@@ -144,51 +250,37 @@ protected final void run(StructuredGraph graph, MidTierContext context) {
144250
if (allowsLoopLimitChecks && allowsFloatingGuards) {
145251
loop.counted().createOverFlowGuard();
146252
} else {
147-
// Cannot disable this safepoint, because the loop could overflow.
148-
continue;
253+
/*
254+
* Cannot disable this safepoint, because the loop could overflow.
255+
*/
256+
return false;
149257
}
150258
}
151-
loop.loopBegin().disableSafepoint();
259+
loop.loopBegin().disableSafepoint(SafepointState.OPTIMIZER_DISABLED);
152260
if (loop.loopBegin().isStripMinedInner()) {
153-
// graal strip mined this loop, trust the heuristics and remove the
154-
// inner
155-
// loop safepoint
156-
loop.loopBegin().disableGuestSafepoint();
261+
/*
262+
* graal strip mined this loop, trust the heuristics and remove the
263+
* inner loop safepoint
264+
*/
265+
loop.loopBegin().disableGuestSafepoint(SafepointState.OPTIMIZER_DISABLED);
157266
} else {
158-
// let the shape of the loop decide whether a guest safepoint is needed
267+
/*
268+
* let the shape of the loop decide whether a guest safepoint is needed
269+
*/
159270
onSafepointDisabledLoopBegin(loop);
160271
}
161272
graph.getOptimizationLog().report(LoopSafepointEliminationPhase.class, "SafepointElimination", loop.loopBegin());
273+
return true;
162274
}
163275
}
164276
}
277+
return false;
165278
}
166-
for (Loop loop : loops.loops()) {
167-
if (!allowGuestSafepoints()) {
168-
loop.loopBegin().disableGuestSafepoint();
169-
}
170-
for (LoopEndNode loopEnd : loop.loopBegin().loopEnds()) {
171-
HIRBlock b = loops.getCFG().blockFor(loopEnd);
172-
blocks: while (b != loop.getCFGLoop().getHeader()) {
173-
assert b != null;
174-
for (FixedNode node : b.getNodes()) {
175-
boolean canDisableSafepoint = canDisableSafepoint(node, context);
176-
boolean disabledInSubclass = onCallInLoop(loopEnd, node);
177-
if (canDisableSafepoint) {
178-
loopEnd.disableSafepoint();
179-
graph.getOptimizationLog().report(LoopSafepointEliminationPhase.class, "SafepointElimination", loop.loopBegin());
180279

181-
// we can only stop if subclasses also say we can stop iterating blocks
182-
if (disabledInSubclass) {
183-
break blocks;
184-
}
185-
}
186-
}
187-
b = b.getDominator();
188-
}
189-
}
280+
public boolean loopIsIn32BitRange(Loop loop) {
281+
return iterationRangeIsIn32Bit(loop);
190282
}
191-
loops.deleteUnusedNodes();
283+
192284
}
193285

194286
public static boolean canDisableSafepoint(FixedNode node, CoreProviders context) {

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/loop/phases/LoopTransformations.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ private static void processPreLoopPhis(Loop preLoop, LoopExitNode mainLoopCounte
659659
*/
660660
LoopBeginNode preLoopBegin = preLoop.loopBegin();
661661
StructuredGraph graph = preLoopBegin.graph();
662-
for (PhiNode prePhiNode : preLoopBegin.phis()) {
662+
for (PhiNode prePhiNode : preLoopBegin.phis().snapshot()) {
663663
PhiNode postPhiNode = postLoop.getDuplicatedNode(prePhiNode);
664664
PhiNode mainPhiNode = mainLoop.getDuplicatedNode(prePhiNode);
665665

0 commit comments

Comments
 (0)