Skip to content

[GR-62081] Fix TypeFlowGraph linearization #11012

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 1 commit into from
Apr 9, 2025
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 @@ -32,7 +32,7 @@
* A sink type flow for the context insensitive invoke used to link in parameters in each caller
* context.
*/
public class ActualParameterTypeFlow extends TypeFlow<ValueNode> {
public class ActualParameterTypeFlow extends TypeFlow<ValueNode> implements GlobalFlow {
public ActualParameterTypeFlow(AnalysisType declaredType) {
super(null, filterUncheckedInterface(declaredType));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/**
* Keeps track of all synchronized types.
*/
public class AllSynchronizedTypeFlow extends TypeFlow<Object> {
public class AllSynchronizedTypeFlow extends TypeFlow<Object> implements GlobalFlow {

@Override
public String toString() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,23 @@
import com.oracle.graal.pointsto.PointsToAnalysis;
import com.oracle.graal.pointsto.meta.AnalysisType;

import jdk.vm.ci.code.BytecodePosition;

/**
* A local use of AllInstantiatedFlow that can have a predicate.
*/
public class LocalAllInstantiatedFlow extends TypeFlow<AnalysisType> {
public class LocalAllInstantiatedFlow extends TypeFlow<BytecodePosition> {

public LocalAllInstantiatedFlow(AnalysisType declaredType) {
super(declaredType, declaredType);
public LocalAllInstantiatedFlow(BytecodePosition position, AnalysisType declaredType) {
super(position, declaredType);
}

private LocalAllInstantiatedFlow(MethodFlowsGraph methodFlows, LocalAllInstantiatedFlow original) {
super(original, methodFlows);
}

@Override
public TypeFlow<AnalysisType> copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
public TypeFlow<BytecodePosition> copy(PointsToAnalysis bb, MethodFlowsGraph methodFlows) {
return new LocalAllInstantiatedFlow(methodFlows, this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

import jdk.graal.compiler.graph.Node;
import jdk.graal.compiler.nodes.EncodedGraph.EncodedNodeReference;
import jdk.vm.ci.code.BytecodePosition;

public class MethodFlowsGraph implements MethodFlowsGraphInfo {
/**
Expand Down Expand Up @@ -124,7 +125,7 @@ public static boolean nonMethodFlow(TypeFlow<?> flow) {
*
* AnyPrimitiveFlow can be either global (source == null) or local (source != null)
*/
return flow instanceof AllInstantiatedTypeFlow || flow instanceof AllSynchronizedTypeFlow || (flow instanceof AnyPrimitiveSourceTypeFlow && flow.getSource() == null);
return flow instanceof GlobalFlow || flow.isContextInsensitive() || (flow instanceof AnyPrimitiveSourceTypeFlow && flow.getSource() == null);
}

/**
Expand Down Expand Up @@ -224,10 +225,27 @@ public boolean hasNext() {
@Override
public TypeFlow<?> next() {
TypeFlow<?> current = next;
assert withinMethod(current) : "Flow " + current + " has source " + current.source + " that is not a part of " + method;
next = findNext();
return current;
}

/**
* Check that the flow is local to the typeflow graph of this method. The iterator
* should not leave the scope of this method, but it may include flows originated from
* inlined methods.
*/
private boolean withinMethod(TypeFlow<?> flow) {
var source = flow.getSource();
while (source instanceof BytecodePosition position) {
if (method.equals(position.getMethod())) {
return true;
}
source = position.getCaller();
}
return false;
}

/** Get the next flow and expand the work list. */
private TypeFlow<?> findNext() {
/* pollFirst returns null if the deque is empty. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,9 @@ protected void apply(boolean forceReparse, Object reason) {
* It only makes sense to create a local version of all instantiated if it will be guarded by a
* predicate more precise than alwaysEnabled.
*/
protected TypeFlow<?> maybePatchAllInstantiated(TypeFlow<?> flow, AnalysisType declaredType, Object predicate) {
protected TypeFlow<?> maybePatchAllInstantiated(TypeFlow<?> flow, BytecodePosition position, AnalysisType declaredType, Object predicate) {
if (bb.usePredicates() && flow instanceof AllInstantiatedTypeFlow && predicate != alwaysEnabled) {
var localFlow = new LocalAllInstantiatedFlow(declaredType);
var localFlow = new LocalAllInstantiatedFlow(position, declaredType);
flowsGraph.addMiscEntryFlow(localFlow);
flow.addUse(bb, localFlow);
return localFlow;
Expand Down Expand Up @@ -818,13 +818,14 @@ protected TypeFlowBuilder<?> handleObjectStamp(ObjectStamp stamp, ValueNode node
throw AnalysisError.shouldNotReachHere("Stamp for node " + node + " is empty.");
}
AnalysisType stampType = (AnalysisType) StampTool.typeOrNull(stamp, bb.getMetaAccess());
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(node);
if (stamp.isExactType()) {
/*
* We are lucky: the stamp tells us which type the node has. Happens e.g. for a
* predicated boxed node.
*/
return TypeFlowBuilder.create(bb, method, getPredicate(), node, SourceTypeFlow.class, () -> {
SourceTypeFlow src = new SourceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), stampType, !stamp.nonNull());
SourceTypeFlow src = new SourceTypeFlow(position, stampType, !stamp.nonNull());
flowsGraph.addMiscEntryFlow(src);
return src;
});
Expand All @@ -835,9 +836,9 @@ protected TypeFlowBuilder<?> handleObjectStamp(ObjectStamp stamp, ValueNode node
*/
TypeFlowBuilder<?> predicate = getPredicate();
return TypeFlowBuilder.create(bb, method, predicate, node, TypeFlow.class, () -> {
TypeFlow<?> proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), stampType.getTypeFlow(bb, true));
TypeFlow<?> proxy = bb.analysisPolicy().proxy(position, stampType.getTypeFlow(bb, true));
flowsGraph.addMiscEntryFlow(proxy);
return maybePatchAllInstantiated(proxy, stampType, predicate);
return maybePatchAllInstantiated(proxy, position, stampType, predicate);
});
}
}
Expand Down Expand Up @@ -1470,9 +1471,10 @@ protected void node(FixedNode n) {
TypeFlowBuilder<?> exceptionObjectBuilder = TypeFlowBuilder.create(bb, method, predicate, node, TypeFlow.class, () -> {
AnalysisType analysisType = (AnalysisType) StampTool.typeOrNull(node, bb.getMetaAccess());
TypeFlow<?> input = analysisType.getTypeFlow(bb, false);
TypeFlow<?> exceptionObjectFlow = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(node), input);
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(node);
TypeFlow<?> exceptionObjectFlow = bb.analysisPolicy().proxy(position, input);
flowsGraph.addMiscEntryFlow(exceptionObjectFlow);
return maybePatchAllInstantiated(exceptionObjectFlow, analysisType, predicate);
return maybePatchAllInstantiated(exceptionObjectFlow, position, analysisType, predicate);
});
state.add(node, exceptionObjectBuilder);

Expand Down Expand Up @@ -1533,7 +1535,7 @@ protected void node(FixedNode n) {
instanceType = bb.getObjectType();
TypeFlowBuilder<?> predicate = state.getPredicate();
instanceTypeBuilder = TypeFlowBuilder.create(bb, method, predicate, instanceType, TypeFlow.class,
() -> maybePatchAllInstantiated(instanceType.getTypeFlow(bb, false), instanceType, predicate));
() -> maybePatchAllInstantiated(instanceType.getTypeFlow(bb, false), AbstractAnalysisEngine.sourcePosition(node), instanceType, predicate));
}
TypeFlowBuilder<DynamicNewInstanceTypeFlow> dynamicNewInstanceBuilder = TypeFlowBuilder.create(bb, method, state.getPredicate(), node, DynamicNewInstanceTypeFlow.class, () -> {
DynamicNewInstanceTypeFlow newInstanceTypeFlow = new DynamicNewInstanceTypeFlow(AbstractAnalysisEngine.sourcePosition(node), instanceTypeBuilder.get(), instanceType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ public String toString() {
/**
* The state of the StoreFieldTypeFlow reflects the state of the stored value. The
* StoreFieldTypeFlow is an observer of the receiver flow, i.e. flow modeling the receiver
* object of the store operation..
* object of the store operation.
*
* Every time the state of the receiver flow changes the corresponding field flows are added as
* uses to the store field flow. Thus the stored value is propagated to the store field flow
* uses to the store field flow. Thus, the stored value is propagated to the store field flow
* into the field flows.
*/
public static class StoreInstanceFieldTypeFlow extends StoreFieldTypeFlow {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,10 @@ private void storeVMThreadLocal(TypeFlowsOfNodes state, ValueNode storeNode, Val

TypeFlowBuilder<?> predicate = state.getPredicate();
TypeFlowBuilder<?> storeBuilder = TypeFlowBuilder.create(bb, method, predicate, storeNode, TypeFlow.class, () -> {
TypeFlow<?> proxy = bb.analysisPolicy().proxy(AbstractAnalysisEngine.sourcePosition(storeNode), valueType.getTypeFlow(bb, false));
BytecodePosition position = AbstractAnalysisEngine.sourcePosition(storeNode);
TypeFlow<?> proxy = bb.analysisPolicy().proxy(position, valueType.getTypeFlow(bb, false));
flowsGraph.addMiscEntryFlow(proxy);
return maybePatchAllInstantiated(proxy, valueType, predicate);
return maybePatchAllInstantiated(proxy, position, valueType, predicate);
});
storeBuilder.addUseDependency(valueBuilder);
typeFlowGraphBuilder.registerSinkBuilder(storeBuilder);
Expand Down
Loading