# HG changeset patch # User Lukas Stadler # Date 1340035116 -7200 # Node ID f53a347eae936438e06fd76358fa9aa7137249a3 # Parent 731789427441496342f3d67098b6034aab724030 add inliningIdentifier to FrameState (fixes problem with duplicated FrameStates and locking) diff -r 731789427441 -r f53a347eae93 graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/DebugInfoBuilder.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/DebugInfoBuilder.java Mon Jun 18 10:07:33 2012 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/DebugInfoBuilder.java Mon Jun 18 17:58:36 2012 +0200 @@ -82,7 +82,7 @@ VirtualObjectState currentField = objectStates.get(vobj); assert currentField != null; for (int i = 0; i < vobj.fieldsCount(); i++) { - values[i] = toCiValue(currentField.fields().get(i)); + values[i] = toCiValue(currentField.fieldValues().get(i)); } } } @@ -100,7 +100,7 @@ private BytecodeFrame computeFrameForState(FrameState state, LockScope locks, long leafGraphId) { int numLocals = state.localsSize(); int numStack = state.stackSize(); - int numLocks = (locks != null && locks.callerState == state.outerFrameState()) ? locks.stateDepth + 1 : 0; + int numLocks = (locks != null && locks.inliningIdentifier == state.inliningIdentifier()) ? locks.stateDepth + 1 : 0; Value[] values = new Value[numLocals + numStack + numLocks]; for (int i = 0; i < numLocals; i++) { @@ -112,7 +112,7 @@ LockScope nextLock = locks; for (int i = numLocks - 1; i >= 0; i--) { - assert locks != null && nextLock.callerState == state.outerFrameState() && nextLock.stateDepth == i; + assert locks != null && nextLock.inliningIdentifier == state.inliningIdentifier() && nextLock.stateDepth == i; Value owner = toCiValue(nextLock.monitor.object()); Value lockData = nextLock.lockData; @@ -153,7 +153,7 @@ } else if (value != null) { Debug.metric("StateVariables").increment(); Value operand = nodeOperands.get(value); - assert operand != null && (operand instanceof Variable || operand instanceof Constant); + assert operand != null && (operand instanceof Variable || operand instanceof Constant) : operand; return operand; } else { diff -r 731789427441 -r f53a347eae93 graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/LIRGenerator.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/LIRGenerator.java Mon Jun 18 10:07:33 2012 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/gen/LIRGenerator.java Mon Jun 18 17:58:36 2012 +0200 @@ -45,6 +45,7 @@ import com.oracle.graal.lir.StandardOp.PhiLabelOp; import com.oracle.graal.lir.cfg.*; import com.oracle.graal.nodes.*; +import com.oracle.graal.nodes.FrameState.InliningIdentifier; import com.oracle.graal.nodes.PhiNode.PhiType; import com.oracle.graal.nodes.calc.*; import com.oracle.graal.nodes.extended.*; @@ -94,12 +95,10 @@ public final LockScope outer; /** - * The frame state of the caller of the method performing the lock, or null if the outermost method + * The identifier of the actual inlined method instance performing the lock, or null if the outermost method * performs the lock. This information is used to compute the {@link BytecodeFrame} that this lock belongs to. - * We cannot use the actual frame state of the locking method, because it is not unique for a method. The - * caller frame states are unique, i.e., all frame states of inlined methods refer to the same caller frame state. */ - public final FrameState callerState; + public final InliningIdentifier inliningIdentifier; /** * The number of locks already found for this frame state. @@ -116,12 +115,12 @@ */ public final StackSlot lockData; - public LockScope(LockScope outer, FrameState callerState, MonitorEnterNode monitor, StackSlot lockData) { + public LockScope(LockScope outer, InliningIdentifier inliningIdentifier, MonitorEnterNode monitor, StackSlot lockData) { this.outer = outer; - this.callerState = callerState; + this.inliningIdentifier = inliningIdentifier; this.monitor = monitor; this.lockData = lockData; - if (outer != null && outer.callerState == callerState) { + if (outer != null && outer.inliningIdentifier == inliningIdentifier) { this.stateDepth = outer.stateDepth + 1; } else { this.stateDepth = 0; @@ -539,7 +538,7 @@ if (x.eliminated()) { // No code is emitted for eliminated locks, but for proper debug information generation we need to // register the monitor and its lock data. - curLocks = new LockScope(curLocks, x.stateAfter().outerFrameState(), x, lockData); + curLocks = new LockScope(curLocks, x.stateAfter().inliningIdentifier(), x, lockData); return; } @@ -548,7 +547,7 @@ LIRDebugInfo stateBefore = state(); // The state before the monitor enter is used for null checks, so it must not contain the newly locked object. - curLocks = new LockScope(curLocks, x.stateAfter().outerFrameState(), x, lockData); + curLocks = new LockScope(curLocks, x.stateAfter().inliningIdentifier(), x, lockData); // The state after the monitor enter is used for deoptimization, after the monitor has blocked, so it must contain the newly locked object. LIRDebugInfo stateAfter = stateFor(x.stateAfter(), -1); diff -r 731789427441 -r f53a347eae93 graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java Mon Jun 18 10:07:33 2012 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java Mon Jun 18 17:58:36 2012 +0200 @@ -35,6 +35,7 @@ import com.oracle.graal.debug.*; import com.oracle.graal.graph.*; import com.oracle.graal.nodes.*; +import com.oracle.graal.nodes.FrameState.*; import com.oracle.graal.nodes.calc.*; import com.oracle.graal.nodes.extended.*; import com.oracle.graal.nodes.java.*; @@ -771,6 +772,7 @@ * @param receiverNullCheck true if a null check needs to be generated for non-static inlinings, false if no such check is required */ public static void inline(Invoke invoke, StructuredGraph inlineGraph, boolean receiverNullCheck) { + InliningIdentifier identifier = new InliningIdentifier(inlineGraph.method(), invoke.toString()); NodeInputList parameters = invoke.callTarget().arguments(); StructuredGraph graph = (StructuredGraph) invoke.node().graph(); @@ -874,6 +876,7 @@ outerFrameState.setDuringCall(true); } frameState.setOuterFrameState(outerFrameState); + frameState.setInliningIdentifier(identifier); } } } diff -r 731789427441 -r f53a347eae93 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 Mon Jun 18 10:07:33 2012 +0200 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/FrameStateBuilder.java Mon Jun 18 17:58:36 2012 +0200 @@ -116,7 +116,7 @@ } public FrameState create(int bci) { - return graph.add(new FrameState(method, bci, locals, stack, stackSize, rethrowException, false)); + return graph.add(new FrameState(method, bci, locals, stack, stackSize, rethrowException, false, null)); } public FrameStateBuilder copy() { diff -r 731789427441 -r f53a347eae93 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 Mon Jun 18 10:07:33 2012 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/FrameState.java Mon Jun 18 17:58:36 2012 +0200 @@ -37,6 +37,25 @@ */ public final class FrameState extends VirtualState implements Node.IterableNodeType, LIRLowerable { + /** + * An instance of this class is an identifier for all nodes that were generated by one specific inlining operation. + * It is used to generate the correct debug information for nested locks. + */ + public static final class InliningIdentifier { + private final ResolvedJavaMethod method; + private final String context; + + public InliningIdentifier(ResolvedJavaMethod method, String context) { + this.method = method; + this.context = context; + } + + @Override + public String toString() { + return method + "@" + context; + } + } + protected final int localsSize; protected final int stackSize; @@ -46,6 +65,12 @@ private boolean duringCall; /** + * This object identifies the concrete inlining operation that produced this frame state. + * It is set during inlining, therefore for the outermost frame states of a graph this field is null. + */ + private InliningIdentifier inliningIdentifier; + + /** * This BCI should be used for frame states that are built for code with no meaningful BCI. */ public static final int UNKNOWN_BCI = -4; @@ -92,7 +117,7 @@ * @param stackSize size of the stack * @param rethrowException if true the VM should re-throw the exception on top of the stack when deopt'ing using this framestate */ - public FrameState(ResolvedJavaMethod method, int bci, List values, int stackSize, boolean rethrowException, boolean duringCall, List virtualObjectMappings) { + public FrameState(ResolvedJavaMethod method, int bci, List values, int stackSize, boolean rethrowException, boolean duringCall, InliningIdentifier inliningIdentifier, List virtualObjectMappings) { assert stackSize >= 0; assert (bci >= 0 && method != null) || (bci < 0 && method == null && values.isEmpty()); this.method = method; @@ -103,6 +128,7 @@ this.virtualObjectMappings = new NodeInputList<>(this, virtualObjectMappings); this.rethrowException = rethrowException; this.duringCall = duringCall; + this.inliningIdentifier = inliningIdentifier; assert !rethrowException || stackSize == 1 : "must have exception on top of the stack"; } @@ -111,10 +137,10 @@ * @param bci marker bci, needs to be < 0 */ public FrameState(int bci) { - this(null, bci, Collections.emptyList(), 0, false, false, Collections.emptyList()); + this(null, bci, Collections.emptyList(), 0, false, false, null, Collections.emptyList()); } - public FrameState(ResolvedJavaMethod method, int bci, ValueNode[] locals, ValueNode[] stack, int stackSize, boolean rethrowException, boolean duringCall) { + public FrameState(ResolvedJavaMethod method, int bci, ValueNode[] locals, ValueNode[] stack, int stackSize, boolean rethrowException, boolean duringCall, InliningIdentifier inliningIdentifier) { this.method = method; this.bci = bci; this.localsSize = locals.length; @@ -132,6 +158,7 @@ this.virtualObjectMappings = new NodeInputList<>(this); this.rethrowException = rethrowException; this.duringCall = duringCall; + this.inliningIdentifier = inliningIdentifier; assert !rethrowException || stackSize == 1 : "must have exception on top of the stack"; } @@ -148,6 +175,14 @@ this.outerFrameState = x; } + public InliningIdentifier inliningIdentifier() { + return inliningIdentifier; + } + + public void setInliningIdentifier(InliningIdentifier inliningIdentifier) { + this.inliningIdentifier = inliningIdentifier; + } + public boolean rethrowException() { return rethrowException; } @@ -176,7 +211,7 @@ return virtualObjectMappings.get(i); } - public Iterable virtualObjectMappings() { + public NodeInputList virtualObjectMappings() { return virtualObjectMappings; } @@ -184,7 +219,9 @@ * Gets a copy of this frame state. */ public FrameState duplicate(int newBci) { - return duplicate(newBci, false); + FrameState other = graph().add(new FrameState(method, newBci, values, stackSize, rethrowException, duringCall, inliningIdentifier, virtualObjectMappings)); + other.setOuterFrameState(outerFrameState()); + return other; } /** @@ -194,12 +231,21 @@ return duplicate(bci); } - public FrameState duplicate(int newBci, boolean duplicateOuter) { - FrameState other = graph().add(new FrameState(method, newBci, values, stackSize, rethrowException, duringCall, virtualObjectMappings)); + /** + * Duplicates a FrameState, along with a deep copy of all connected VirtualState (outer FrameStates, + * VirtualObjectStates, ...). + */ + @Override + public FrameState duplicateWithVirtualState() { FrameState newOuterFrameState = outerFrameState(); - if (duplicateOuter && newOuterFrameState != null) { - newOuterFrameState = newOuterFrameState.duplicate(newOuterFrameState.bci, duplicateOuter); + if (newOuterFrameState != null) { + newOuterFrameState = newOuterFrameState.duplicateWithVirtualState(); } + ArrayList newVirtualMappings = new ArrayList<>(virtualObjectMappings.size()); + for (VirtualObjectState state : virtualObjectMappings) { + newVirtualMappings.add(state.duplicateWithVirtualState()); + } + FrameState other = graph().add(new FrameState(method, bci, values, stackSize, rethrowException, duringCall, inliningIdentifier, newVirtualMappings)); other.setOuterFrameState(newOuterFrameState); return other; } @@ -221,7 +267,7 @@ } Collections.addAll(copy, pushedValues); - FrameState other = graph().add(new FrameState(method, newBci, copy, copy.size() - localsSize, newRethrowException, false, virtualObjectMappings)); + FrameState other = graph().add(new FrameState(method, newBci, copy, copy.size() - localsSize, newRethrowException, false, inliningIdentifier, virtualObjectMappings)); other.setOuterFrameState(outerFrameState()); return other; } @@ -342,4 +388,14 @@ } return super.verify(); } + + @Override + public void applyToNonVirtual(NodeClosure< ? super ValueNode> closure) { + for (ValueNode value : values.nonNull()) { + closure.apply(value); + } + for (VirtualObjectState state : virtualObjectMappings) { + state.applyToNonVirtual(closure); + } + } }