# HG changeset patch # User Doug Simon # Date 1424340979 -3600 # Node ID 9525e4d5b38565044c62e503120d389ae474330c # Parent c4173ea6c8c7f4a0e56be6ad878d73fb8f8aa92f disable (asserting) type checks in the FrameStateBuilder when parsing a replacement added GraphBuilderContext.getCurrentBlockGuard() diff -r c4173ea6c8c7 -r 9525e4d5b385 graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java Thu Feb 19 11:02:48 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java Thu Feb 19 11:16:19 2015 +0100 @@ -37,7 +37,6 @@ import com.oracle.graal.compiler.common.calc.*; import com.oracle.graal.debug.*; import com.oracle.graal.java.BciBlockMapping.BciBlock; -import com.oracle.graal.java.GraphBuilderPlugin.InvocationPlugin; import com.oracle.graal.java.GraphBuilderPlugin.LoadFieldPlugin; import com.oracle.graal.nodes.*; import com.oracle.graal.options.*; @@ -119,7 +118,7 @@ if (kind == Kind.Object) { value = frameState.xpop(); // astore and astore_ may be used to store a returnAddress (jsr) - assert value.getKind() == Kind.Object || value.getKind() == Kind.Int; + assert parsingReplacement || (value.getKind() == Kind.Object || value.getKind() == Kind.Int) : value + ":" + value.getKind(); } else { value = frameState.pop(kind); } @@ -573,7 +572,7 @@ } private JavaTypeProfile getProfileForTypeCheck(ResolvedJavaType type) { - if (profilingInfo == null || !optimisticOpts.useTypeCheckHints() || !canHaveSubtype(type)) { + if (parsingReplacement || profilingInfo == null || !optimisticOpts.useTypeCheckHints() || !canHaveSubtype(type)) { return null; } else { return profilingInfo.getTypeProfile(bci()); @@ -757,7 +756,7 @@ private void genGetStatic(JavaField field) { Kind kind = field.getKind(); if (field instanceof ResolvedJavaField && ((ResolvedJavaType) field.getDeclaringClass()).isInitialized()) { - InvocationPlugin.LoadFieldPlugin loadFieldPlugin = this.graphBuilderConfig.getLoadFieldPlugin(); + LoadFieldPlugin loadFieldPlugin = this.graphBuilderConfig.getLoadFieldPlugin(); if (loadFieldPlugin == null || !loadFieldPlugin.apply((GraphBuilderContext) this, (ResolvedJavaField) field)) { appendOptimizedLoadField(kind, genLoadField(null, (ResolvedJavaField) field)); } diff -r c4173ea6c8c7 -r 9525e4d5b385 graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractFrameStateBuilder.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractFrameStateBuilder.java Thu Feb 19 11:02:48 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractFrameStateBuilder.java Thu Feb 19 11:16:19 2015 +0100 @@ -36,15 +36,21 @@ protected T[] lockedObjects; /** + * Specifies if asserting type checks are enabled. + */ + protected final boolean checkTypes; + + /** * @see BytecodeFrame#rethrowException */ protected boolean rethrowException; - public AbstractFrameStateBuilder(ResolvedJavaMethod method) { + public AbstractFrameStateBuilder(ResolvedJavaMethod method, boolean checkTypes) { this.method = method; this.locals = allocateArray(method.getMaxLocals()); this.stack = allocateArray(Math.max(1, method.getMaxStackSize())); this.lockedObjects = allocateArray(0); + this.checkTypes = checkTypes; } protected AbstractFrameStateBuilder(S other) { @@ -54,6 +60,7 @@ this.stack = other.stack.clone(); this.lockedObjects = other.lockedObjects.length == 0 ? other.lockedObjects : other.lockedObjects.clone(); this.rethrowException = other.rethrowException; + this.checkTypes = other.checkTypes; assert locals.length == method.getMaxLocals(); assert stack.length == Math.max(1, method.getMaxStackSize()); @@ -171,8 +178,8 @@ public T loadLocal(int i) { T x = locals[i]; assert x != null : i; - assert x.getKind().getSlotCount() == 1 || locals[i + 1] == null; - assert i == 0 || locals[i - 1] == null || locals[i - 1].getKind().getSlotCount() == 1; + assert !checkTypes || (x.getKind().getSlotCount() == 1 || locals[i + 1] == null); + assert !checkTypes || (i == 0 || locals[i - 1] == null || locals[i - 1].getKind().getSlotCount() == 1); return x; } @@ -184,7 +191,7 @@ * @param x the instruction which produces the value for the local */ public void storeLocal(int i, T x) { - assert x == null || x.getKind() != Kind.Void && x.getKind() != Kind.Illegal : "unexpected value: " + x; + assert x == null || !checkTypes || (x.getKind() != Kind.Void && x.getKind() != Kind.Illegal) : "unexpected value: " + x; locals[i] = x; if (x != null && x.getKind().needsTwoSlots()) { // if this is a double word, then kill i+1 @@ -211,7 +218,7 @@ * @param x the instruction to push onto the stack */ public void push(Kind kind, T x) { - assert x.getKind() != Kind.Void && x.getKind() != Kind.Illegal; + assert !checkTypes || (x.getKind() != Kind.Void && x.getKind() != Kind.Illegal) : x; xpush(assertKind(kind, x)); if (kind.needsTwoSlots()) { xpush(null); @@ -224,7 +231,7 @@ * @param x the instruction to push onto the stack */ public void xpush(T x) { - assert x == null || (x.getKind() != Kind.Void && x.getKind() != Kind.Illegal); + assert !checkTypes || (x == null || (x.getKind() != Kind.Void && x.getKind() != Kind.Illegal)); stack[stackSize++] = x; } @@ -368,7 +375,7 @@ newStackSize--; assert stack[newStackSize].getKind().needsTwoSlots(); } else { - assert stack[newStackSize].getKind().getSlotCount() == 1; + assert !checkTypes || (stack[newStackSize].getKind().getSlotCount() == 1); } result[i] = stack[newStackSize]; } @@ -404,7 +411,7 @@ } private T assertKind(Kind kind, T x) { - assert x != null && x.getKind() == kind : "kind=" + kind + ", value=" + x + ((x == null) ? "" : ", value.kind=" + x.getKind()); + assert x != null && (!checkTypes || x.getKind() == kind) : "kind=" + kind + ", value=" + x + ((x == null) ? "" : ", value.kind=" + x.getKind()); return x; } @@ -424,7 +431,7 @@ } private T assertObject(T x) { - assert x != null && (x.getKind() == Kind.Object); + assert x != null && (!checkTypes || (x.getKind() == Kind.Object)) : x + ":" + x.getKind(); return x; } diff -r c4173ea6c8c7 -r 9525e4d5b385 graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderContext.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderContext.java Thu Feb 19 11:02:48 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderContext.java Thu Feb 19 11:16:19 2015 +0100 @@ -27,6 +27,7 @@ import com.oracle.graal.api.replacements.*; import com.oracle.graal.nodes.*; import com.oracle.graal.nodes.calc.*; +import com.oracle.graal.nodes.extended.*; import com.oracle.graal.nodes.spi.*; /** @@ -56,6 +57,8 @@ void push(Kind kind, ValueNode value); + StructuredGraph getGraph(); + /** * Determines if the graph builder is parsing a snippet or method substitution. */ @@ -71,4 +74,6 @@ } return nonNullValue; } + + GuardingNode getCurrentBlockGuard(); } diff -r c4173ea6c8c7 -r 9525e4d5b385 graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Thu Feb 19 11:02:48 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Thu Feb 19 11:16:19 2015 +0100 @@ -26,6 +26,7 @@ import static com.oracle.graal.api.meta.DeoptimizationReason.*; import static com.oracle.graal.bytecode.Bytecodes.*; import static com.oracle.graal.compiler.common.GraalOptions.*; +import static com.oracle.graal.graph.iterators.NodePredicates.*; import static com.oracle.graal.nodes.StructuredGraph.*; import static java.lang.String.*; @@ -126,7 +127,7 @@ int entryBCI = graph.getEntryBCI(); assert method.getCode() != null : "method must contain bytecodes: " + method; this.currentGraph = graph; - HIRFrameStateBuilder frameState = new HIRFrameStateBuilder(method, graph, null); + HIRFrameStateBuilder frameState = new HIRFrameStateBuilder(method, graph, true, null); frameState.initializeForMethodStart(graphBuilderConfig.eagerResolving(), this.graphBuilderConfig.getParameterPlugin()); TTY.Filter filter = new TTY.Filter(PrintFilter.getValue(), method); try { @@ -889,7 +890,8 @@ Mark mark = needsNullCheck ? currentGraph.getMark() : null; if (InvocationPlugin.execute(this, plugin, args)) { assert beforeStackSize + resultType.getSlotCount() == frameState.stackSize : "plugin manipulated the stack incorrectly " + targetMethod; - assert !needsNullCheck || containsNullCheckOf(currentGraph.getNewNodes(mark), args[0]) : "plugin needs to null check the receiver of " + targetMethod + ": " + args[0]; + assert !needsNullCheck || args[0].usages().filter(isNotA(FrameState.class)).isEmpty() || containsNullCheckOf(currentGraph.getNewNodes(mark), args[0]) : "plugin needs to null check the receiver of " + + targetMethod + ": " + args[0]; return true; } assert nodeCount == currentGraph.getNodeCount() : "plugin that returns false must not create new nodes"; @@ -938,7 +940,13 @@ private void parseAndInlineCallee(ResolvedJavaMethod targetMethod, ValueNode[] args, boolean isReplacement) { BytecodeParser parser = new BytecodeParser(metaAccess, targetMethod, graphBuilderConfig, optimisticOpts, INVOCATION_ENTRY_BCI, isReplacement); final FrameState[] lazyFrameState = new FrameState[1]; - HIRFrameStateBuilder startFrameState = new HIRFrameStateBuilder(targetMethod, currentGraph, () -> { + + // Replacements often produce nodes with an illegal kind (e.g., pointer stamps) + // so the frame state builder should not check the types flowing through the frame + // since all such assertions are in terms of Java kinds. + boolean checkTypes = !isReplacement; + + HIRFrameStateBuilder startFrameState = new HIRFrameStateBuilder(targetMethod, currentGraph, checkTypes, () -> { if (lazyFrameState[0] == null) { lazyFrameState[0] = frameState.create(bci()); } @@ -1924,6 +1932,19 @@ public boolean parsingReplacement() { return parsingReplacement; } + + public StructuredGraph getGraph() { + return currentGraph; + } + + public GuardingNode getCurrentBlockGuard() { + return (GuardingNode) getFirstInstruction(currentBlock, getCurrentDimension()); + } + + @Override + public String toString() { + return method.format("%H.%n(%p)@") + bci(); + } } } diff -r c4173ea6c8c7 -r 9525e4d5b385 graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java Thu Feb 19 11:02:48 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java Thu Feb 19 11:16:19 2015 +0100 @@ -53,8 +53,8 @@ * @param method the method whose frame is simulated * @param graph the target graph of Graal nodes created by the builder */ - public HIRFrameStateBuilder(ResolvedJavaMethod method, StructuredGraph graph, Supplier outerFrameStateSupplier) { - super(method); + public HIRFrameStateBuilder(ResolvedJavaMethod method, StructuredGraph graph, boolean checkTypes, Supplier outerFrameStateSupplier) { + super(method, checkTypes); assert graph != null;