Skip to content

Commit 66dd4e2

Browse files
gergo-gergo-
committed
[GR-65001] [JDK-8361486] Rework expansion of inputs to Vector API components
PullRequest: graal/21345
2 parents b1ffedb + 42a6487 commit 66dd4e2

File tree

2 files changed

+69
-3
lines changed

2 files changed

+69
-3
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/vector/replacements/vectorapi/VectorAPIBoxingUtils.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import jdk.graal.compiler.nodes.memory.address.AddressNode;
5151
import jdk.graal.compiler.nodes.memory.address.OffsetAddressNode;
5252
import jdk.graal.compiler.nodes.spi.CoreProviders;
53+
import jdk.graal.compiler.nodes.spi.ValueProxy;
5354
import jdk.graal.compiler.vector.architecture.VectorArchitecture;
5455
import jdk.graal.compiler.vector.architecture.VectorLoweringProvider;
5556
import jdk.graal.compiler.vector.nodes.simd.LogicValueStamp;
@@ -171,7 +172,7 @@ static VectorAPIType asUnboxableVectorType(ValueNode value, CoreProviders provid
171172
* Unboxing an object involves placing fixed read nodes. Therefore the value must be fixed,
172173
* or it must be a pi with a fixed guard so we have a valid insertion position.
173174
*/
174-
if (!(value instanceof FixedWithNextNode || (value instanceof PiNode pi && pi.getGuard() != null && pi.getGuard() instanceof FixedWithNextNode))) {
175+
if (!(value instanceof FixedWithNextNode || (value instanceof ValueProxy pi && pi.getGuard() != null && pi.getGuard() instanceof FixedWithNextNode))) {
175176
return null;
176177
}
177178
/* Now check if this is a Vector API object we can do a SIMD read from. */
@@ -222,7 +223,7 @@ static ValueNode unboxObject(ValueNode value, CoreProviders providers) {
222223
if (value instanceof FixedWithNextNode fixed) {
223224
insertionPoint = fixed;
224225
} else {
225-
insertionPoint = (FixedWithNextNode) ((PiNode) value).getGuard();
226+
insertionPoint = (FixedWithNextNode) ((ValueProxy) value).getGuard();
226227
}
227228
StructuredGraph graph = value.graph();
228229
LoadFieldNode loadPayloadArray = graph.add(LoadFieldNode.create(value.graph().getAssumptions(), value, payloadField));

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/vector/replacements/vectorapi/VectorAPIExpansionPhase.java

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import jdk.graal.compiler.graph.NodeUnionFind;
4747
import jdk.graal.compiler.nodes.AbstractBeginNode;
4848
import jdk.graal.compiler.nodes.ConstantNode;
49+
import jdk.graal.compiler.nodes.FixedNode;
4950
import jdk.graal.compiler.nodes.FixedWithNextNode;
5051
import jdk.graal.compiler.nodes.FrameState;
5152
import jdk.graal.compiler.nodes.GraphState;
@@ -55,6 +56,7 @@
5556
import jdk.graal.compiler.nodes.ValuePhiNode;
5657
import jdk.graal.compiler.nodes.ValueProxyNode;
5758
import jdk.graal.compiler.nodes.calc.MinMaxNode;
59+
import jdk.graal.compiler.nodes.extended.FixedValueAnchorNode;
5860
import jdk.graal.compiler.nodes.spi.CoreProviders;
5961
import jdk.graal.compiler.nodes.spi.SimplifierTool;
6062
import jdk.graal.compiler.nodes.type.StampTool;
@@ -476,6 +478,8 @@ private static void expandComponents(StructuredGraph graph, HighTierContext cont
476478
NodeMap<ValueNode> expanded = new NodeMap<>(graph);
477479
expanded.putAll(simdConstantCache);
478480

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

505+
/**
506+
* For all unboxable inputs to nodes in the component, insert the necessary unboxing code. We
507+
* insert separate occurrences of the unboxing code for each usage of an unboxable input because
508+
* otherwise it would be difficult to find a single correct insertion point for the unboxing
509+
* code.
510+
*/
511+
private static void unboxComponentInputs(StructuredGraph graph, CoreProviders providers, ConnectedComponent component, NodeMap<ValueNode> expanded) {
512+
GraalError.guarantee(component.canExpand, "should only place unbox nodes once we know the component can expand");
513+
for (ValueNode unboxableInput : component.unboxes) {
514+
for (ValueNode usage : unboxableInput.usages().filter(ValueNode.class).snapshot()) {
515+
if (component.simdStamps.containsKey(usage)) {
516+
if (usage instanceof VectorAPIMacroNode macro) {
517+
anchorAndUnboxInput(graph, providers, macro, unboxableInput, macro, expanded);
518+
} else if (usage instanceof ValuePhiNode phi) {
519+
for (int i = 0; i < phi.valueCount(); i++) {
520+
if (phi.valueAt(i) == unboxableInput) {
521+
anchorAndUnboxInput(graph, providers, phi, unboxableInput, phi.merge().phiPredecessorAt(i), expanded, i);
522+
}
523+
}
524+
} else if (usage instanceof ValueProxyNode proxy) {
525+
anchorAndUnboxInput(graph, providers, proxy, unboxableInput, proxy.proxyPoint(), expanded);
526+
} else {
527+
throw GraalError.shouldNotReachHereUnexpectedValue(usage);
528+
}
529+
}
530+
}
531+
}
532+
if (!component.unboxes.isEmpty()) {
533+
graph.getDebug().dump(DebugContext.DETAILED_LEVEL, graph, "after unboxing %s inputs for %s", component.unboxes.size(), component);
534+
}
535+
/* Remove these unboxing operations from the component to mark them as handled. */
536+
for (ValueNode unboxableInput : component.unboxes) {
537+
component.simdStamps.removeKey(unboxableInput);
538+
}
539+
component.unboxes.clear();
540+
}
541+
542+
private static void anchorAndUnboxInput(StructuredGraph graph, CoreProviders providers, ValueNode usage, ValueNode unboxableInput, FixedNode insertionPoint, NodeMap<ValueNode> expanded) {
543+
anchorAndUnboxInput(graph, providers, usage, unboxableInput, insertionPoint, expanded, -1);
544+
}
545+
546+
/**
547+
* Adds a node anchoring {@code unboxableInput} before {@code insertionPoint}, replaces the
548+
* anchor value in {@code usage}, then inserts unboxing code and records the unboxed version in
549+
* the {@code expanded} map. If {@code usage} is a {@link ValuePhiNode}, {@code phiInputIndex}
550+
* indicates the input to replace; it must be -1 otherwise.
551+
*/
552+
private static void anchorAndUnboxInput(StructuredGraph graph, CoreProviders providers, ValueNode usage, ValueNode unboxableInput, FixedNode insertionPoint, NodeMap<ValueNode> expanded,
553+
int phiInputIndex) {
554+
GraalError.guarantee(usage instanceof ValuePhiNode == (phiInputIndex != -1), "input index must be defined for phis, but not for any other node");
555+
FixedValueAnchorNode anchor = graph.add(new FixedValueAnchorNode(unboxableInput));
556+
graph.addBeforeFixed(insertionPoint, anchor);
557+
if (phiInputIndex != -1) {
558+
((ValuePhiNode) usage).setValueAt(phiInputIndex, anchor);
559+
} else {
560+
usage.replaceAllInputs(unboxableInput, anchor);
561+
}
562+
ValueNode unboxed = VectorAPIBoxingUtils.unboxObject(anchor, providers);
563+
expanded.setAndGrow(anchor, unboxed);
564+
}
565+
501566
/**
502567
* Expand the given {@code node}, recursively expanding its input first if needed. Record a
503568
* mapping from each node to its expansion in the {@code expanded} map. Do nothing on nodes that
@@ -599,7 +664,7 @@ private static void expandOneNode(StructuredGraph graph, CoreProviders providers
599664
expansion = proxy.duplicateOn(proxy.proxyPoint(), expanded.get(proxy.getOriginalNode()));
600665
}
601666
} else if (VectorAPIBoxingUtils.asUnboxableVectorType(node, providers) != null) {
602-
expansion = VectorAPIBoxingUtils.unboxObject(node, providers);
667+
throw GraalError.shouldNotReachHere("unboxable input should have been expanded before members of the component: " + node);
603668
} else {
604669
throw GraalError.shouldNotReachHere("unexpected node during expansion: " + node);
605670
}

0 commit comments

Comments
 (0)