Skip to content

Commit 95f3bd9

Browse files
teshullteshull
committed
Track extra metadata about DeoptEntry/DeoptProxyAnchor nodes and more aggresssively remove them.
1 parent 8cc2315 commit 95f3bd9

File tree

9 files changed

+257
-107
lines changed

9 files changed

+257
-107
lines changed

‎compiler/src/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BciBlockMapping.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,12 @@ public void getDebugProperties(Map<String, ? super Object> properties) {
616616
}
617617
}
618618

619+
/**
620+
* Represents an entry into an exception handler routine. This block is needed to ensure there
621+
* is a place to put checks that determine whether the handler should be entered (i.e., the
622+
* exception kind is compatible) or execution should continue to check the next handler (or
623+
* unwind).
624+
*/
619625
public static class ExceptionDispatchBlock extends BciBlock {
620626
public final ExceptionHandler handler;
621627
public final int deoptBci;
@@ -1321,7 +1327,7 @@ private void addSuccessor(int predBci, BciBlock sux) {
13211327
}
13221328

13231329
/**
1324-
* Logic for adding an the "normal" invoke successor link.
1330+
* Logic for adding the "normal" invoke successor link.
13251331
*/
13261332
protected void addInvokeNormalSuccessor(int invokeBci, BciBlock sux) {
13271333
addSuccessor(invokeBci, sux);

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntryNode.java‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343

4444
import com.oracle.svm.core.graal.lir.DeoptEntryOp;
4545

46+
import jdk.vm.ci.code.BytecodeFrame;
47+
4648
/**
4749
* A landing-pad for deoptimization. This node is generated in deoptimization target methods for all
4850
* deoptimization entry points.
@@ -53,8 +55,20 @@ public final class DeoptEntryNode extends WithExceptionNode implements DeoptEntr
5355

5456
@OptionalInput(InputType.State) protected FrameState stateAfter;
5557

56-
public DeoptEntryNode() {
58+
private final int proxifiedInvokeBci;
59+
60+
protected DeoptEntryNode(int proxifiedInvokeBci) {
5761
super(TYPE, StampFactory.forVoid());
62+
this.proxifiedInvokeBci = proxifiedInvokeBci;
63+
}
64+
65+
public static DeoptEntryNode create(int proxifiedInvokeBci) {
66+
assert proxifiedInvokeBci != BytecodeFrame.UNKNOWN_BCI;
67+
return new DeoptEntryNode(proxifiedInvokeBci);
68+
}
69+
70+
public static DeoptEntryNode create() {
71+
return new DeoptEntryNode(BytecodeFrame.UNKNOWN_BCI);
5872
}
5973

6074
@Override
@@ -99,4 +113,9 @@ public void setStateAfter(FrameState x) {
99113
public boolean hasSideEffect() {
100114
return true;
101115
}
116+
117+
@Override
118+
public int getProxifiedInvokeBci() {
119+
return proxifiedInvokeBci;
120+
}
102121
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptEntrySupport.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,12 @@
3030
import org.graalvm.compiler.nodes.spi.LIRLowerable;
3131

3232
public interface DeoptEntrySupport extends LIRLowerable, ControlFlowAnchored, FixedNodeInterface, StateSplit {
33+
34+
/**
35+
* Returns the invoke bci this deopt is serving as a proxy anchor for or
36+
* {@link jdk.vm.ci.code.BytecodeFrame#UNKNOWN_BCI} if it is not serving as a proxy anchor for
37+
* an invoke. Note this will be different than the entry's stateafter bci since this node will
38+
* be after the invoke it is "proxifying".
39+
*/
40+
int getProxifiedInvokeBci();
3341
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/nodes/DeoptProxyAnchorNode.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,22 @@
4141
public class DeoptProxyAnchorNode extends AbstractStateSplit implements DeoptEntrySupport {
4242
public static final NodeClass<DeoptProxyAnchorNode> TYPE = NodeClass.create(DeoptProxyAnchorNode.class);
4343

44-
public DeoptProxyAnchorNode() {
45-
this(TYPE);
46-
}
44+
private final int proxifiedInvokeBci;
45+
46+
public DeoptProxyAnchorNode(int proxifiedInvokeBci) {
47+
super(TYPE, StampFactory.forVoid());
48+
assert proxifiedInvokeBci >= 0 : "DeoptProxyAnchorNode should be proxing an invoke";
4749

48-
protected DeoptProxyAnchorNode(NodeClass<? extends DeoptProxyAnchorNode> c) {
49-
super(c, StampFactory.forVoid());
50+
this.proxifiedInvokeBci = proxifiedInvokeBci;
5051
}
5152

5253
@Override
5354
public void generate(NodeLIRBuilderTool gen) {
5455
/* No-op */
5556
}
57+
58+
@Override
59+
public int getProxifiedInvokeBci() {
60+
return proxifiedInvokeBci;
61+
}
5662
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/replacements/SubstrateGraphKit.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ public ValueNode createCFunctionCall(ValueNode targetAddress, List<ValueNode> ar
211211
* exception handlers and directly unwind.
212212
*/
213213
int bci = invoke.stateAfter().bci;
214-
appendWithUnwind(new DeoptEntryNode(), bci);
214+
appendWithUnwind(DeoptEntryNode.create(invoke.bci()), bci);
215215
}
216216

217217
ValueNode result = invoke;

‎substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/ParseOnceRuntimeCompilationFeature.java‎

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderConfiguration;
6060
import org.graalvm.compiler.nodes.graphbuilderconf.IntrinsicContext;
6161
import org.graalvm.compiler.options.OptionValues;
62+
import org.graalvm.compiler.nodes.java.ExceptionObjectNode;
6263
import org.graalvm.compiler.phases.OptimisticOptimizations;
6364
import org.graalvm.compiler.phases.Phase;
6465
import org.graalvm.compiler.phases.PhaseSuite;
@@ -85,6 +86,8 @@
8586
import com.oracle.svm.common.meta.MultiMethod;
8687
import com.oracle.svm.core.config.ConfigurationValues;
8788
import com.oracle.svm.core.graal.nodes.DeoptEntryNode;
89+
import com.oracle.svm.core.graal.nodes.DeoptEntrySupport;
90+
import com.oracle.svm.core.graal.nodes.DeoptProxyAnchorNode;
8891
import com.oracle.svm.core.graal.stackvalue.StackValueNode;
8992
import com.oracle.svm.core.util.VMError;
9093
import com.oracle.svm.graal.GraalSupport;
@@ -909,53 +912,95 @@ public boolean insertPlaceholderParamAndReturnFlows(MultiMethod.MultiMethodKey m
909912
}
910913

911914
/**
912-
* Removes Deoptimizations Entrypoints which are deemed to be unnecessary after the runtime
913-
* compilation methods are optimized.
915+
* Removes {@link DeoptEntryNode}s, {@link DeoptProxyAnchorNode}s, and {@link DeoptProxyNode}s
916+
* which are determined to be unnecessary after the runtime compilation methods are optimized.
914917
*/
915918
static class RemoveDeoptEntries extends Phase {
919+
enum RemovalDecision {
920+
KEEP,
921+
PROXIFY,
922+
REMOVE
923+
}
916924

917925
@Override
918926
protected void run(StructuredGraph graph) {
919-
EconomicMap<StateSplit, Boolean> decisionCache = EconomicMap.create();
927+
EconomicMap<StateSplit, RemovalDecision> decisionCache = EconomicMap.create();
920928

921929
// First go through and delete all unneeded proxies
922930
for (DeoptProxyNode proxyNode : graph.getNodes(DeoptProxyNode.TYPE).snapshot()) {
923931
ValueNode proxyPoint = proxyNode.getProxyPoint();
924932
if (proxyPoint instanceof StateSplit) {
925-
if (proxyPoint instanceof DeoptEntryNode && shouldRemove((StateSplit) proxyPoint, decisionCache)) {
933+
if (getDecision((StateSplit) proxyPoint, decisionCache) == RemovalDecision.REMOVE) {
926934
proxyNode.replaceAtAllUsages(proxyNode.getOriginalNode(), true);
927935
proxyNode.safeDelete();
928936
}
929937
}
930938
}
931939

932-
// Next remove all unneeded DeoptEntryNodes
940+
// Next, remove all unneeded DeoptEntryNodes
933941
for (DeoptEntryNode deoptEntry : graph.getNodes().filter(DeoptEntryNode.class).snapshot()) {
934-
if (shouldRemove(deoptEntry, decisionCache)) {
935-
deoptEntry.killExceptionEdge();
936-
graph.removeSplit(deoptEntry, deoptEntry.getPrimarySuccessor());
942+
switch (getDecision(deoptEntry, decisionCache)) {
943+
case REMOVE -> {
944+
deoptEntry.killExceptionEdge();
945+
graph.removeSplit(deoptEntry, deoptEntry.getPrimarySuccessor());
946+
}
947+
case PROXIFY -> {
948+
deoptEntry.killExceptionEdge();
949+
DeoptProxyAnchorNode newAnchor = graph.add(new DeoptProxyAnchorNode(deoptEntry.getProxifiedInvokeBci()));
950+
newAnchor.setStateAfter(deoptEntry.stateAfter());
951+
graph.replaceSplitWithFixed(deoptEntry, newAnchor, deoptEntry.getPrimarySuccessor());
952+
}
953+
}
954+
}
955+
956+
// Finally, remove all unneeded DeoptProxyAnchorNodes
957+
for (DeoptProxyAnchorNode proxyAnchor : graph.getNodes().filter(DeoptProxyAnchorNode.class).snapshot()) {
958+
if (getDecision(proxyAnchor, decisionCache) == RemovalDecision.REMOVE) {
959+
graph.removeFixed(proxyAnchor);
937960
}
938961
}
939962
}
940963

941-
boolean shouldRemove(StateSplit node, EconomicMap<StateSplit, Boolean> decisionCache) {
942-
Boolean cached = decisionCache.get(node);
964+
RemovalDecision getDecision(StateSplit node, EconomicMap<StateSplit, RemovalDecision> decisionCache) {
965+
RemovalDecision cached = decisionCache.get(node);
943966
if (cached != null) {
944967
return cached;
945968
}
946969

970+
DeoptEntrySupport proxyNode;
971+
if (node instanceof ExceptionObjectNode exceptionObject) {
972+
/*
973+
* For the exception edge of a DeoptEntryNode, we insert the proxies on the
974+
* exception object.
975+
*/
976+
proxyNode = (DeoptEntrySupport) exceptionObject.predecessor();
977+
} else {
978+
proxyNode = (DeoptEntrySupport) node;
979+
}
980+
981+
RemovalDecision decision = RemovalDecision.REMOVE;
947982
var directive = SubstrateCompilationDirectives.singleton();
948-
FrameState state = node.stateAfter();
983+
FrameState state = proxyNode.stateAfter();
949984
HostedMethod method = (HostedMethod) state.getMethod();
985+
if (proxyNode instanceof DeoptEntryNode) {
986+
if (directive.isDeoptEntry(method, state.bci, state.duringCall(), state.rethrowException())) {
987+
decision = RemovalDecision.KEEP;
988+
}
989+
}
950990

951-
boolean result = true;
952-
if (directive.isRegisteredDeoptTarget(method)) {
953-
result = !directive.isDeoptEntry(method, state.bci, state.duringCall(), state.rethrowException());
991+
if (decision == RemovalDecision.REMOVE) {
992+
// now check for any implicit deopt entry being protected against
993+
if (directive.isDeoptEntry(method, proxyNode.getProxifiedInvokeBci(), true, false)) {
994+
decision = proxyNode instanceof DeoptEntryNode ? RemovalDecision.PROXIFY : RemovalDecision.KEEP;
995+
}
954996
}
955997

956998
// cache the decision
957-
decisionCache.put(node, result);
958-
return result;
999+
decisionCache.put(node, decision);
1000+
if (proxyNode != node) {
1001+
decisionCache.put(proxyNode, decision);
1002+
}
1003+
return decision;
9591004
}
9601005
}
9611006
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/PodFactorySubstitutionMethod.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ private static int startMethod(HostedGraphKit kit, DeoptInfoProvider deoptInfo,
174174
if (deoptInfo != null) {
175175
FrameState initialState = kit.getGraph().start().stateAfter();
176176
if (shouldInsertDeoptEntry(deoptInfo, initialState.bci, false, false)) {
177-
return appendDeoptWithExceptionUnwind(kit, initialState, initialState.bci, nextDeoptIndex);
177+
return appendDeoptWithExceptionUnwind(kit, initialState, initialState.bci, nextDeoptIndex, DeoptEntryNode.create());
178178
}
179179
}
180180
return nextDeoptIndex;
@@ -223,7 +223,7 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn
223223
if (shouldInsertDeoptEntry(deoptInfo, bci, false, true)) {
224224
// Exception during invoke
225225

226-
var exceptionDeopt = kit.add(new DeoptEntryNode());
226+
var exceptionDeopt = kit.add(DeoptEntryNode.create(invoke.bci()));
227227
exceptionDeopt.setStateAfter(exception.stateAfter().duplicate());
228228
var exceptionDeoptBegin = kit.add(new DeoptEntryBeginNode());
229229
int exceptionDeoptIndex = nextDeoptIndex++;
@@ -244,7 +244,7 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn
244244
// Deopt entry after invoke without exception
245245

246246
kit.noExceptionPart();
247-
nextDeoptIndex = appendDeoptWithExceptionUnwind(kit, invoke.stateAfter(), invoke.stateAfter().bci, nextDeoptIndex);
247+
nextDeoptIndex = appendDeoptWithExceptionUnwind(kit, invoke.stateAfter(), invoke.stateAfter().bci, nextDeoptIndex, DeoptEntryNode.create(invoke.bci()));
248248
} else {
249249
VMError.guarantee(!shouldInsertDeoptEntry(deoptInfo, bci, true, false), "need to add support for inserting DeoptProxyAnchorNode");
250250
}
@@ -254,8 +254,8 @@ private static int invokeWithDeoptAndExceptionUnwind(HostedGraphKit kit, DeoptIn
254254
}
255255

256256
/** @see com.oracle.svm.hosted.phases.SharedGraphBuilderPhase */
257-
private static int appendDeoptWithExceptionUnwind(HostedGraphKit kit, FrameState state, int exceptionBci, int nextDeoptIndex) {
258-
var entry = kit.add(new DeoptEntryNode());
257+
private static int appendDeoptWithExceptionUnwind(HostedGraphKit kit, FrameState state, int exceptionBci, int nextDeoptIndex, DeoptEntryNode deoptEntryNode) {
258+
var entry = kit.add(deoptEntryNode);
259259
entry.setStateAfter(state.duplicate());
260260
var begin = kit.append(new DeoptEntryBeginNode());
261261
((FixedWithNextNode) begin.predecessor()).setNext(entry);

0 commit comments

Comments
 (0)