Mercurial > hg > truffle
changeset 21289:3b5ec1a2b3b5
consolidate frame state creation and processing for intrinsics into graph parsing, removing need for CollapseFrameForSingleSideEffectPhase
line wrap: on
line diff
--- a/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/GraphBuilderContext.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/GraphBuilderContext.java Mon May 11 17:12:15 2015 +0200 @@ -115,7 +115,7 @@ T equivalentValue = append(value); if (equivalentValue instanceof StateSplit) { StateSplit stateSplit = (StateSplit) equivalentValue; - if (stateSplit.stateAfter() == null) { + if (stateSplit.stateAfter() == null && stateSplit.hasSideEffect()) { setStateAfter(stateSplit); } } @@ -149,7 +149,7 @@ push(kind.getStackKind(), equivalentValue); if (equivalentValue instanceof StateSplit) { StateSplit stateSplit = (StateSplit) equivalentValue; - if (stateSplit.stateAfter() == null) { + if (stateSplit.stateAfter() == null && stateSplit.hasSideEffect()) { setStateAfter(stateSplit); } } @@ -194,11 +194,12 @@ /** * Creates a snap shot of the current frame state with the BCI of the instruction after the one - * currently being parsed and assigns it to a given state split node. + * currently being parsed and assigns it to a given {@linkplain StateSplit#hasSideEffect() side + * effect} node. * - * @param stateSplit a node just appended to the graph that needs a frame state + * @param sideEffect a side effect node just appended to the graph */ - void setStateAfter(StateSplit stateSplit); + void setStateAfter(StateSplit sideEffect); /** * Gets the parsing context for the method that inlines the method being parsed by this context. @@ -206,6 +207,18 @@ GraphBuilderContext getParent(); /** + * Gets the first ancestor parsing context that is not parsing a + * {@linkplain #parsingReplacement() replacement}. + */ + default GraphBuilderContext getNonReplacementAncestor() { + GraphBuilderContext ancestor = getParent(); + while (ancestor != null && ancestor.parsingReplacement()) { + ancestor = ancestor.getParent(); + } + return ancestor; + } + + /** * Gets the method currently being parsed. */ ResolvedJavaMethod getMethod();
--- a/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/HotSpotCryptoSubstitutionTest.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/HotSpotCryptoSubstitutionTest.java Mon May 11 17:12:15 2015 +0200 @@ -132,7 +132,7 @@ StructuredGraph graph = new StructuredGraph(substMethod, AllowAssumptions.YES); Plugins plugins = new Plugins(((HotSpotProviders) getProviders()).getGraphBuilderPlugins()); GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); - IntrinsicContext initialReplacementContext = new IntrinsicContext(installedCodeOwner, substMethod, null, ROOT_COMPILATION_BCI); + IntrinsicContext initialReplacementContext = new IntrinsicContext(installedCodeOwner, substMethod, ROOT_COMPILATION_BCI); new GraphBuilderPhase.Instance(getMetaAccess(), getProviders().getStampProvider(), getConstantReflection(), config, OptimisticOptimizations.NONE, initialReplacementContext).apply(graph); Assert.assertNotNull(getCode(installedCodeOwner, graph, true)); atLeastOneCompiled = true;
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/SnippetStub.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/stubs/SnippetStub.java Mon May 11 17:12:15 2015 +0200 @@ -22,6 +22,8 @@ */ package com.oracle.graal.hotspot.stubs; +import static com.oracle.graal.java.IntrinsicContext.*; + import java.lang.reflect.*; import com.oracle.graal.api.meta.*; @@ -94,7 +96,7 @@ assert SnippetGraphUnderConstruction.get() == null; SnippetGraphUnderConstruction.set(graph); - ReplacementContext initialReplacementContext = new ReplacementContext(method, method); + ReplacementContext initialReplacementContext = new IntrinsicContext(method, method, POST_PARSE_INLINE_BCI); new GraphBuilderPhase.Instance(metaAccess, providers.getStampProvider(), providers.getConstantReflection(), config, OptimisticOptimizations.NONE, initialReplacementContext).apply(graph); SnippetGraphUnderConstruction.set(null);
--- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Mon May 11 17:12:15 2015 +0200 @@ -52,6 +52,7 @@ import com.oracle.graal.graphbuilderconf.InvocationPlugins.InvocationPluginReceiver; import com.oracle.graal.java.BciBlockMapping.BciBlock; import com.oracle.graal.java.BciBlockMapping.ExceptionDispatchBlock; +import com.oracle.graal.java.GraphBuilderPhase.Instance.BytecodeParser; import com.oracle.graal.nodeinfo.*; import com.oracle.graal.nodes.*; import com.oracle.graal.nodes.CallTargetNode.InvokeKind; @@ -84,6 +85,87 @@ return graphBuilderConfig; } + static class ReplacementScope implements AutoCloseable { + FrameState stateBefore; + final Mark mark; + final BytecodeParser parser; + + static ReplacementScope create(boolean enteringReplacement, BytecodeParser parser, HIRFrameStateBuilder currentFrameState, ValueNode[] args) { + if (enteringReplacement) { + return new ReplacementScope(parser, currentFrameState, args); + } + return null; + } + + public ReplacementScope(BytecodeParser parser, HIRFrameStateBuilder currentFrameState, ValueNode[] args) { + this.parser = parser; + if (args == null) { + assert parser.parent == null; + assert parser.bci() == 0; + mark = null; + } else { + mark = parser.getGraph().getMark(); + for (ValueNode arg : args) { + currentFrameState.push(arg.getKind(), arg); + } + stateBefore = currentFrameState.create(parser.bci(), null); + for (int i = args.length - 1; i >= 0; i--) { + ValueNode arg = args[i]; + currentFrameState.pop(arg.getKind()); + } + } + } + + public void close() { + IntrinsicContext intrinsic = (IntrinsicContext) parser.replacementContext; + if (intrinsic != null && intrinsic.isPostParseInlined()) { + return; + } + + FrameState stateAfterReturn = null; + StructuredGraph graph = parser.getGraph(); + for (Node node : graph.getNewNodes(mark)) { + if (node instanceof FrameState) { + FrameState fs = (FrameState) node; + if (BytecodeFrame.isPlaceholderBci(fs.bci)) { + if (fs.bci == BytecodeFrame.AFTER_BCI) { + if (fs.stackSize() != 0) { + assert fs.usages().count() == 1; + ValueNode returnVal = fs.stackAt(0); + assert returnVal == fs.usages().first(); + + ValueNode tos = parser.frameState.pop(returnVal.getKind()); + parser.frameState.push(returnVal.getKind(), returnVal); + FrameState newFs = parser.frameState.create(parser.stream.nextBCI(), null); + fs.replaceAndDelete(newFs); + parser.frameState.pop(returnVal.getKind()); + parser.frameState.push(tos.getKind(), tos); + } else { + if (stateAfterReturn == null) { + if (intrinsic != null && intrinsic.isCompilationRoot()) { + stateAfterReturn = graph.add(new FrameState(BytecodeFrame.INVALID_FRAMESTATE_BCI)); + } else { + stateAfterReturn = parser.frameState.create(parser.stream.nextBCI(), null); + } + } + fs.replaceAndDelete(stateAfterReturn); + } + } else if (fs.bci == BytecodeFrame.BEFORE_BCI) { + if (stateBefore == null) { + stateBefore = graph.start().stateAfter(); + } + if (stateBefore != fs) { + fs.replaceAndDelete(stateBefore); + } + } else { + assert fs.bci == BytecodeFrame.INVALID_FRAMESTATE_BCI; + } + } + } + } + } + } + // Fully qualified name is a workaround for JDK-8056066 public static class Instance extends com.oracle.graal.phases.Phase { @@ -123,8 +205,10 @@ HIRFrameStateBuilder frameState = new HIRFrameStateBuilder(parser, method, graph); frameState.initializeForMethodStart(graphBuilderConfig.eagerResolving() || replacementContext != null, graphBuilderConfig.getPlugins().getParameterPlugin()); - parser.build(graph.start(), frameState); + try (ReplacementScope rs = ReplacementScope.create(replacementContext != null, parser, frameState, null)) { + parser.build(graph.start(), frameState); + } GraphUtil.normalizeLoops(graph); // Remove dead parameters. @@ -327,17 +411,27 @@ if (startInstruction == graph.start()) { StartNode startNode = graph.start(); if (method.isSynchronized()) { - startNode.setStateAfter(createFrameState(BytecodeFrame.BEFORE_BCI)); + assert !parsingReplacement(); + startNode.setStateAfter(createFrameState(BytecodeFrame.BEFORE_BCI, startNode)); } else { - - if (graph.method() != null && graph.method().isJavaLangObjectInit()) { - // Don't clear the receiver when Object.<init> is the compilation - // root. The receiver is needed as input to RegisterFinalizerNode. + if (!parsingReplacement()) { + if (graph.method() != null && graph.method().isJavaLangObjectInit()) { + /* + * Don't clear the receiver when Object.<init> is the + * compilation root. The receiver is needed as input to + * RegisterFinalizerNode. + */ + } else { + frameState.clearNonLiveLocals(startBlock, liveness, true); + } + assert bci() == 0; + startNode.setStateAfter(createFrameState(bci(), startNode)); } else { - frameState.clearNonLiveLocals(startBlock, liveness, true); + if (startNode.stateAfter() == null) { + FrameState stateAfterStart = createStateAfterStartOfReplacementGraph(); + startNode.setStateAfter(stateAfterStart); + } } - assert bci() == 0; - startNode.setStateAfter(createFrameState(bci())); } } @@ -379,6 +473,42 @@ } } + /** + * Creates the frame state after the start node of a graph for a + * {@link ReplacementContext replacement} that is the parse root (either for root + * compiling or for post-parse inlining). + */ + private FrameState createStateAfterStartOfReplacementGraph() { + assert parent == null; + assert frameState.method.equals(replacementContext.replacement); + assert bci() == 0; + assert frameState.stackSize == 0; + FrameState stateAfterStart; + if (replacementContext.isIntrinsic() && replacementContext.asIntrinsic().isPostParseInlined()) { + stateAfterStart = graph.add(new FrameState(BytecodeFrame.BEFORE_BCI)); + } else { + ResolvedJavaMethod original = replacementContext.method; + ValueNode[] locals; + if (original.getMaxLocals() == frameState.localsSize() || original.isNative()) { + locals = frameState.locals; + } else { + locals = new ValueNode[original.getMaxLocals()]; + int parameterCount = original.getSignature().getParameterCount(!original.isStatic()); + for (int i = 0; i < parameterCount; i++) { + ValueNode param = frameState.locals[i]; + locals[i] = param; + assert param == null || param instanceof ParameterNode || param.isConstant(); + } + } + ValueNode[] stack = {}; + int stackSize = 0; + ValueNode[] locks = {}; + List<MonitorIdNode> monitorIds = Collections.emptyList(); + stateAfterStart = graph.add(new FrameState(null, original, 0, locals, stack, stackSize, locks, monitorIds, false, false)); + } + return stateAfterStart; + } + private void detectLoops(FixedNode startInstruction) { NodeBitMap visited = graph.createNodeBitMap(); NodeBitMap active = graph.createNodeBitMap(); @@ -735,11 +865,11 @@ dispatchBegin = graph.add(new ExceptionObjectNode(metaAccess)); dispatchState.apush(dispatchBegin); dispatchState.setRethrowException(true); - dispatchBegin.setStateAfter(dispatchState.create(bci)); + dispatchBegin.setStateAfter(dispatchState.create(bci, dispatchBegin)); } else { dispatchBegin = graph.add(new DispatchBeginNode()); dispatchState.apush(exceptionObject); - dispatchBegin.setStateAfter(dispatchState.create(bci)); + dispatchBegin.setStateAfter(dispatchState.create(bci, dispatchBegin)); dispatchState.setRethrowException(true); } this.controlFlowSplit = true; @@ -953,7 +1083,7 @@ append(new IfNode(graph.unique(new IsNullNode(receiver)), exception, falseSucc, 0.01)); lastInstr = falseSucc; - exception.setStateAfter(createFrameState(bci())); + exception.setStateAfter(createFrameState(bci(), exception)); exception.setNext(handleException(exception, bci())); return nonNullReceiver; } @@ -965,7 +1095,7 @@ append(new IfNode(graph.unique(IntegerBelowNode.create(index, length, constantReflection)), trueSucc, exception, 0.99)); lastInstr = trueSucc; - exception.setStateAfter(createFrameState(bci())); + exception.setStateAfter(createFrameState(bci(), exception)); exception.setNext(handleException(exception, bci())); } @@ -978,7 +1108,7 @@ protected void genStoreField(ValueNode receiver, ResolvedJavaField field, ValueNode value) { StoreFieldNode storeFieldNode = new StoreFieldNode(receiver, field, value); append(storeFieldNode); - storeFieldNode.setStateAfter(this.createFrameState(stream.nextBCI())); + storeFieldNode.setStateAfter(this.createFrameState(stream.nextBCI(), storeFieldNode)); } /** @@ -1217,8 +1347,8 @@ for (Node n : newNodes) { if (n instanceof StateSplit) { StateSplit stateSplit = (StateSplit) n; - assert stateSplit.stateAfter() != null : error("%s node added by plugin for %s need to have a non-null frame state: %s", StateSplit.class.getSimpleName(), - targetMethod.format("%H.%n(%p)"), stateSplit); + assert stateSplit.stateAfter() != null || !stateSplit.hasSideEffect() : error("%s node added by plugin for %s need to have a non-null frame state: %s", + StateSplit.class.getSimpleName(), targetMethod.format("%H.%n(%p)"), stateSplit); } } try { @@ -1294,14 +1424,18 @@ if (intrinsic != null) { if (intrinsic.isCompilationRoot()) { // A root compiled intrinsic needs to deoptimize - // if the slow path is taken - DeoptimizeNode deopt = append(new DeoptimizeNode(InvalidateRecompile, RuntimeConstraint)); - deopt.setStateBefore(intrinsic.getInvokeStateBefore(graph, null)); + // if the slow path is taken. During frame state + // assignment, the deopt node will get its stateBefore + // from the start node of the intrinsic + append(new DeoptimizeNode(InvalidateRecompile, RuntimeConstraint)); return true; } else { // Otherwise inline the original method. Any frame state created // during the inlining will exclude frame(s) in the // intrinsic method (see HIRFrameStateBuilder.create(int bci)). + if (context.method.isNative()) { + return false; + } parseAndInlineCallee(context.method, args, null); return true; } @@ -1319,8 +1453,9 @@ if (context == null && isReplacement) { assert !inlinedMethod.equals(targetMethod); if (isIntrinsic) { - context = new IntrinsicContext(targetMethod, inlinedMethod, args, bci); + context = new IntrinsicContext(targetMethod, inlinedMethod, bci); } else { + assert false; context = new ReplacementContext(targetMethod, inlinedMethod); } } @@ -1370,33 +1505,37 @@ } private void parseAndInlineCallee(ResolvedJavaMethod targetMethod, ValueNode[] args, ReplacementContext calleeReplacementContext) { - BytecodeParser parser = new BytecodeParser(this, metaAccess, targetMethod, graphBuilderConfig, optimisticOpts, INVOCATION_ENTRY_BCI, calleeReplacementContext); - HIRFrameStateBuilder startFrameState = new HIRFrameStateBuilder(parser, targetMethod, graph); - if (!targetMethod.isStatic()) { - args[0] = nullCheckedValue(args[0]); - } - startFrameState.initializeFromArgumentsArray(args); - parser.build(this.lastInstr, startFrameState); + try (ReplacementScope rs = ReplacementScope.create(calleeReplacementContext != null && !parsingReplacement(), this, frameState, args)) { + + BytecodeParser parser = new BytecodeParser(this, metaAccess, targetMethod, graphBuilderConfig, optimisticOpts, INVOCATION_ENTRY_BCI, calleeReplacementContext); + HIRFrameStateBuilder startFrameState = new HIRFrameStateBuilder(parser, targetMethod, graph); + if (!targetMethod.isStatic()) { + args[0] = nullCheckedValue(args[0]); + } + startFrameState.initializeFromArgumentsArray(args); + parser.build(this.lastInstr, startFrameState); - FixedWithNextNode calleeBeforeReturnNode = parser.getBeforeReturnNode(); - this.lastInstr = calleeBeforeReturnNode; - if (calleeBeforeReturnNode != null) { - ValueNode calleeReturnValue = parser.getReturnValue(); - if (calleeReturnValue != null) { - frameState.push(targetMethod.getSignature().getReturnKind().getStackKind(), calleeReturnValue); + FixedWithNextNode calleeBeforeReturnNode = parser.getBeforeReturnNode(); + this.lastInstr = calleeBeforeReturnNode; + Kind calleeReturnKind = targetMethod.getSignature().getReturnKind(); + if (calleeBeforeReturnNode != null) { + ValueNode calleeReturnValue = parser.getReturnValue(); + if (calleeReturnValue != null) { + frameState.push(calleeReturnKind.getStackKind(), calleeReturnValue); + } } - } - FixedWithNextNode calleeBeforeUnwindNode = parser.getBeforeUnwindNode(); - if (calleeBeforeUnwindNode != null) { - ValueNode calleeUnwindValue = parser.getUnwindValue(); - assert calleeUnwindValue != null; - calleeBeforeUnwindNode.setNext(handleException(calleeUnwindValue, bci())); - } + FixedWithNextNode calleeBeforeUnwindNode = parser.getBeforeUnwindNode(); + if (calleeBeforeUnwindNode != null) { + ValueNode calleeUnwindValue = parser.getUnwindValue(); + assert calleeUnwindValue != null; + calleeBeforeUnwindNode.setNext(handleException(calleeUnwindValue, bci())); + } - // Record inlined method dependency in the graph - if (graph.isInlinedMethodRecordingEnabled()) { - graph.getInlinedMethods().add(targetMethod); + // Record inlined method dependency in the graph + if (graph.isInlinedMethodRecordingEnabled()) { + graph.getInlinedMethods().add(targetMethod); + } } } @@ -1407,7 +1546,7 @@ protected InvokeNode createInvoke(CallTargetNode callTarget, Kind resultType) { InvokeNode invoke = append(new InvokeNode(callTarget, bci())); frameState.pushReturn(resultType, invoke); - invoke.setStateAfter(createFrameState(stream.nextBCI())); + invoke.setStateAfter(createFrameState(stream.nextBCI(), invoke)); return invoke; } @@ -1415,13 +1554,35 @@ DispatchBeginNode exceptionEdge = handleException(null, bci()); InvokeWithExceptionNode invoke = append(new InvokeWithExceptionNode(callTarget, exceptionEdge, bci())); frameState.pushReturn(resultType, invoke); - invoke.setStateAfter(createFrameState(stream.nextBCI())); + invoke.setStateAfter(createFrameState(stream.nextBCI(), invoke)); return invoke; } @Override protected void genReturn(ValueNode returnVal, Kind returnKind) { - + if (parsingReplacement() && returnVal != null) { + if (returnVal instanceof StateSplit) { + StateSplit stateSplit = (StateSplit) returnVal; + FrameState stateAfter = stateSplit.stateAfter(); + if (stateSplit.hasSideEffect()) { + assert stateSplit != null; + if (stateAfter.bci == BytecodeFrame.AFTER_BCI) { + assert stateAfter.usages().count() == 1; + assert stateAfter.usages().first() == stateSplit; + stateAfter.replaceAtUsages(graph.add(new FrameState(BytecodeFrame.AFTER_BCI, returnVal))); + GraphUtil.killWithUnusedFloatingInputs(stateAfter); + } else { + /* + * This must be the return value from within a partial + * intrinsification. + */ + assert !BytecodeFrame.isPlaceholderBci(stateAfter.bci); + } + } else { + assert stateAfter == null; + } + } + } if (parent == null) { frameState.setRethrowException(false); frameState.clearStack(); @@ -1465,7 +1626,7 @@ MonitorIdNode monitorId = graph.add(new MonitorIdNode(frameState.lockDepth())); MonitorEnterNode monitorEnter = append(new MonitorEnterNode(x, monitorId)); frameState.pushLock(x, monitorId); - monitorEnter.setStateAfter(createFrameState(bci)); + monitorEnter.setStateAfter(createFrameState(bci, monitorEnter)); } @Override @@ -1476,7 +1637,7 @@ throw bailout(String.format("unbalanced monitors: mismatch at monitorexit, %s != %s", GraphUtil.originalValue(x), GraphUtil.originalValue(lockedObject))); } MonitorExitNode monitorExit = append(new MonitorExitNode(x, monitorId, escapedReturnValue)); - monitorExit.setStateAfter(createFrameState(bci)); + monitorExit.setStateAfter(createFrameState(bci, monitorExit)); } @Override @@ -1627,7 +1788,7 @@ lastLoopExit = loopExit; Debug.log("Target %s Exits %s, scanning framestates...", targetBlock, loop); newState.insertLoopProxies(loopExit, getEntryState(loop, this.getCurrentDimension())); - loopExit.setStateAfter(newState.create(bci)); + loopExit.setStateAfter(newState.create(bci, loopExit)); } lastLoopExit.setNext(target); @@ -1972,7 +2133,7 @@ if (block instanceof ExceptionDispatchBlock) { bci = ((ExceptionDispatchBlock) block).deoptBci; } - abstractMergeNode.setStateAfter(createFrameState(bci)); + abstractMergeNode.setStateAfter(createFrameState(bci, abstractMergeNode)); } } @@ -2055,7 +2216,7 @@ // Create phi functions for all local variables and operand stack slots. frameState.insertLoopPhis(liveness, block.loopId, loopBegin); - loopBegin.setStateAfter(createFrameState(block.startBci)); + loopBegin.setStateAfter(createFrameState(block.startBci, loopBegin)); /* * We have seen all forward branches. All subsequent backward branches will @@ -2103,7 +2264,7 @@ } EntryMarkerNode x = append(new EntryMarkerNode()); frameState.insertProxies(x); - x.setStateAfter(createFrameState(bci)); + x.setStateAfter(createFrameState(bci, x)); } try { @@ -2120,7 +2281,7 @@ bci = stream.currentBCI(); assert block == currentBlock; - assert !(lastInstr instanceof StateSplit) || lastInstr instanceof BeginNode || ((StateSplit) lastInstr).stateAfter() != null : lastInstr; + assert checkLastInstruction(); lastInstr = finishInstruction(lastInstr, frameState); if (bci < endBCI) { if (bci > block.endBci) { @@ -2134,6 +2295,18 @@ } } + protected boolean checkLastInstruction() { + if (lastInstr instanceof BeginNode) { + // ignore + } else if (lastInstr instanceof StateSplit) { + StateSplit stateSplit = (StateSplit) lastInstr; + if (stateSplit.hasSideEffect()) { + assert stateSplit.stateAfter() != null : "side effect " + lastInstr + " requires a non-null stateAfter"; + } + } + return true; + } + private LoopBeginNode appendLoopBegin(FixedWithNextNode fixedWithNext) { EndNode preLoopEnd = graph.add(new EndNode()); LoopBeginNode loopBegin = graph.add(new LoopBeginNode()); @@ -2156,7 +2329,7 @@ private InfopointNode createInfoPointNode(InfopointReason reason) { if (graphBuilderConfig.insertFullDebugInfo()) { - return new FullInfopointNode(reason, createFrameState(bci())); + return new FullInfopointNode(reason, createFrameState(bci(), null)); } else { return new SimpleInfopointNode(reason, new BytecodePosition(null, method, bci())); } @@ -2409,22 +2582,23 @@ } public BailoutException bailout(String string) { - FrameState currentFrameState = createFrameState(bci()); + FrameState currentFrameState = createFrameState(bci(), null); StackTraceElement[] elements = GraphUtil.approxSourceStackTraceElement(currentFrameState); BailoutException bailout = new BailoutException(string); throw GraphUtil.createBailoutException(string, bailout, elements); } - private FrameState createFrameState(int bci) { + private FrameState createFrameState(int bci, StateSplit forStateSplit) { if (currentBlock != null && bci > currentBlock.endBci) { frameState.clearNonLiveLocals(currentBlock, liveness, false); } - return frameState.create(bci); + return frameState.create(bci, forStateSplit); } - public void setStateAfter(StateSplit stateSplit) { - FrameState stateAfter = createFrameState(stream.nextBCI()); - stateSplit.setStateAfter(stateAfter); + public void setStateAfter(StateSplit sideEffect) { + assert sideEffect.hasSideEffect(); + FrameState stateAfter = createFrameState(stream.nextBCI(), sideEffect); + sideEffect.setStateAfter(stateAfter); } } }
--- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java Mon May 11 17:12:15 2015 +0200 @@ -62,6 +62,12 @@ private FrameState outerFrameState; /** + * The closest {@link StateSplit#hasSideEffect() side-effect} predecessors. There will be more + * than one when the current block contains no side-effects but merging predecessor blocks do. + */ + protected StateSplit[] lastSideEffects; + + /** * Creates a new frame state builder for the given method and the given target graph. * * @param method the method whose frame is simulated @@ -144,14 +150,6 @@ javaIndex += kind.getSlotCount(); index++; } - - if (parser.replacementContext instanceof IntrinsicContext) { - IntrinsicContext intrinsic = (IntrinsicContext) parser.replacementContext; - if (intrinsic.isCompilationRoot()) { - // Records the parameters to an root compiled intrinsic - intrinsic.args = locals.clone(); - } - } } private HIRFrameStateBuilder(HIRFrameStateBuilder other) { @@ -202,25 +200,22 @@ return sb.toString(); } - public FrameState create(int bci) { - BytecodeParser parent = parser.getParent(); + public FrameState create(int bci, StateSplit forStateSplit) { if (parser.parsingReplacement()) { IntrinsicContext intrinsic = parser.replacementContext.asIntrinsic(); if (intrinsic != null) { - return intrinsic.getInvokeStateBefore(parser.getGraph(), parent); + return intrinsic.createFrameState(parser.getGraph(), this, forStateSplit); } } - // If this is the recursive call in a partial intrinsification - // the frame(s) of the intrinsic method are omitted - while (parent != null && parent.parsingReplacement() && parent.replacementContext.asIntrinsic() != null) { - parent = parent.getParent(); - } + + // Skip intrinsic frames + BytecodeParser parent = (BytecodeParser) parser.getNonReplacementAncestor(); return create(bci, parent, false); } public FrameState create(int bci, BytecodeParser parent, boolean duringCall) { if (outerFrameState == null && parent != null) { - outerFrameState = parent.getFrameState().create(parent.bci()); + outerFrameState = parent.getFrameState().create(parent.bci(), null); } if (bci == BytecodeFrame.AFTER_EXCEPTION_BCI && parent != null) { FrameState newFrameState = outerFrameState.duplicateModified(outerFrameState.bci, true, Kind.Void, this.peek(0)); @@ -283,6 +278,17 @@ lockedObjects[i] = merge(lockedObjects[i], other.lockedObjects[i], block); assert monitorIds[i] == other.monitorIds[i]; } + + if (lastSideEffects == null) { + lastSideEffects = other.lastSideEffects; + } else { + if (other.lastSideEffects != null) { + int thisLength = lastSideEffects.length; + int otherLength = other.lastSideEffects.length; + lastSideEffects = Arrays.copyOf(lastSideEffects, thisLength + otherLength); + System.arraycopy(other.lastSideEffects, 0, lastSideEffects, thisLength, otherLength); + } + } } private ValueNode merge(ValueNode currentValue, ValueNode otherValue, AbstractMergeNode block) { @@ -970,4 +976,15 @@ } } } + + public void addLastSideEffect(StateSplit sideEffect) { + assert sideEffect != null; + assert sideEffect.hasSideEffect(); + if (lastSideEffects == null) { + lastSideEffects = new StateSplit[]{sideEffect}; + } else { + lastSideEffects = Arrays.copyOf(lastSideEffects, lastSideEffects.length + 1); + lastSideEffects[lastSideEffects.length - 1] = sideEffect; + } + } }
--- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/IntrinsicContext.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/IntrinsicContext.java Mon May 11 17:12:15 2015 +0200 @@ -22,25 +22,19 @@ */ package com.oracle.graal.java; -import static com.oracle.graal.java.HIRFrameStateBuilder.*; - -import java.util.*; +import static com.oracle.graal.api.code.BytecodeFrame.*; -import com.oracle.graal.api.code.*; import com.oracle.graal.api.meta.*; -import com.oracle.graal.java.GraphBuilderPhase.Instance.*; import com.oracle.graal.nodes.*; -import com.oracle.graal.nodes.java.*; /** - * Context for a replacement being inlined as a compiler intrinsic. Deoptimization within a compiler - * intrinsic must replay the intrinsified call. This context object retains the information required - * to build a frame state denoting the JVM state just before the intrinsified call. + * Context for a replacement being inlined as a compiler intrinsic. */ public class IntrinsicContext extends ReplacementContext { /** - * BCI denoting an intrinsic is being parsed for inlining after the caller has been parsed. + * BCI denoting an intrinsic is being parsed for inlining after the graph of the caller has been + * built. */ public static final int POST_PARSE_INLINE_BCI = -1; @@ -50,24 +44,15 @@ public static final int ROOT_COMPILATION_BCI = -2; /** - * The arguments to the intrinsic. - */ - ValueNode[] args; - - /** * The BCI of the intrinsified invocation, {@link #POST_PARSE_INLINE_BCI} or * {@link #ROOT_COMPILATION_BCI}. */ final int bci; - private FrameState stateBeforeCache; - - public IntrinsicContext(ResolvedJavaMethod method, ResolvedJavaMethod substitute, ValueNode[] args, int bci) { + public IntrinsicContext(ResolvedJavaMethod method, ResolvedJavaMethod substitute, int bci) { super(method, substitute); - assert bci != POST_PARSE_INLINE_BCI || args == null; - this.args = args; this.bci = bci; - assert !isCompilationRoot() || method.hasBytecodes() : "Cannot intrinsic for native or abstract method " + method.format("%H.%n(%p)"); + assert !isCompilationRoot() || method.hasBytecodes() : "Cannot root compile intrinsic for native or abstract method " + method.format("%H.%n(%p)"); } @Override @@ -83,34 +68,33 @@ return bci == ROOT_COMPILATION_BCI; } - public FrameState getInvokeStateBefore(StructuredGraph graph, BytecodeParser parent) { - if (isCompilationRoot()) { - int maxLocals = method.getMaxLocals(); - // The 'args' were initialized based on the intrinsic method but a - // frame state's 'locals' needs to have the same length as the frame - // state method's 'max_locals'. - ValueNode[] locals = maxLocals == args.length ? args : Arrays.copyOf(args, maxLocals); - ValueNode[] stack = EMPTY_ARRAY; - int stackSize = 0; - ValueNode[] locks = EMPTY_ARRAY; - List<MonitorIdNode> monitorIds = Collections.emptyList(); - return graph.add(new FrameState(null, method, 0, locals, stack, stackSize, locks, monitorIds, false, false)); - } else if (isPostParseInlined()) { - return graph.add(new FrameState(BytecodeFrame.BEFORE_BCI)); + public FrameState createFrameState(StructuredGraph graph, HIRFrameStateBuilder frameState, StateSplit forStateSplit) { + assert forStateSplit != graph.start(); + if (forStateSplit.hasSideEffect()) { + if (frameState.lastSideEffects != null) { + // Only the last side effect on any execution path in a replacement + // can inherit the stateAfter of the replaced node + FrameState invalid = graph.add(new FrameState(INVALID_FRAMESTATE_BCI)); + for (StateSplit lastSideEffect : frameState.lastSideEffects) { + lastSideEffect.setStateAfter(invalid); + } + } + frameState.addLastSideEffect(forStateSplit); + return graph.add(new FrameState(AFTER_BCI)); } else { - assert !parent.parsingReplacement() || parent.replacementContext instanceof IntrinsicContext; - if (stateBeforeCache == null) { - - // Find the non-intrinsic ancestor calling the intrinsified method - BytecodeParser ancestor = parent; - while (ancestor.parsingReplacement()) { - assert ancestor.replacementContext instanceof IntrinsicContext; - ancestor = ancestor.getParent(); + if (forStateSplit instanceof AbstractMergeNode) { + // Merge nodes always need a frame state + if (frameState.lastSideEffects != null) { + // A merge after one or more side effects + return graph.add(new FrameState(AFTER_BCI)); + } else { + // A merge before any side effects + return graph.add(new FrameState(BEFORE_BCI)); } - FrameState stateDuring = ancestor.getFrameState().create(ancestor.bci(), ancestor.getParent(), true); - stateBeforeCache = stateDuring.duplicateModifiedBeforeCall(bci, Kind.Void, args); + } else { + // Other non-side-effects do not need a state + return null; } - return stateBeforeCache; } } @@ -121,7 +105,6 @@ @Override public String toString() { - return "Intrinsic{original: " + method.format("%H.%n(%p)") + ", replacement: " + replacement.format("%H.%n(%p)") + ", bci: " + bci + (args == null ? "" : ", args: " + Arrays.toString(args)) + - (stateBeforeCache == null ? "" : ", stateBefore: " + stateBeforeCache) + "}"; + return "Intrinsic{original: " + method.format("%H.%n(%p)") + ", replacement: " + replacement.format("%H.%n(%p)") + ", bci: " + bci + "}"; } }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java Mon May 11 17:12:15 2015 +0200 @@ -117,6 +117,19 @@ bci == BytecodeFrame.INVALID_FRAMESTATE_BCI; } + /** + * Creates a placeholder frame state with a single element on the stack representing a return + * value. This allows the parsing of an intrinsic to communicate the returned value in a + * {@link StateSplit#stateAfter() stateAfter} to the inlining call site. + * + * @param bci this must be {@link BytecodeFrame#AFTER_BCI} + */ + public FrameState(int bci, ValueNode returnValue) { + this(null, null, bci, 0, returnValue.getKind().getSlotCount(), 0, false, false, null, Collections.<EscapeObjectState> emptyList()); + assert bci == BytecodeFrame.AFTER_BCI; + this.values.initialize(0, returnValue); + } + public FrameState(FrameState outerFrameState, ResolvedJavaMethod method, int bci, ValueNode[] locals, ValueNode[] stack, int stackSize, ValueNode[] locks, List<MonitorIdNode> monitorIds, boolean rethrowException, boolean duringCall) { this(outerFrameState, method, bci, locals.length, stackSize, locks.length, rethrowException, duringCall, monitorIds, Collections.<EscapeObjectState> emptyList()); @@ -410,7 +423,11 @@ String nl = CodeUtil.NEW_LINE; FrameState fs = frameState; while (fs != null) { - MetaUtil.appendLocation(sb, fs.method, fs.bci).append(nl); + MetaUtil.appendLocation(sb, fs.method, fs.bci); + if (BytecodeFrame.isPlaceholderBci(fs.bci)) { + sb.append("//").append(getPlaceholderBciName(fs.bci)); + } + sb.append(nl); sb.append("locals: ["); for (int i = 0; i < fs.localsSize(); i++) { sb.append(i == 0 ? "" : ", ").append(fs.localAt(i) == null ? "_" : fs.localAt(i).toString(Verbosity.Id)); @@ -434,7 +451,11 @@ if (verbosity == Verbosity.Debugger) { return toString(this); } else if (verbosity == Verbosity.Name) { - return super.toString(Verbosity.Name) + "@" + bci; + String res = super.toString(Verbosity.Name) + "@" + bci; + if (BytecodeFrame.isPlaceholderBci(bci)) { + res += "[" + getPlaceholderBciName(bci) + "]"; + } + return res; } else { return super.toString(verbosity); }
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ForeignCallNode.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/ForeignCallNode.java Mon May 11 17:12:15 2015 +0200 @@ -107,6 +107,12 @@ } @Override + public void setStateAfter(FrameState x) { + assert hasSideEffect() || x == null; + super.setStateAfter(x); + } + + @Override public FrameState stateDuring() { return stateDuring; }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/inlining/InliningUtil.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/inlining/InliningUtil.java Mon May 11 17:12:15 2015 +0200 @@ -468,14 +468,28 @@ Kind invokeReturnKind = invoke.asNode().getKind(); if (frameState.bci == BytecodeFrame.AFTER_BCI) { + FrameState stateAfterReturn = stateAtReturn; + if (frameState.method() == null) { + // This is a frame state for a side effect within an intrinsic + // that was parsed for post-parse intrinsification + for (Node usage : frameState.usages()) { + if (usage instanceof ForeignCallNode) { + // A foreign call inside an intrinsic needs to have + // the BCI of the invoke being intrinsified + ForeignCallNode foreign = (ForeignCallNode) usage; + foreign.setBci(invoke.bci()); + } + } + } + /* * pop return kind from invoke's stateAfter and replace with this frameState's return * value (top of stack) */ - FrameState stateAfterReturn = stateAtReturn; if (invokeReturnKind != Kind.Void && (alwaysDuplicateStateAfter || (frameState.stackSize() > 0 && stateAfterReturn.stackAt(0) != frameState.stackAt(0)))) { stateAfterReturn = stateAtReturn.duplicateModified(invokeReturnKind, frameState.stackAt(0)); } + frameState.replaceAndDelete(stateAfterReturn); return stateAfterReturn; } else if (stateAtExceptionEdge != null && isStateAfterException(frameState)) { @@ -581,25 +595,26 @@ ValueNode singleReturnValue = null; PhiNode returnValuePhi = null; for (ReturnNode returnNode : returnNodes) { - if (returnNode.result() != null) { - if (returnValuePhi == null && (singleReturnValue == null || singleReturnValue == returnNode.result())) { + ValueNode result = returnNode.result(); + if (result != null) { + if (returnValuePhi == null && (singleReturnValue == null || singleReturnValue == result)) { /* Only one return value, so no need yet for a phi node. */ - singleReturnValue = returnNode.result(); + singleReturnValue = result; } else if (returnValuePhi == null) { /* Found a second return value, so create phi node. */ - returnValuePhi = merge.graph().addWithoutUnique(new ValuePhiNode(returnNode.result().stamp().unrestricted(), merge)); + returnValuePhi = merge.graph().addWithoutUnique(new ValuePhiNode(result.stamp().unrestricted(), merge)); if (canonicalizedNodes != null) { canonicalizedNodes.add(returnValuePhi); } for (int i = 0; i < merge.forwardEndCount(); i++) { returnValuePhi.addInput(singleReturnValue); } - returnValuePhi.addInput(returnNode.result()); + returnValuePhi.addInput(result); } else { /* Multiple return values, just add to existing phi node. */ - returnValuePhi.addInput(returnNode.result()); + returnValuePhi.addInput(result); } }
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/DefaultGenericInvocationPlugin.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/DefaultGenericInvocationPlugin.java Mon May 11 17:12:15 2015 +0200 @@ -145,12 +145,18 @@ b.add(new UnsafeStoreNode(copy.destinationObject(), copy.destinationOffset(), value, copy.accessKind(), copy.getLocationIdentity())); return true; } else if (res instanceof ForeignCallNode) { - ForeignCallNode foreign = (ForeignCallNode) res; - foreign.setBci(b.bci()); + /* + * Need to update the BCI of a ForeignCallNode so that it gets the stateDuring in the + * case that the foreign call can deoptimize. As with all deoptimization, we need a + * state in a normal method as opposed to an intrinsic. + */ + GraphBuilderContext ancestor = b.getNonReplacementAncestor(); + if (ancestor != null) { + ForeignCallNode foreign = (ForeignCallNode) res; + foreign.setBci(ancestor.bci()); + } } - res = b.add(res); - boolean nonValueType = false; if (returnKind == Kind.Object && stamp instanceof ObjectStamp) { ResolvedJavaType type = ((ObjectStamp) stamp).type(); @@ -162,16 +168,10 @@ if (returnKind != Kind.Void) { assert nonValueType || res.getKind().getStackKind() != Kind.Void; - b.push(returnKind.getStackKind(), res); + res = b.addPush(returnKind.getStackKind(), res); } else { assert res.getKind().getStackKind() == Kind.Void; - } - - if (res instanceof StateSplit) { - StateSplit stateSplit = (StateSplit) res; - if (stateSplit.stateAfter() == null) { - b.setStateAfter(stateSplit); - } + res = b.add(res); } return true;
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java Mon May 11 17:12:15 2015 +0200 @@ -152,7 +152,7 @@ if (invoke.getKind() != Kind.Void) { frameStateBuilder.push(returnType.getKind(), invoke); } - invoke.setStateAfter(frameStateBuilder.create(bci)); + invoke.setStateAfter(frameStateBuilder.create(bci, invoke)); if (invoke.getKind() != Kind.Void) { frameStateBuilder.pop(returnType.getKind()); } @@ -219,7 +219,7 @@ GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); StructuredGraph calleeGraph = new StructuredGraph(method, AllowAssumptions.NO); - IntrinsicContext initialReplacementContext = new IntrinsicContext(method, method, null, IntrinsicContext.POST_PARSE_INLINE_BCI); + IntrinsicContext initialReplacementContext = new IntrinsicContext(method, method, IntrinsicContext.POST_PARSE_INLINE_BCI); new GraphBuilderPhase.Instance(metaAccess, providers.getStampProvider(), providers.getConstantReflection(), config, OptimisticOptimizations.NONE, initialReplacementContext).apply(calleeGraph); // Remove all frame states from inlinee
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/IntrinsicGraphBuilder.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/IntrinsicGraphBuilder.java Mon May 11 17:12:15 2015 +0200 @@ -152,9 +152,10 @@ return graph; } - public void setStateAfter(StateSplit stateSplit) { + public void setStateAfter(StateSplit sideEffect) { + assert sideEffect.hasSideEffect(); FrameState stateAfter = getGraph().add(new FrameState(BytecodeFrame.BEFORE_BCI)); - stateSplit.setStateAfter(stateAfter); + sideEffect.setStateAfter(stateAfter); } public GraphBuilderContext getParent() {
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon May 11 17:10:26 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon May 11 17:12:15 2015 +0200 @@ -25,6 +25,7 @@ import static com.oracle.graal.api.meta.MetaUtil.*; import static com.oracle.graal.compiler.common.GraalOptions.*; import static com.oracle.graal.java.AbstractBytecodeParser.Options.*; +import static com.oracle.graal.java.IntrinsicContext.*; import static com.oracle.graal.phases.common.DeadCodeEliminationPhase.Optionality.*; import static java.lang.String.*; @@ -629,12 +630,13 @@ OptimisticOptimizations optimisticOpts) { ReplacementContext initialReplacementContext = null; if (method.getAnnotation(Snippet.class) == null) { - // Late inlined intrinsic - initialReplacementContext = new IntrinsicContext(substitutedMethod, method, null, -1); + // Post-parse inlined intrinsic + initialReplacementContext = new IntrinsicContext(substitutedMethod, method, POST_PARSE_INLINE_BCI); } else { // Snippet ResolvedJavaMethod original = substitutedMethod != null ? substitutedMethod : method; - initialReplacementContext = new ReplacementContext(original, method); + // initialReplacementContext = new ReplacementContext(original, method); + initialReplacementContext = new IntrinsicContext(original, method, POST_PARSE_INLINE_BCI); } return new GraphBuilderPhase.Instance(metaAccess, stampProvider, constantReflection, graphBuilderConfig, optimisticOpts, initialReplacementContext); }