changeset 19496:9525e4d5b385

disable (asserting) type checks in the FrameStateBuilder when parsing a replacement added GraphBuilderContext.getCurrentBlockGuard()
author Doug Simon <doug.simon@oracle.com>
date Thu, 19 Feb 2015 11:16:19 +0100
parents c4173ea6c8c7
children 1fd4b4c20924
files graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractFrameStateBuilder.java graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderContext.java graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java graal/com.oracle.graal.java/src/com/oracle/graal/java/HIRFrameStateBuilder.java
diffstat 5 files changed, 50 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- 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_<n> 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));
             }
--- 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;
     }
 
--- 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();
 }
--- 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();
+            }
         }
     }
 
--- 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<FrameState> outerFrameStateSupplier) {
-        super(method);
+    public HIRFrameStateBuilder(ResolvedJavaMethod method, StructuredGraph graph, boolean checkTypes, Supplier<FrameState> outerFrameStateSupplier) {
+        super(method, checkTypes);
 
         assert graph != null;