8221514: [Graal] java/lang/String/CompactString/ tests fail with "GraalError: failed guarantee: no FrameState at DeoptimizingNode" in Graal -Xcomp mode
Summary: Put FrameState directly on final StateSplit in intrinsics.
Reviewed-by: thartmann, kvn, dnsimon, gdub
--- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java Sun Jul 07 18:17:46 2019 -0700
+++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java Sun Jul 07 18:27:38 2019 -0700
@@ -355,6 +355,7 @@
import org.graalvm.compiler.nodes.StructuredGraph;
import org.graalvm.compiler.nodes.UnwindNode;
import org.graalvm.compiler.nodes.ValueNode;
+import org.graalvm.compiler.nodes.ValuePhiNode;
import org.graalvm.compiler.nodes.calc.AddNode;
import org.graalvm.compiler.nodes.calc.AndNode;
import org.graalvm.compiler.nodes.calc.CompareNode;
@@ -627,7 +628,8 @@
}
static class IntrinsicScope extends InliningScope {
- boolean sawInvalidFrameState;
+ StateSplit returnStateSplit;
+ ArrayList<StateSplit> invalidStateUsers;
IntrinsicScope(BytecodeParser parser) {
super(parser);
@@ -650,30 +652,64 @@
isRootCompilation = false;
}
processPlaceholderFrameStates(isRootCompilation);
- if (sawInvalidFrameState) {
+ if (invalidStateUsers != null) {
JavaKind returnKind = parser.getInvokeReturnType().getJavaKind();
- FrameStateBuilder frameStateBuilder = parser.frameState;
- ValueNode returnValue = frameStateBuilder.pop(returnKind);
- StructuredGraph graph = parser.lastInstr.graph();
- StateSplitProxyNode proxy = graph.add(new StateSplitProxyNode(returnValue));
- parser.lastInstr.setNext(proxy);
- frameStateBuilder.push(returnKind, proxy);
- proxy.setStateAfter(parser.createFrameState(parser.stream.nextBCI(), proxy));
- parser.lastInstr = proxy;
+ ValueNode returnValue = parser.frameState.pop(returnKind);
+ if (invalidStateUsers.size() == 1 && invalidStateUsers.get(0) == parser.lastInstr) {
+ updateSplitFrameState(invalidStateUsers.get(0), returnKind, returnValue);
+ } else if (parser.lastInstr instanceof MergeNode) {
+ ValuePhiNode returnValues = null;
+ MergeNode merge = (MergeNode) parser.lastInstr;
+
+ if (returnValue instanceof ValuePhiNode && ((ValuePhiNode) returnValue).merge() == parser.lastInstr) {
+ returnValues = (ValuePhiNode) returnValue;
+ }
+ if (invalidStateUsers.remove(merge)) {
+ updateSplitFrameState(merge, returnKind, returnValue);
+ }
+ for (EndNode pred : merge.cfgPredecessors()) {
+ Node lastPred = pred.predecessor();
+ if (invalidStateUsers.remove(lastPred)) {
+ ValueNode predReturnValue = returnValue;
+ if (returnValues != null) {
+ int index = merge.phiPredecessorIndex(pred);
+ predReturnValue = ((ValuePhiNode) returnValue).valueAt(index);
+ }
+ updateSplitFrameState((StateSplit) lastPred, returnKind, predReturnValue);
+ }
+ }
+ if (invalidStateUsers.size() != 0) {
+ throw new GraalError("unexpected StateSplit above merge %s", invalidStateUsers);
+ }
+ } else {
+ throw new GraalError("unexpected node between return StateSplit and last instruction %s", parser.lastInstr);
+ }
+ // Restore the original return value
+ parser.frameState.push(returnKind, returnValue);
+ }
+ }
+
+ private void updateSplitFrameState(StateSplit split, JavaKind returnKind, ValueNode returnValue) {
+ parser.frameState.push(returnKind, returnValue);
+ FrameState oldState = split.stateAfter();
+ split.setStateAfter(parser.createFrameState(parser.stream.nextBCI(), split));
+ parser.frameState.pop(returnKind);
+ if (oldState.hasNoUsages()) {
+ oldState.safeDelete();
}
}
@Override
protected void handleReturnMismatch(StructuredGraph g, FrameState fs) {
- // If the intrinsic returns a non-void value, then any frame
- // state with an empty stack is invalid as it cannot
- // be used to deoptimize to just after the call returns.
- // These invalid frame states are expected to be removed
- // by later compilation stages.
- FrameState newFrameState = g.add(new FrameState(BytecodeFrame.INVALID_FRAMESTATE_BCI));
- newFrameState.setNodeSourcePosition(fs.getNodeSourcePosition());
- fs.replaceAndDelete(newFrameState);
- sawInvalidFrameState = true;
+ if (invalidStateUsers == null) {
+ invalidStateUsers = new ArrayList<>();
+ }
+ for (Node use : fs.usages()) {
+ if (!(use instanceof StateSplit)) {
+ throw new GraalError("Expected StateSplit for return mismatch");
+ }
+ invalidStateUsers.add((StateSplit) use);
+ }
}
}
@@ -2642,7 +2678,6 @@
List<ReturnToCallerData> calleeReturnDataList = parser.returnDataList;
- processCalleeReturn(targetMethod, s, calleeReturnDataList);
/*
* Propagate any side effects into the caller when parsing intrinsics.
*/
@@ -2652,6 +2687,8 @@
}
}
+ processCalleeReturn(targetMethod, s, calleeReturnDataList);
+
calleeBeforeUnwindNode = parser.getBeforeUnwindNode();
if (calleeBeforeUnwindNode != null) {
calleeUnwindValue = parser.getUnwindValue();