Skip to content

[GR-34935] Run escape analysis before static analysis. #3993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@
import org.graalvm.compiler.nodes.spi.NodeWithState;
import org.graalvm.compiler.nodes.util.GraphUtil;
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
import org.graalvm.compiler.nodes.virtual.MaterializedObjectState;
import org.graalvm.compiler.nodes.virtual.VirtualBoxingNode;
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;
import org.graalvm.compiler.nodes.virtual.VirtualObjectState;
import org.graalvm.compiler.serviceprovider.GraalServices;
import org.graalvm.compiler.virtual.nodes.MaterializedObjectState;
import org.graalvm.compiler.virtual.nodes.VirtualObjectState;

import jdk.vm.ci.code.BytecodeFrame;
import jdk.vm.ci.code.RegisterValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@
import org.graalvm.compiler.nodes.java.DynamicNewInstanceNode;
import org.graalvm.compiler.nodes.java.InstanceOfNode;
import org.graalvm.compiler.nodes.java.NewArrayNode;
import org.graalvm.compiler.nodes.java.ValidateNewInstanceClassNode;
import org.graalvm.compiler.nodes.memory.OnHeapMemoryAccess.BarrierType;
import org.graalvm.compiler.nodes.memory.address.AddressNode;
import org.graalvm.compiler.nodes.memory.address.OffsetAddressNode;
Expand All @@ -141,7 +140,6 @@
import jdk.vm.ci.code.TargetDescription;
import jdk.vm.ci.hotspot.VMIntrinsicMethod;
import jdk.vm.ci.meta.ConstantReflectionProvider;
import jdk.vm.ci.meta.DeoptimizationAction;
import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.MetaAccessProvider;
import jdk.vm.ci.meta.ResolvedJavaField;
Expand Down Expand Up @@ -523,8 +521,7 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
* check. Such a DynamicNewInstanceNode is also never constant folded to a
* NewInstanceNode.
*/
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(b.nullCheckedValue(clazzLegal, DeoptimizationAction.None), true));
DynamicNewInstanceNode.createAndPush(b, clazz);
return true;
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public FrameState create(int bci, BytecodeParser parent, boolean duringCall, Jav
outerFrameState = parent.getFrameStateBuilder().create(parent.bci(), parent.getNonIntrinsicAncestor(), true, null, null);
}
if (bci == BytecodeFrame.AFTER_EXCEPTION_BCI && parent != null) {
return outerFrameState.duplicateModified(graph, outerFrameState.bci, true, false, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{stack[0]});
return outerFrameState.duplicateModified(graph, outerFrameState.bci, true, false, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{stack[0]}, null);
}
if (bci == BytecodeFrame.INVALID_FRAMESTATE_BCI) {
throw shouldNotReachHere();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.graalvm.compiler.nodes.java.ExceptionObjectNode;
import org.graalvm.compiler.nodes.java.MonitorIdNode;
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
import org.graalvm.compiler.nodes.virtual.MaterializedObjectState;
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;

import jdk.vm.ci.code.BytecodeFrame;
Expand Down Expand Up @@ -365,24 +366,20 @@ public FrameState duplicateWithVirtualState() {
* the stack.
*/
public FrameState duplicateModifiedDuringCall(int newBci, JavaKind popKind) {
return duplicateModified(graph(), newBci, rethrowException, true, popKind, null, null);
return duplicateModified(graph(), newBci, rethrowException, true, popKind, null, null, null);
}

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

/**
* Creates a copy of this frame state with the top of stack replaced with with
* {@code pushedValue} which must be of type {@code popKind}.
*/
public FrameState duplicateModified(JavaKind popKind, JavaKind pushedSlotKind, ValueNode pushedValue) {
public FrameState duplicateModified(JavaKind popKind, JavaKind pushedSlotKind, ValueNode pushedValue, List<EscapeObjectState> pushedVirtualObjectMappings) {
assert pushedValue != null && pushedValue.getStackKind() == popKind;
return duplicateModified(graph(), bci, rethrowException, duringCall, popKind, new JavaKind[]{pushedSlotKind}, new ValueNode[]{pushedValue});
}

public FrameState duplicateRethrow(ValueNode exceptionObject) {
return duplicateModified(graph(), bci, true, duringCall, JavaKind.Void, new JavaKind[]{JavaKind.Object}, new ValueNode[]{exceptionObject});
return duplicateModified(graph(), bci, rethrowException, duringCall, popKind, new JavaKind[]{pushedSlotKind}, new ValueNode[]{pushedValue}, pushedVirtualObjectMappings);
}

/**
Expand All @@ -391,7 +388,9 @@ public FrameState duplicateRethrow(ValueNode exceptionObject) {
* correctly in slot encoding: a long or double will be followed by a null slot. The bci will be
* changed to newBci.
*/
public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean newRethrowException, boolean newDuringCall, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues) {
public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean newRethrowException, boolean newDuringCall, JavaKind popKind, JavaKind[] pushedSlotKinds, ValueNode[] pushedValues,
List<EscapeObjectState> pushedVirtualObjectMappings) {
List<EscapeObjectState> copiedVirtualObjectMappings = null;
ArrayList<ValueNode> copy;
if (newRethrowException && !rethrowException && popKind == JavaKind.Void) {
assert popKind == JavaKind.Void;
Expand All @@ -410,7 +409,11 @@ public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean n
if (pushedValues != null) {
assert pushedSlotKinds.length == pushedValues.length;
for (int i = 0; i < pushedValues.length; i++) {
copy.add(pushedValues[i]);
ValueNode pushedValue = pushedValues[i];
if (pushedValue instanceof VirtualObjectNode) {
copiedVirtualObjectMappings = ensureHasVirtualObjectMapping((VirtualObjectNode) pushedValue, pushedVirtualObjectMappings, copiedVirtualObjectMappings);
}
copy.add(pushedValue);
if (pushedSlotKinds[i].needsTwoSlots()) {
copy.add(null);
}
Expand All @@ -420,7 +423,56 @@ public FrameState duplicateModified(StructuredGraph graph, int newBci, boolean n
copy.addAll(values.subList(localsSize + stackSize, values.size()));

assert checkStackDepth(bci, stackSize, duringCall, rethrowException, newBci, newStackSize, newDuringCall, newRethrowException);
return graph.add(new FrameState(outerFrameState(), code, newBci, copy, localsSize, newStackSize, newRethrowException, newDuringCall, monitorIds, virtualObjectMappings));
return graph.add(new FrameState(outerFrameState(), code, newBci, copy, localsSize, newStackSize, newRethrowException, newDuringCall, monitorIds,
copiedVirtualObjectMappings != null ? copiedVirtualObjectMappings : virtualObjectMappings));
}

/**
* A {@link VirtualObjectNode} in a frame state requires a corresponding
* {@link EscapeObjectState} entry in {@link FrameState#virtualObjectMappings}. So when a
* {@link VirtualObjectNode} is pushed as part of a frame state modification, the
* {@link EscapeObjectState} must either be already there, or it must be passed in explicitly
* from another frame state where the pushed value is coming from.
*/
private List<EscapeObjectState> ensureHasVirtualObjectMapping(VirtualObjectNode pushedValue, List<EscapeObjectState> pushedVirtualObjectMappings,
List<EscapeObjectState> copiedVirtualObjectMappings) {
if (virtualObjectMappings != null) {
for (EscapeObjectState existingEscapeObjectState : virtualObjectMappings) {
if (existingEscapeObjectState.object() == pushedValue) {
/* Found a matching EscapeObjectState, nothing needs to be added. */
return copiedVirtualObjectMappings;
}
}
}

if (pushedVirtualObjectMappings == null) {
throw GraalError.shouldNotReachHere("Pushing a virtual object, but no virtual object mapping provided: " + pushedValue);
}
for (EscapeObjectState pushedEscapeObjectState : pushedVirtualObjectMappings) {
if (pushedEscapeObjectState.object() == pushedValue) {
/*
* A VirtualObjectState could have transitive dependencies on other object states
* that are would also need to be added. For now, we do not have a case where a
* FrameState with a VirtualObjectState is duplicated, therefore this case is not
* implemented yet.
*/
GraalError.guarantee(pushedEscapeObjectState instanceof MaterializedObjectState, "A VirtualObjectState could have transitive dependencies");
/*
* Found a new EscapeObjectState that needs to be added to the
* virtualObjectMappings.
*/
List<EscapeObjectState> result = copiedVirtualObjectMappings;
if (result == null) {
result = new ArrayList<>();
if (virtualObjectMappings != null) {
result.addAll(virtualObjectMappings);
}
}
result.add(pushedEscapeObjectState);
return result;
}
}
throw GraalError.shouldNotReachHere("Did not find a virtual object mapping: " + pushedValue);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public FrameState createStateDuring() {
boolean rethrowException = false;
boolean duringCall = true;
return stateAfter.duplicateModified(graph(), stateAfter.bci, rethrowException, duringCall,
JavaKind.Object, null, null);
JavaKind.Object, null, null, null);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ default void computeStateDuring(FrameState currentStateAfter) {
(currentStateAfter.stackSize() > 1 && currentStateAfter.stackAt(currentStateAfter.stackSize() - 2) == this)) {
// The result of this call is on the top of stack, so roll back to the previous bci.
assert bci() != BytecodeFrame.UNKNOWN_BCI : this;
newStateDuring = currentStateAfter.duplicateModified(currentStateAfter.graph(), bci(), false, true, this.asNode().getStackKind(), null, null);
newStateDuring = currentStateAfter.duplicateModified(currentStateAfter.graph(), bci(), false, true, this.asNode().getStackKind(), null, null, null);
} else {
newStateDuring = currentStateAfter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,10 @@ static Stamp computeStampForArrayAccess(ValueNode object, JavaKind accessKind, S
// precise stamp but array accesses will not, so manually compute a better stamp from
// the underlying object.
if (accessKind.isObject() && type != null && type.getType().isArray() && type.getType().getComponentType().getJavaKind().isObject()) {
TypeReference oldType = StampTool.typeReferenceOrNull(oldStamp);
TypeReference componentType = TypeReference.create(object.graph().getAssumptions(), type.getType().getComponentType());
Stamp newStamp = StampFactory.object(componentType);
// Don't allow the type to get worse
if (oldType == null || oldType.getType().isAssignableFrom(componentType.getType())) {
return StampFactory.object(componentType);
}
return oldStamp == null ? newStamp : oldStamp.improveWith(newStamp);
}
if (oldStamp != null) {
return oldStamp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,36 @@
import org.graalvm.compiler.core.common.type.StampFactory;
import org.graalvm.compiler.graph.Node;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodes.spi.Canonicalizable;
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
import org.graalvm.compiler.nodeinfo.NodeInfo;
import org.graalvm.compiler.nodes.FrameState;
import org.graalvm.compiler.nodes.NodeView;
import org.graalvm.compiler.nodes.ValueNode;
import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext;
import org.graalvm.compiler.nodes.spi.Canonicalizable;
import org.graalvm.compiler.nodes.spi.CanonicalizerTool;
import org.graalvm.compiler.nodes.spi.CoreProviders;

import jdk.vm.ci.meta.JavaKind;
import jdk.vm.ci.meta.MetaAccessProvider;
import jdk.vm.ci.meta.ResolvedJavaType;

@NodeInfo
public class DynamicNewInstanceNode extends AbstractNewObjectNode implements Canonicalizable {
public final class DynamicNewInstanceNode extends AbstractNewObjectNode implements Canonicalizable {
public static final NodeClass<DynamicNewInstanceNode> TYPE = NodeClass.create(DynamicNewInstanceNode.class);

@Input ValueNode clazz;

public DynamicNewInstanceNode(ValueNode clazz, boolean fillContents) {
this(TYPE, clazz, fillContents, null);
public static void createAndPush(GraphBuilderContext b, ValueNode clazz) {
ResolvedJavaType constantType = tryConvertToNonDynamic(clazz, b);
if (constantType != null) {
b.addPush(JavaKind.Object, new NewInstanceNode(constantType, true));
} else {
ValueNode clazzLegal = b.add(new ValidateNewInstanceClassNode(clazz));
b.addPush(JavaKind.Object, new DynamicNewInstanceNode(clazzLegal, true));
}
}

protected DynamicNewInstanceNode(NodeClass<? extends DynamicNewInstanceNode> c, ValueNode clazz, boolean fillContents, FrameState stateBefore) {
super(c, StampFactory.objectNonNull(), fillContents, stateBefore);
protected DynamicNewInstanceNode(ValueNode clazz, boolean fillContents) {
super(TYPE, StampFactory.objectNonNull(), fillContents, null);
this.clazz = clazz;
assert ((ObjectStamp) clazz.stamp(NodeView.DEFAULT)).nonNull();
}
Expand All @@ -60,20 +68,20 @@ public ValueNode getInstanceType() {
return clazz;
}

public static boolean canConvertToNonDynamic(ValueNode clazz, CanonicalizerTool tool) {
static ResolvedJavaType tryConvertToNonDynamic(ValueNode clazz, CoreProviders tool) {
if (clazz.isConstant()) {
ResolvedJavaType type = tool.getConstantReflection().asJavaType(clazz.asConstant());
if (type != null && !throwsInstantiationException(type, tool.getMetaAccess()) && tool.getMetaAccessExtensionProvider().canConstantFoldDynamicAllocation(type)) {
return true;
return type;
}
}
return false;
return null;
}

@Override
public Node canonical(CanonicalizerTool tool) {
if (canConvertToNonDynamic(clazz, tool)) {
ResolvedJavaType type = tool.getConstantReflection().asJavaType(clazz.asConstant());
ResolvedJavaType type = tryConvertToNonDynamic(clazz, tool);
if (type != null) {
return new NewInstanceNode(type, fillContents(), stateBefore());
}
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
* or {@link Class} type.
*/
@NodeInfo(size = NodeSize.SIZE_8, cycles = NodeCycles.CYCLES_8, cyclesRationale = "Performs multiple checks.")
public class ValidateNewInstanceClassNode extends WithExceptionNode implements Lowerable, Simplifiable {
public final class ValidateNewInstanceClassNode extends WithExceptionNode implements Lowerable, Simplifiable {

@Input ValueNode clazz;

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

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

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

@Override
public void simplify(SimplifierTool tool) {
if (DynamicNewInstanceNode.canConvertToNonDynamic(clazz, tool)) {
if (DynamicNewInstanceNode.tryConvertToNonDynamic(clazz, tool) != null) {
killExceptionEdge();
tool.addToWorkList(usages());
replaceAtUsages(clazz);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.graalvm.compiler.virtual.nodes;
package org.graalvm.compiler.nodes.virtual;

import org.graalvm.compiler.graph.Node;
import org.graalvm.compiler.graph.NodeClass;
import org.graalvm.compiler.nodeinfo.NodeInfo;
import org.graalvm.compiler.nodes.ValueNode;
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;

/**
* This class encapsulated the materialized state of an escape analyzed object.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.graalvm.compiler.virtual.nodes;
package org.graalvm.compiler.nodes.virtual;

import java.util.List;

Expand All @@ -31,8 +31,6 @@
import org.graalvm.compiler.graph.NodeInputList;
import org.graalvm.compiler.nodeinfo.NodeInfo;
import org.graalvm.compiler.nodes.ValueNode;
import org.graalvm.compiler.nodes.virtual.EscapeObjectState;
import org.graalvm.compiler.nodes.virtual.VirtualObjectNode;

/**
* This class encapsulated the virtual state of an escape analyzed object.
Expand Down
Loading