Skip to content

[GR-65001] [JDK-8361486] Rework expansion of inputs to Vector API components #11595

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
Jul 14, 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 @@ -50,6 +50,7 @@
import jdk.graal.compiler.nodes.memory.address.AddressNode;
import jdk.graal.compiler.nodes.memory.address.OffsetAddressNode;
import jdk.graal.compiler.nodes.spi.CoreProviders;
import jdk.graal.compiler.nodes.spi.ValueProxy;
import jdk.graal.compiler.vector.architecture.VectorArchitecture;
import jdk.graal.compiler.vector.architecture.VectorLoweringProvider;
import jdk.graal.compiler.vector.nodes.simd.LogicValueStamp;
Expand Down Expand Up @@ -171,7 +172,7 @@ static VectorAPIType asUnboxableVectorType(ValueNode value, CoreProviders provid
* Unboxing an object involves placing fixed read nodes. Therefore the value must be fixed,
* or it must be a pi with a fixed guard so we have a valid insertion position.
*/
if (!(value instanceof FixedWithNextNode || (value instanceof PiNode pi && pi.getGuard() != null && pi.getGuard() instanceof FixedWithNextNode))) {
if (!(value instanceof FixedWithNextNode || (value instanceof ValueProxy pi && pi.getGuard() != null && pi.getGuard() instanceof FixedWithNextNode))) {
return null;
}
/* Now check if this is a Vector API object we can do a SIMD read from. */
Expand Down Expand Up @@ -222,7 +223,7 @@ static ValueNode unboxObject(ValueNode value, CoreProviders providers) {
if (value instanceof FixedWithNextNode fixed) {
insertionPoint = fixed;
} else {
insertionPoint = (FixedWithNextNode) ((PiNode) value).getGuard();
insertionPoint = (FixedWithNextNode) ((ValueProxy) value).getGuard();
}
StructuredGraph graph = value.graph();
LoadFieldNode loadPayloadArray = graph.add(LoadFieldNode.create(value.graph().getAssumptions(), value, payloadField));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import jdk.graal.compiler.graph.NodeUnionFind;
import jdk.graal.compiler.nodes.AbstractBeginNode;
import jdk.graal.compiler.nodes.ConstantNode;
import jdk.graal.compiler.nodes.FixedNode;
import jdk.graal.compiler.nodes.FixedWithNextNode;
import jdk.graal.compiler.nodes.FrameState;
import jdk.graal.compiler.nodes.GraphState;
Expand All @@ -55,6 +56,7 @@
import jdk.graal.compiler.nodes.ValuePhiNode;
import jdk.graal.compiler.nodes.ValueProxyNode;
import jdk.graal.compiler.nodes.calc.MinMaxNode;
import jdk.graal.compiler.nodes.extended.FixedValueAnchorNode;
import jdk.graal.compiler.nodes.spi.CoreProviders;
import jdk.graal.compiler.nodes.spi.SimplifierTool;
import jdk.graal.compiler.nodes.type.StampTool;
Expand Down Expand Up @@ -476,6 +478,8 @@ private static void expandComponents(StructuredGraph graph, HighTierContext cont
NodeMap<ValueNode> expanded = new NodeMap<>(graph);
expanded.putAll(simdConstantCache);

/* Expand unboxing operations that are inputs to the component. */
unboxComponentInputs(graph, context, component, expanded);
/* Expand, starting from sinks and recursing upwards through inputs. */
for (VectorAPISinkNode sink : component.sinks) {
expandRecursivelyUpwards(graph, context, expanded, component.simdStamps, sink, vectorArch);
Expand All @@ -498,6 +502,67 @@ private static void expandComponents(StructuredGraph graph, HighTierContext cont
}
}

/**
* For all unboxable inputs to nodes in the component, insert the necessary unboxing code. We
* insert separate occurrences of the unboxing code for each usage of an unboxable input because
* otherwise it would be difficult to find a single correct insertion point for the unboxing
* code.
*/
private static void unboxComponentInputs(StructuredGraph graph, CoreProviders providers, ConnectedComponent component, NodeMap<ValueNode> expanded) {
GraalError.guarantee(component.canExpand, "should only place unbox nodes once we know the component can expand");
for (ValueNode unboxableInput : component.unboxes) {
for (ValueNode usage : unboxableInput.usages().filter(ValueNode.class).snapshot()) {
if (component.simdStamps.containsKey(usage)) {
if (usage instanceof VectorAPIMacroNode macro) {
anchorAndUnboxInput(graph, providers, macro, unboxableInput, macro, expanded);
} else if (usage instanceof ValuePhiNode phi) {
for (int i = 0; i < phi.valueCount(); i++) {
if (phi.valueAt(i) == unboxableInput) {
anchorAndUnboxInput(graph, providers, phi, unboxableInput, phi.merge().phiPredecessorAt(i), expanded, i);
}
}
} else if (usage instanceof ValueProxyNode proxy) {
anchorAndUnboxInput(graph, providers, proxy, unboxableInput, proxy.proxyPoint(), expanded);
} else {
throw GraalError.shouldNotReachHereUnexpectedValue(usage);
}
}
}
}
if (!component.unboxes.isEmpty()) {
graph.getDebug().dump(DebugContext.DETAILED_LEVEL, graph, "after unboxing %s inputs for %s", component.unboxes.size(), component);
}
/* Remove these unboxing operations from the component to mark them as handled. */
for (ValueNode unboxableInput : component.unboxes) {
component.simdStamps.removeKey(unboxableInput);
}
component.unboxes.clear();
}

private static void anchorAndUnboxInput(StructuredGraph graph, CoreProviders providers, ValueNode usage, ValueNode unboxableInput, FixedNode insertionPoint, NodeMap<ValueNode> expanded) {
anchorAndUnboxInput(graph, providers, usage, unboxableInput, insertionPoint, expanded, -1);
}

/**
* Adds a node anchoring {@code unboxableInput} before {@code insertionPoint}, replaces the
* anchor value in {@code usage}, then inserts unboxing code and records the unboxed version in
* the {@code expanded} map. If {@code usage} is a {@link ValuePhiNode}, {@code phiInputIndex}
* indicates the input to replace; it must be -1 otherwise.
*/
private static void anchorAndUnboxInput(StructuredGraph graph, CoreProviders providers, ValueNode usage, ValueNode unboxableInput, FixedNode insertionPoint, NodeMap<ValueNode> expanded,
int phiInputIndex) {
GraalError.guarantee(usage instanceof ValuePhiNode == (phiInputIndex != -1), "input index must be defined for phis, but not for any other node");
FixedValueAnchorNode anchor = graph.add(new FixedValueAnchorNode(unboxableInput));
graph.addBeforeFixed(insertionPoint, anchor);
if (phiInputIndex != -1) {
((ValuePhiNode) usage).setValueAt(phiInputIndex, anchor);
} else {
usage.replaceAllInputs(unboxableInput, anchor);
}
ValueNode unboxed = VectorAPIBoxingUtils.unboxObject(anchor, providers);
expanded.setAndGrow(anchor, unboxed);
}

/**
* Expand the given {@code node}, recursively expanding its input first if needed. Record a
* mapping from each node to its expansion in the {@code expanded} map. Do nothing on nodes that
Expand Down Expand Up @@ -599,7 +664,7 @@ private static void expandOneNode(StructuredGraph graph, CoreProviders providers
expansion = proxy.duplicateOn(proxy.proxyPoint(), expanded.get(proxy.getOriginalNode()));
}
} else if (VectorAPIBoxingUtils.asUnboxableVectorType(node, providers) != null) {
expansion = VectorAPIBoxingUtils.unboxObject(node, providers);
throw GraalError.shouldNotReachHere("unboxable input should have been expanded before members of the component: " + node);
} else {
throw GraalError.shouldNotReachHere("unexpected node during expansion: " + node);
}
Expand Down
Loading