Skip to content

Commit 06736cd

Browse files
author
Christian Wimmer
committed
[GR-34935] Run escape analysis before static analysis.
PullRequest: graal/10255
2 parents 9b5bc2b + 4cd2b1e commit 06736cd

File tree

38 files changed

+500
-258
lines changed

38 files changed

+500
-258
lines changed

‎compiler/src/org.graalvm.compiler.core/src/org/graalvm/compiler/core/gen/DebugInfoBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@
4646
import org.graalvm.compiler.nodes.spi.NodeWithState;
4747
import org.graalvm.compiler.nodes.util.GraphUtil;
4848
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
49+
import org.graalvm.compiler.nodes.virtual.MaterializedObjectState;
4950
import org.graalvm.compiler.nodes.virtual.VirtualBoxingNode;
5051
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;
52+
import org.graalvm.compiler.nodes.virtual.VirtualObjectState;
5153
import org.graalvm.compiler.serviceprovider.GraalServices;
52-
import org.graalvm.compiler.virtual.nodes.MaterializedObjectState;
53-
import org.graalvm.compiler.virtual.nodes.VirtualObjectState;
5454

5555
import jdk.vm.ci.code.BytecodeFrame;
5656
import jdk.vm.ci.code.RegisterValue;

‎compiler/src/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/meta/HotSpotGraphBuilderPlugins.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@
115115
import org.graalvm.compiler.nodes.java.DynamicNewInstanceNode;
116116
import org.graalvm.compiler.nodes.java.InstanceOfNode;
117117
import org.graalvm.compiler.nodes.java.NewArrayNode;
118-
import org.graalvm.compiler.nodes.java.ValidateNewInstanceClassNode;
119118
import org.graalvm.compiler.nodes.memory.OnHeapMemoryAccess.BarrierType;
120119
import org.graalvm.compiler.nodes.memory.address.AddressNode;
121120
import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
@@ -141,7 +140,6 @@
141140
import jdk.vm.ci.code.TargetDescription;
142141
import jdk.vm.ci.hotspot.VMIntrinsicMethod;
143142
import jdk.vm.ci.meta.ConstantReflectionProvider;
144-
import jdk.vm.ci.meta.DeoptimizationAction;
145143
import jdk.vm.ci.meta.JavaKind;
146144
import jdk.vm.ci.meta.MetaAccessProvider;
147145
import jdk.vm.ci.meta.ResolvedJavaField;
@@ -523,8 +521,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
523521
* check. Such a DynamicNewInstanceNode is also never constant folded to a
524522
* NewInstanceNode.
525523
*/
526-
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
527-
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(b.nullCheckedValue(clazzLegal, DeoptimizationAction.None), true));
524+
DynamicNewInstanceNode.createAndPush(b, clazz);
528525
return true;
529526
}
530527
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ public FrameState create(int bci, BytecodeParser parent, boolean duringCall, Jav
334334
outerFrameState = parent.getFrameStateBuilder().create(parent.bci(), parent.getNonIntrinsicAncestor(), true, null, null);
335335
}
336336
if (bci == BytecodeFrame.AFTER_EXCEPTION_BCI && parent != null) {
337-
return outerFrameState.duplicateModified(graph, outerFrameState.bci, true, false, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{stack[0]});
337+
return outerFrameState.duplicateModified(graph, outerFrameState.bci, true, false, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{stack[0]}, null);
338338
}
339339
if (bci == BytecodeFrame.INVALID_FRAMESTATE_BCI) {
340340
throw shouldNotReachHere();

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/FrameState.java‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.graalvm.compiler.nodes.java.ExceptionObjectNode;
5959
import org.graalvm.compiler.nodes.java.MonitorIdNode;
6060
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
61+
import org.graalvm.compiler.nodes.virtual.MaterializedObjectState;
6162
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;
6263

6364
import jdk.vm.ci.code.BytecodeFrame;
@@ -365,24 +366,20 @@ public FrameState duplicateWithVirtualState() {
365366
* the stack.
366367
*/
367368
public FrameState duplicateModifiedDuringCall(int newBci, JavaKind popKind) {
368-
return duplicateModified(graph(), newBci, rethrowException, true, popKind, null, null);
369+
return duplicateModified(graph(), newBci, rethrowException, true, popKind, null, null, null);
369370
}
370371

371-
public FrameState duplicateModifiedBeforeCall(int newBci, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues) {
372-
return duplicateModified(graph(), newBci, rethrowException, false, popKind, pushedSlotKinds, pushedValues);
372+
public FrameState duplicateModifiedBeforeCall(int newBci, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues, List<EscapeObjectState> pushedVirtualObjectMappings) {
373+
return duplicateModified(graph(), newBci, rethrowException, false, popKind, pushedSlotKinds, pushedValues, pushedVirtualObjectMappings);
373374
}
374375

375376
/**
376377
* Creates a copy of this frame state with the top of stack replaced with with
377378
* {@code pushedValue} which must be of type {@code popKind}.
378379
*/
379-
public FrameState duplicateModified(JavaKind popKind, JavaKind pushedSlotKind, ValueNode pushedValue) {
380+
public FrameState duplicateModified(JavaKind popKind, JavaKind pushedSlotKind, ValueNode pushedValue, List<EscapeObjectState> pushedVirtualObjectMappings) {
380381
assert pushedValue != null && pushedValue.getStackKind() == popKind;
381-
return duplicateModified(graph(), bci, rethrowException, duringCall, popKind, new JavaKind[]{pushedSlotKind}, new ValueNode[]{pushedValue});
382-
}
383-
384-
public FrameState duplicateRethrow(ValueNode exceptionObject) {
385-
return duplicateModified(graph(), bci, true, duringCall, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{exceptionObject});
382+
return duplicateModified(graph(), bci, rethrowException, duringCall, popKind, new JavaKind[]{pushedSlotKind}, new ValueNode[]{pushedValue}, pushedVirtualObjectMappings);
386383
}
387384

388385
/**
@@ -391,7 +388,9 @@ public FrameState duplicateRethrow(ValueNode exceptionObject) {
391388
* correctly in slot encoding: a long or double will be followed by a null slot. The bci will be
392389
* changed to newBci.
393390
*/
394-
public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean newRethrowException, boolean newDuringCall, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues) {
391+
public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean newRethrowException, boolean newDuringCall, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues,
392+
List<EscapeObjectState> pushedVirtualObjectMappings) {
393+
List<EscapeObjectState> copiedVirtualObjectMappings = null;
395394
ArrayList<ValueNode> copy;
396395
if (newRethrowException && !rethrowException && popKind == JavaKind.Void) {
397396
assert popKind == JavaKind.Void;
@@ -410,7 +409,11 @@ public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean n
410409
if (pushedValues != null) {
411410
assert pushedSlotKinds.length == pushedValues.length;
412411
for (int i = 0; i < pushedValues.length; i++) {
413-
copy.add(pushedValues[i]);
412+
ValueNode pushedValue = pushedValues[i];
413+
if (pushedValue instanceof VirtualObjectNode) {
414+
copiedVirtualObjectMappings = ensureHasVirtualObjectMapping((VirtualObjectNode) pushedValue, pushedVirtualObjectMappings, copiedVirtualObjectMappings);
415+
}
416+
copy.add(pushedValue);
414417
if (pushedSlotKinds[i].needsTwoSlots()) {
415418
copy.add(null);
416419
}
@@ -420,7 +423,56 @@ public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean n
420423
copy.addAll(values.subList(localsSize + stackSize, values.size()));
421424

422425
assert checkStackDepth(bci, stackSize, duringCall, rethrowException, newBci, newStackSize, newDuringCall, newRethrowException);
423-
return graph.add(new FrameState(outerFrameState(), code, newBci, copy, localsSize, newStackSize, newRethrowException, newDuringCall, monitorIds, virtualObjectMappings));
426+
return graph.add(new FrameState(outerFrameState(), code, newBci, copy, localsSize, newStackSize, newRethrowException, newDuringCall, monitorIds,
427+
copiedVirtualObjectMappings != null ? copiedVirtualObjectMappings : virtualObjectMappings));
428+
}
429+
430+
/**
431+
* A {@link VirtualObjectNode} in a frame state requires a corresponding
432+
* {@link EscapeObjectState} entry in {@link FrameState#virtualObjectMappings}. So when a
433+
* {@link VirtualObjectNode} is pushed as part of a frame state modification, the
434+
* {@link EscapeObjectState} must either be already there, or it must be passed in explicitly
435+
* from another frame state where the pushed value is coming from.
436+
*/
437+
private List<EscapeObjectState> ensureHasVirtualObjectMapping(VirtualObjectNode pushedValue, List<EscapeObjectState> pushedVirtualObjectMappings,
438+
List<EscapeObjectState> copiedVirtualObjectMappings) {
439+
if (virtualObjectMappings != null) {
440+
for (EscapeObjectState existingEscapeObjectState : virtualObjectMappings) {
441+
if (existingEscapeObjectState.object() == pushedValue) {
442+
/* Found a matching EscapeObjectState, nothing needs to be added. */
443+
return copiedVirtualObjectMappings;
444+
}
445+
}
446+
}
447+
448+
if (pushedVirtualObjectMappings == null) {
449+
throw GraalError.shouldNotReachHere("Pushing a virtual object, but no virtual object mapping provided: " + pushedValue);
450+
}
451+
for (EscapeObjectState pushedEscapeObjectState : pushedVirtualObjectMappings) {
452+
if (pushedEscapeObjectState.object() == pushedValue) {
453+
/*
454+
* A VirtualObjectState could have transitive dependencies on other object states
455+
* that are would also need to be added. For now, we do not have a case where a
456+
* FrameState with a VirtualObjectState is duplicated, therefore this case is not
457+
* implemented yet.
458+
*/
459+
GraalError.guarantee(pushedEscapeObjectState instanceof MaterializedObjectState, "A VirtualObjectState could have transitive dependencies");
460+
/*
461+
* Found a new EscapeObjectState that needs to be added to the
462+
* virtualObjectMappings.
463+
*/
464+
List<EscapeObjectState> result = copiedVirtualObjectMappings;
465+
if (result == null) {
466+
result = new ArrayList<>();
467+
if (virtualObjectMappings != null) {
468+
result.addAll(virtualObjectMappings);
469+
}
470+
}
471+
result.add(pushedEscapeObjectState);
472+
return result;
473+
}
474+
}
475+
throw GraalError.shouldNotReachHere("Did not find a virtual object mapping: " + pushedValue);
424476
}
425477

426478
/**

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/BytecodeExceptionNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ public FrameState createStateDuring() {
216216
boolean rethrowException = false;
217217
boolean duringCall = true;
218218
return stateAfter.duplicateModified(graph(), stateAfter.bci, rethrowException, duringCall,
219-
JavaKind.Object, null, null);
219+
JavaKind.Object, null, null, null);
220220
}
221221

222222
}

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/ForeignCall.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ default void computeStateDuring(FrameState currentStateAfter) {
7373
(currentStateAfter.stackSize() > 1 && currentStateAfter.stackAt(currentStateAfter.stackSize() - 2) == this)) {
7474
// The result of this call is on the top of stack, so roll back to the previous bci.
7575
assert bci() != BytecodeFrame.UNKNOWN_BCI : this;
76-
newStateDuring = currentStateAfter.duplicateModified(currentStateAfter.graph(), bci(), false, true, this.asNode().getStackKind(), null, null);
76+
newStateDuring = currentStateAfter.duplicateModified(currentStateAfter.graph(), bci(), false, true, this.asNode().getStackKind(), null, null, null);
7777
} else {
7878
newStateDuring = currentStateAfter;
7979
}

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/extended/RawLoadNode.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,10 @@ static Stamp computeStampForArrayAccess(ValueNode object, JavaKind accessKind, S
8686
// precise stamp but array accesses will not, so manually compute a better stamp from
8787
// the underlying object.
8888
if (accessKind.isObject() && type != null && type.getType().isArray() && type.getType().getComponentType().getJavaKind().isObject()) {
89-
TypeReference oldType = StampTool.typeReferenceOrNull(oldStamp);
9089
TypeReference componentType = TypeReference.create(object.graph().getAssumptions(), type.getType().getComponentType());
90+
Stamp newStamp = StampFactory.object(componentType);
9191
// Don't allow the type to get worse
92-
if (oldType == null || oldType.getType().isAssignableFrom(componentType.getType())) {
93-
return StampFactory.object(componentType);
94-
}
92+
return oldStamp == null ? newStamp : oldStamp.improveWith(newStamp);
9593
}
9694
if (oldStamp != null) {
9795
return oldStamp;

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/DynamicNewInstanceNode.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,36 @@
3030
import org.graalvm.compiler.core.common.type.StampFactory;
3131
import org.graalvm.compiler.graph.Node;
3232
import org.graalvm.compiler.graph.NodeClass;
33-
import org.graalvm.compiler.nodes.spi.Canonicalizable;
34-
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
3533
import org.graalvm.compiler.nodeinfo.NodeInfo;
36-
import org.graalvm.compiler.nodes.FrameState;
3734
import org.graalvm.compiler.nodes.NodeView;
3835
import org.graalvm.compiler.nodes.ValueNode;
36+
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext;
37+
import org.graalvm.compiler.nodes.spi.Canonicalizable;
38+
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
39+
import org.graalvm.compiler.nodes.spi.CoreProviders;
3940

41+
import jdk.vm.ci.meta.JavaKind;
4042
import jdk.vm.ci.meta.MetaAccessProvider;
4143
import jdk.vm.ci.meta.ResolvedJavaType;
4244

4345
@NodeInfo
44-
public class DynamicNewInstanceNode extends AbstractNewObjectNode implements Canonicalizable {
46+
public final class DynamicNewInstanceNode extends AbstractNewObjectNode implements Canonicalizable {
4547
public static final NodeClass<DynamicNewInstanceNode> TYPE = NodeClass.create(DynamicNewInstanceNode.class);
4648

4749
@Input ValueNode clazz;
4850

49-
public DynamicNewInstanceNode(ValueNode clazz, boolean fillContents) {
50-
this(TYPE, clazz, fillContents, null);
51+
public static void createAndPush(GraphBuilderContext b, ValueNode clazz) {
52+
ResolvedJavaType constantType = tryConvertToNonDynamic(clazz, b);
53+
if (constantType != null) {
54+
b.addPush(JavaKind.Object, new NewInstanceNode(constantType, true));
55+
} else {
56+
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
57+
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(clazzLegal, true));
58+
}
5159
}
5260

53-
protected DynamicNewInstanceNode(NodeClass<? extends DynamicNewInstanceNode> c, ValueNode clazz, boolean fillContents, FrameState stateBefore) {
54-
super(c, StampFactory.objectNonNull(), fillContents, stateBefore);
61+
protected DynamicNewInstanceNode(ValueNode clazz, boolean fillContents) {
62+
super(TYPE, StampFactory.objectNonNull(), fillContents, null);
5563
this.clazz = clazz;
5664
assert ((ObjectStamp) clazz.stamp(NodeView.DEFAULT)).nonNull();
5765
}
@@ -60,20 +68,20 @@ public ValueNode getInstanceType() {
6068
return clazz;
6169
}
6270

63-
public static boolean canConvertToNonDynamic(ValueNode clazz, CanonicalizerTool tool) {
71+
static ResolvedJavaType tryConvertToNonDynamic(ValueNode clazz, CoreProviders tool) {
6472
if (clazz.isConstant()) {
6573
ResolvedJavaType type = tool.getConstantReflection().asJavaType(clazz.asConstant());
6674
if (type != null && !throwsInstantiationException(type, tool.getMetaAccess()) && tool.getMetaAccessExtensionProvider().canConstantFoldDynamicAllocation(type)) {
67-
return true;
75+
return type;
6876
}
6977
}
70-
return false;
78+
return null;
7179
}
7280

7381
@Override
7482
public Node canonical(CanonicalizerTool tool) {
75-
if (canConvertToNonDynamic(clazz, tool)) {
76-
ResolvedJavaType type = tool.getConstantReflection().asJavaType(clazz.asConstant());
83+
ResolvedJavaType type = tryConvertToNonDynamic(clazz, tool);
84+
if (type != null) {
7785
return new NewInstanceNode(type, fillContents(), stateBefore());
7886
}
7987
return this;

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/ValidateNewInstanceClassNode.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
* or {@link Class} type.
4444
*/
4545
@NodeInfo(size = NodeSize.SIZE_8, cycles = NodeCycles.CYCLES_8, cyclesRationale = "Performs multiple checks.")
46-
public class ValidateNewInstanceClassNode extends WithExceptionNode implements Lowerable, Simplifiable {
46+
public final class ValidateNewInstanceClassNode extends WithExceptionNode implements Lowerable, Simplifiable {
4747

4848
@Input ValueNode clazz;
4949

@@ -56,7 +56,7 @@ public class ValidateNewInstanceClassNode extends WithExceptionNode implements L
5656

5757
public static final NodeClass<ValidateNewInstanceClassNode> TYPE = NodeClass.create(ValidateNewInstanceClassNode.class);
5858

59-
public ValidateNewInstanceClassNode(ValueNode clazz) {
59+
protected ValidateNewInstanceClassNode(ValueNode clazz) {
6060
super(TYPE, AbstractPointerStamp.pointerNonNull(clazz.stamp(NodeView.DEFAULT)));
6161
this.clazz = clazz;
6262
}
@@ -76,7 +76,7 @@ public void setClassClass(ValueNode newClassClass) {
7676

7777
@Override
7878
public void simplify(SimplifierTool tool) {
79-
if (DynamicNewInstanceNode.canConvertToNonDynamic(clazz, tool)) {
79+
if (DynamicNewInstanceNode.tryConvertToNonDynamic(clazz, tool) != null) {
8080
killExceptionEdge();
8181
tool.addToWorkList(usages());
8282
replaceAtUsages(clazz);

‎compiler/src/org.graalvm.compiler.virtual/src/org/graalvm/compiler/virtual/nodes/MaterializedObjectState.java‎ renamed to ‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/virtual/MaterializedObjectState.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,12 @@
2222
* or visit www.oracle.com if you need additional information or have any
2323
* questions.
2424
*/
25-
package org.graalvm.compiler.virtual.nodes;
25+
package org.graalvm.compiler.nodes.virtual;
2626

2727
import org.graalvm.compiler.graph.Node;
2828
import org.graalvm.compiler.graph.NodeClass;
2929
import org.graalvm.compiler.nodeinfo.NodeInfo;
3030
import org.graalvm.compiler.nodes.ValueNode;
31-
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
32-
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;
3331

3432
/**
3533
* This class encapsulated the materialized state of an escape analyzed object.

0 commit comments

Comments
 (0)