# HG changeset patch # User Michael Van De Vanter # Date 1433304959 25200 # Node ID 889b45a0dedd7e23868fe40bb9b680fae09cf15f # Parent afea1d08c3935cbfa51a3e150ec8932c12dbb292# Parent c74d3c9b9de7f007fe4339b987973830131ad0fe Merge with c74d3c9b9de7f007fe4339b987973830131ad0fe diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/NodePlugin.java --- a/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/NodePlugin.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/NodePlugin.java Tue Jun 02 21:15:59 2015 -0700 @@ -152,4 +152,15 @@ default boolean handleInstanceOf(GraphBuilderContext b, ValueNode object, ResolvedJavaType type, JavaTypeProfile profile) { return false; } + + /** + * If the plugin {@link GraphBuilderContext#push pushes} a value with a different {@link Kind} + * than specified by the bytecode, it must override this method and return {@code true}. This + * disables assertion checking for value kinds. + * + * @param b the context + */ + default boolean canChangeStackKind(GraphBuilderContext b) { + return false; + } } diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotNodePlugin.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotNodePlugin.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotNodePlugin.java Tue Jun 02 21:15:59 2015 -0700 @@ -66,6 +66,14 @@ } @Override + public boolean canChangeStackKind(GraphBuilderContext b) { + if (b.parsingIntrinsic()) { + return wordOperationPlugin.canChangeStackKind(b) || nodeIntrinsificationPlugin.canChangeStackKind(b); + } + return false; + } + + @Override public FloatingNode interceptParameter(GraphBuilderContext b, int index, Stamp stamp) { if (b.parsingIntrinsic()) { return wordOperationPlugin.interceptParameter(b, index, stamp); diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.java/src/com/oracle/graal/java/BytecodeParser.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/BytecodeParser.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/BytecodeParser.java Tue Jun 02 21:15:59 2015 -0700 @@ -365,6 +365,12 @@ this.firstInstructionArray = new FixedWithNextNode[blockMap.getBlockCount()]; this.entryStateArray = new FrameStateBuilder[blockMap.getBlockCount()]; + /* + * Configure the assertion checking behavior of the FrameStateBuilder. This needs to be + * done only when assertions are enabled, so it is wrapped in an assertion itself. + */ + assert computeKindVerification(startFrameState); + try (Scope s = Debug.scope("LivenessAnalysis")) { int maxLocals = method.getMaxLocals(); liveness = LocalLiveness.compute(stream, blockMap.getBlocks(), maxLocals, blockMap.getLoopCount()); @@ -443,6 +449,28 @@ } } + private boolean computeKindVerification(FrameStateBuilder startFrameState) { + if (blockMap.hasJsrBytecodes) { + /* + * The JSR return address is an int value, but stored using the astore bytecode. Instead + * of weakening the kind assertion checking for all methods, we disable it completely + * for methods that contain a JSR bytecode. + */ + startFrameState.disableKindVerification(); + } + + for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) { + if (plugin.canChangeStackKind(this)) { + /* + * We have a plugin that can change the kind of values, so no kind assertion + * checking is possible. + */ + startFrameState.disableKindVerification(); + } + } + return true; + } + /** * Hook for subclasses to modify the graph start instruction or append new instructions to it. * @@ -524,12 +552,23 @@ ResolvedJavaMethod original = intrinsicContext.getOriginalMethod(); ValueNode[] locals; if (original.getMaxLocals() == frameState.localsSize() || original.isNative()) { + locals = new ValueNode[original.getMaxLocals()]; + for (int i = 0; i < locals.length; i++) { + ValueNode node = frameState.locals[i]; + if (node == FrameState.TWO_SLOT_MARKER) { + node = null; + } + locals[i] = node; + } 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]; + if (param == FrameState.TWO_SLOT_MARKER) { + param = null; + } locals[i] = param; assert param == null || param instanceof ParameterNode || param.isConstant(); } @@ -1249,9 +1288,9 @@ try { currentInvokeReturnType = returnType; currentInvokeKind = invokeKind; - if (tryGenericInvocationPlugin(args, targetMethod)) { + if (tryNodePluginForInvocation(args, targetMethod)) { if (TraceParserPlugins.getValue()) { - traceWithContext("used generic invocation plugin for %s", targetMethod.format("%h.%n(%p)")); + traceWithContext("used node plugin for %s", targetMethod.format("%h.%n(%p)")); } return; } @@ -1370,7 +1409,7 @@ return false; } - private boolean tryGenericInvocationPlugin(ValueNode[] args, ResolvedJavaMethod targetMethod) { + private boolean tryNodePluginForInvocation(ValueNode[] args, ResolvedJavaMethod targetMethod) { for (NodePlugin plugin : graphBuilderConfig.getPlugins().getNodePlugins()) { if (plugin.handleInvoke(this, targetMethod, args)) { return true; diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.java/src/com/oracle/graal/java/FrameStateBuilder.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/FrameStateBuilder.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/FrameStateBuilder.java Tue Jun 02 21:15:59 2015 -0700 @@ -25,6 +25,7 @@ import static com.oracle.graal.bytecode.Bytecodes.*; import static com.oracle.graal.graph.iterators.NodePredicates.*; import static com.oracle.graal.java.BytecodeParser.Options.*; +import static com.oracle.graal.nodes.FrameState.*; import static com.oracle.jvmci.common.JVMCIError.*; import java.util.*; @@ -54,6 +55,7 @@ protected final ValueNode[] locals; protected final ValueNode[] stack; private ValueNode[] lockedObjects; + private boolean canVerifyKind; /** * @see BytecodeFrame#rethrowException @@ -87,6 +89,11 @@ this.monitorIds = EMPTY_MONITOR_ARRAY; this.graph = graph; + this.canVerifyKind = true; + } + + public void disableKindVerification() { + canVerifyKind = false; } public void initializeFromArgumentsArray(ValueNode[] arguments) { @@ -104,7 +111,11 @@ for (int i = 0; i < max; i++) { Kind kind = sig.getParameterKind(i); locals[javaIndex] = arguments[index]; - javaIndex += kind.getSlotCount(); + javaIndex++; + if (kind.needsTwoSlots()) { + locals[javaIndex] = TWO_SLOT_MARKER; + javaIndex++; + } index++; } } @@ -156,7 +167,11 @@ param = new ParameterNode(index, stamp); } locals[javaIndex] = graph.unique(param); - javaIndex += kind.getSlotCount(); + javaIndex++; + if (kind.needsTwoSlots()) { + locals[javaIndex] = TWO_SLOT_MARKER; + javaIndex++; + } index++; } } @@ -169,6 +184,7 @@ this.stack = other.stack.clone(); this.lockedObjects = other.lockedObjects.length == 0 ? other.lockedObjects : other.lockedObjects.clone(); this.rethrowException = other.rethrowException; + this.canVerifyKind = other.canVerifyKind; assert locals.length == method.getMaxLocals(); assert stack.length == Math.max(1, method.getMaxStackSize()); @@ -195,11 +211,11 @@ StringBuilder sb = new StringBuilder(); sb.append("[locals: ["); for (int i = 0; i < locals.length; i++) { - sb.append(i == 0 ? "" : ",").append(locals[i] == null ? "_" : locals[i].toString(Verbosity.Id)); + sb.append(i == 0 ? "" : ",").append(locals[i] == null ? "_" : locals[i] == TWO_SLOT_MARKER ? "#" : locals[i].toString(Verbosity.Id)); } sb.append("] stack: ["); for (int i = 0; i < stackSize; i++) { - sb.append(i == 0 ? "" : ",").append(stack[i] == null ? "_" : stack[i].toString(Verbosity.Id)); + sb.append(i == 0 ? "" : ",").append(stack[i] == null ? "_" : stack[i] == TWO_SLOT_MARKER ? "#" : stack[i].toString(Verbosity.Id)); } sb.append("] locks: ["); for (int i = 0; i < lockedObjects.length; i++) { @@ -293,7 +309,8 @@ for (int i = 0; i < stackSize(); i++) { ValueNode x = stack[i]; ValueNode y = other.stack[i]; - if (x != y && (x == null || x.isDeleted() || y == null || y.isDeleted() || x.getKind() != y.getKind())) { + assert x != null && y != null; + if (x != y && (x == TWO_SLOT_MARKER || x.isDeleted() || y == TWO_SLOT_MARKER || y.isDeleted() || x.getKind() != y.getKind())) { return false; } } @@ -358,7 +375,7 @@ if (currentValue == null || currentValue.isDeleted()) { return null; } else if (block.isPhiAtMerge(currentValue)) { - if (otherValue == null || otherValue.isDeleted() || currentValue.getKind() != otherValue.getKind()) { + if (otherValue == null || otherValue == TWO_SLOT_MARKER || otherValue.isDeleted() || currentValue.getKind() != otherValue.getKind()) { propagateDelete((ValuePhiNode) currentValue); return null; } @@ -366,7 +383,9 @@ return currentValue; } else if (currentValue != otherValue) { assert !(block instanceof LoopBeginNode) : String.format("Phi functions for loop headers are create eagerly for changed locals and all stack slots: %s != %s", currentValue, otherValue); - if (otherValue == null || otherValue.isDeleted() || currentValue.getKind() != otherValue.getKind()) { + if (currentValue == TWO_SLOT_MARKER || otherValue == TWO_SLOT_MARKER) { + return null; + } else if (otherValue == null || otherValue.isDeleted() || currentValue.getKind() != otherValue.getKind()) { return null; } return createValuePhi(currentValue, otherValue, block); @@ -422,14 +441,14 @@ public void insertLoopProxies(LoopExitNode loopExit, FrameStateBuilder loopEntryState) { for (int i = 0; i < localsSize(); i++) { ValueNode value = locals[i]; - if (value != null && (!loopEntryState.contains(value) || loopExit.loopBegin().isPhiAtMerge(value))) { + if (value != null && value != TWO_SLOT_MARKER && (!loopEntryState.contains(value) || loopExit.loopBegin().isPhiAtMerge(value))) { Debug.log(" inserting proxy for %s", value); locals[i] = ProxyNode.forValue(value, loopExit, graph); } } for (int i = 0; i < stackSize(); i++) { ValueNode value = stack[i]; - if (value != null && (!loopEntryState.contains(value) || loopExit.loopBegin().isPhiAtMerge(value))) { + if (value != null && value != TWO_SLOT_MARKER && (!loopEntryState.contains(value) || loopExit.loopBegin().isPhiAtMerge(value))) { Debug.log(" inserting proxy for %s", value); stack[i] = ProxyNode.forValue(value, loopExit, graph); } @@ -446,14 +465,14 @@ public void insertProxies(Function proxyFunction) { for (int i = 0; i < localsSize(); i++) { ValueNode value = locals[i]; - if (value != null) { + if (value != null && value != TWO_SLOT_MARKER) { Debug.log(" inserting proxy for %s", value); locals[i] = proxyFunction.apply(value); } } for (int i = 0; i < stackSize(); i++) { ValueNode value = stack[i]; - if (value != null) { + if (value != null && value != TWO_SLOT_MARKER) { Debug.log(" inserting proxy for %s", value); stack[i] = proxyFunction.apply(value); } @@ -467,9 +486,9 @@ } } - private ValuePhiNode createLoopPhi(AbstractMergeNode block, ValueNode value, boolean stampFromValue) { - if (value == null) { - return null; + private ValueNode createLoopPhi(AbstractMergeNode block, ValueNode value, boolean stampFromValue) { + if (value == null || value == TWO_SLOT_MARKER) { + return value; } assert !block.isPhiAtMerge(value) : "phi function for this block already created"; @@ -552,12 +571,14 @@ if (liveIn) { for (int i = 0; i < locals.length; i++) { if (!liveness.localIsLiveIn(block, i)) { + assert locals[i] != TWO_SLOT_MARKER || locals[i - 1] == null : "Clearing of second slot must have cleared the first slot too"; locals[i] = null; } } } else { for (int i = 0; i < locals.length; i++) { if (!liveness.localIsLiveOut(block, i)) { + assert locals[i] != TWO_SLOT_MARKER || locals[i - 1] == null : "Clearing of second slot must have cleared the first slot too"; locals[i] = null; } } @@ -603,6 +624,17 @@ return stackSize; } + private boolean verifyKind(Kind slotKind, ValueNode x) { + assert x != null; + assert x != TWO_SLOT_MARKER; + assert slotKind.getSlotCount() > 0; + + if (canVerifyKind) { + assert x.getKind().getStackKind() == slotKind.getStackKind(); + } + return true; + } + /** * Loads the local variable at the specified index, checking that the returned value is non-null * and that two-stack values are properly handled. @@ -612,11 +644,9 @@ * @return the instruction that produced the specified local */ public ValueNode loadLocal(int i, Kind slotKind) { - assert slotKind.getSlotCount() > 0; - assert slotKind.getSlotCount() == 1 || locals[i + 1] == null; - ValueNode x = locals[i]; - assert x != null : i; + assert verifyKind(slotKind, x); + assert slotKind.needsTwoSlots() ? locals[i + 1] == TWO_SLOT_MARKER : (i == locals.length - 1 || locals[i + 1] != TWO_SLOT_MARKER); return x; } @@ -629,10 +659,21 @@ * @param x the instruction which produces the value for the local */ public void storeLocal(int i, Kind slotKind, ValueNode x) { - assert slotKind.getSlotCount() > 0; + assert verifyKind(slotKind, x); + if (locals[i] == TWO_SLOT_MARKER) { + /* Writing the second slot of a two-slot value invalidates the first slot. */ + locals[i - 1] = null; + } locals[i] = x; if (slotKind.needsTwoSlots()) { + /* Writing a two-slot value: mark the second slot. */ + locals[i + 1] = TWO_SLOT_MARKER; + } else if (i < locals.length - 1 && locals[i + 1] == TWO_SLOT_MARKER) { + /* + * Writing a one-slot value to an index previously occupied by a two-slot value: clear + * the old marker of the second slot. + */ locals[i + 1] = null; } } @@ -644,11 +685,11 @@ * @param x the instruction to push onto the stack */ public void push(Kind slotKind, ValueNode x) { - assert x != null; - assert slotKind.getSlotCount() > 0; + assert verifyKind(slotKind, x); + xpush(x); if (slotKind.needsTwoSlots()) { - xpush(null); + xpush(TWO_SLOT_MARKER); } } @@ -665,26 +706,30 @@ * @return the instruction on the top of the stack */ public ValueNode pop(Kind slotKind) { - assert slotKind.getSlotCount() > 0; if (slotKind.needsTwoSlots()) { ValueNode s = xpop(); - assert s == null; + assert s == TWO_SLOT_MARKER; } ValueNode x = xpop(); - assert x != null; + assert verifyKind(slotKind, x); return x; } private void xpush(ValueNode x) { + assert x != null; stack[stackSize++] = x; } private ValueNode xpop() { - return stack[--stackSize]; + ValueNode result = stack[--stackSize]; + assert result != null; + return result; } private ValueNode xpeek() { - return stack[stackSize - 1]; + ValueNode result = stack[stackSize - 1]; + assert result != null; + return result; } /** @@ -695,16 +740,15 @@ */ public ValueNode[] popArguments(int argSize) { ValueNode[] result = allocateArray(argSize); - int newStackSize = stackSize; for (int i = argSize - 1; i >= 0; i--) { - newStackSize--; - if (stack[newStackSize] == null) { - /* Two-slot value. */ - newStackSize--; + ValueNode x = xpop(); + if (x == TWO_SLOT_MARKER) { + /* Ignore second slot of two-slot value. */ + x = xpop(); } - result[i] = stack[newStackSize]; + assert x != null && x != TWO_SLOT_MARKER; + result[i] = x; } - stackSize = newStackSize; return result; } @@ -723,21 +767,26 @@ public void stackOp(int opcode) { switch (opcode) { case POP: { - xpop(); + ValueNode w1 = xpop(); + assert w1 != TWO_SLOT_MARKER; break; } case POP2: { xpop(); - xpop(); + ValueNode w2 = xpop(); + assert w2 != TWO_SLOT_MARKER; break; } case DUP: { - xpush(xpeek()); + ValueNode w1 = xpeek(); + assert w1 != TWO_SLOT_MARKER; + xpush(w1); break; } case DUP_X1: { ValueNode w1 = xpop(); ValueNode w2 = xpop(); + assert w1 != TWO_SLOT_MARKER; xpush(w1); xpush(w2); xpush(w1); @@ -747,6 +796,7 @@ ValueNode w1 = xpop(); ValueNode w2 = xpop(); ValueNode w3 = xpop(); + assert w1 != TWO_SLOT_MARKER; xpush(w1); xpush(w3); xpush(w2); @@ -789,6 +839,8 @@ case SWAP: { ValueNode w1 = xpop(); ValueNode w2 = xpop(); + assert w1 != TWO_SLOT_MARKER; + assert w2 != TWO_SLOT_MARKER; xpush(w1); xpush(w2); break; @@ -876,11 +928,11 @@ Debug.log(String.format("| state [nr locals = %d, stack depth = %d, method = %s]", localsSize(), stackSize(), method)); for (int i = 0; i < localsSize(); ++i) { ValueNode value = locals[i]; - Debug.log(String.format("| local[%d] = %-8s : %s", i, value == null ? "bogus" : value.getKind().getJavaName(), value)); + Debug.log(String.format("| local[%d] = %-8s : %s", i, value == null ? "bogus" : value == TWO_SLOT_MARKER ? "second" : value.getKind().getJavaName(), value)); } for (int i = 0; i < stackSize(); ++i) { ValueNode value = stack[i]; - Debug.log(String.format("| stack[%d] = %-8s : %s", i, value == null ? "bogus" : value.getKind().getJavaName(), value)); + Debug.log(String.format("| stack[%d] = %-8s : %s", i, value == null ? "bogus" : value == TWO_SLOT_MARKER ? "second" : value.getKind().getJavaName(), value)); } } } diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java Tue Jun 02 21:15:59 2015 -0700 @@ -27,6 +27,7 @@ import java.util.*; import com.oracle.graal.bytecode.*; +import com.oracle.graal.compiler.common.type.*; import com.oracle.graal.graph.*; import com.oracle.graal.graph.iterators.*; import com.oracle.graal.nodeinfo.*; @@ -48,6 +49,22 @@ private static final DebugMetric METRIC_FRAMESTATE_COUNT = Debug.metric("FrameStateCount"); + /** + * Marker value for the second slot of values that occupy two local variable or expression stack + * slots. The marker value is used by the bytecode parser, but replaced with {@code null} in the + * {@link #values} of the {@link FrameState}. + */ + public static final ValueNode TWO_SLOT_MARKER = new TwoSlotMarker(); + + @NodeInfo + private static final class TwoSlotMarker extends ValueNode { + public static final NodeClass TYPE = NodeClass.create(TwoSlotMarker.class); + + protected TwoSlotMarker() { + super(TYPE, StampFactory.forKind(Kind.Illegal)); + } + } + protected final int localsSize; protected final int stackSize; @@ -139,13 +156,23 @@ private void createValues(ValueNode[] locals, ValueNode[] stack, ValueNode[] locks) { int index = 0; for (int i = 0; i < locals.length; ++i) { - this.values.initialize(index++, locals[i]); + ValueNode value = locals[i]; + if (value == TWO_SLOT_MARKER) { + value = null; + } + this.values.initialize(index++, value); } for (int i = 0; i < stackSize; ++i) { - this.values.initialize(index++, stack[i]); + ValueNode value = stack[i]; + if (value == TWO_SLOT_MARKER) { + value = null; + } + this.values.initialize(index++, value); } for (int i = 0; i < locks.length; ++i) { - this.values.initialize(index++, locks[i]); + ValueNode value = locks[i]; + assert value != TWO_SLOT_MARKER; + this.values.initialize(index++, value); } } diff -r afea1d08c393 -r 889b45a0dedd graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/WordOperationPlugin.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/WordOperationPlugin.java Tue Jun 02 18:32:11 2015 -0700 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/WordOperationPlugin.java Tue Jun 02 21:15:59 2015 -0700 @@ -59,6 +59,11 @@ this.wordKind = wordTypes.getWordKind(); } + @Override + public boolean canChangeStackKind(GraphBuilderContext b) { + return true; + } + /** * Processes a call to a method if it is annotated with {@link Operation} by adding nodes to the * graph being built that implement the denoted operation.