changeset 21502:553445b73d99

Bugfix for Graph Decoder: ensure that guard dependencies to block begins are correctly re-wired during decoding
author Christian Wimmer <christian.wimmer@oracle.com>
date Tue, 26 May 2015 16:19:16 -0700
parents ce585b0ac3e2
children 12e3d0dfffeb
files graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PEGraphDecoderTest.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/PEGraphDecoder.java
diffstat 3 files changed, 166 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java	Tue May 26 21:22:00 2015 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java	Tue May 26 16:19:16 2015 -0700
@@ -271,6 +271,13 @@
         LoopScope loopScope = new LoopScope(methodScope);
         FixedNode firstNode;
         if (startNode != null) {
+            /*
+             * The start node of a graph can be referenced as the guard for a GuardedNode. We
+             * register the previous block node, so that such guards are correctly anchored when
+             * doing inlining during graph decoding.
+             */
+            registerNode(loopScope, GraphEncoder.START_NODE_ORDER_ID, AbstractBeginNode.prevBegin(startNode), false, false);
+
             firstNode = makeStubNode(methodScope, loopScope, GraphEncoder.FIRST_NODE_ORDER_ID);
             startNode.setNext(firstNode);
             loopScope.nodesToProcess.set(GraphEncoder.FIRST_NODE_ORDER_ID);
@@ -332,6 +339,10 @@
                         ((AbstractMergeNode) node).forwardEndCount() == 1) {
             AbstractMergeNode merge = (AbstractMergeNode) node;
             EndNode singleEnd = merge.forwardEndAt(0);
+
+            /* Nodes that would use this merge as the guard need to use the previous block. */
+            registerNode(loopScope, nodeOrderId, AbstractBeginNode.prevBegin(singleEnd), true, false);
+
             FixedNode next = makeStubNode(methodScope, loopScope, nodeOrderId + GraphEncoder.BEGIN_NEXT_ORDER_ID_OFFSET);
             singleEnd.replaceAtPredecessor(next);
 
@@ -822,7 +833,13 @@
     protected Node decodeFloatingNode(MethodScope methodScope, LoopScope loopScope, int nodeOrderId) {
         long readerByteIndex = methodScope.reader.getByteIndex();
         Node node = instantiateNode(methodScope, nodeOrderId);
-        assert !(node instanceof FixedNode);
+        if (node instanceof FixedNode) {
+            /*
+             * This is a severe error that will lead to a corrupted graph, so it is better not to
+             * continue decoding at all.
+             */
+            throw shouldNotReachHere("Not a floating node: " + node.getClass().getName());
+        }
 
         /* Read the properties of the node. */
         readProperties(methodScope, node);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PEGraphDecoderTest.java	Tue May 26 16:19:16 2015 -0700
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2015, 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.graal.replacements.test;
+
+import org.junit.*;
+
+import com.oracle.graal.api.meta.*;
+import com.oracle.graal.compiler.common.type.*;
+import com.oracle.graal.compiler.test.*;
+import com.oracle.graal.debug.*;
+import com.oracle.graal.graphbuilderconf.*;
+import com.oracle.graal.graphbuilderconf.InvocationPlugins.Registration;
+import com.oracle.graal.nodes.*;
+import com.oracle.graal.nodes.StructuredGraph.AllowAssumptions;
+import com.oracle.graal.nodes.extended.*;
+import com.oracle.graal.nodes.memory.HeapAccess.BarrierType;
+import com.oracle.graal.nodes.memory.*;
+import com.oracle.graal.phases.*;
+import com.oracle.graal.phases.common.*;
+import com.oracle.graal.phases.tiers.*;
+import com.oracle.graal.replacements.*;
+
+public class PEGraphDecoderTest extends GraalCompilerTest {
+
+    /**
+     * This method is intrinsified to a node with a guard dependency on the block it is in. The
+     * various tests ensure that this guard is correctly updated when blocks are merged during
+     * inlining.
+     */
+    private static native int readInt(Object obj, long offset);
+
+    private static boolean flag;
+    private static int value;
+
+    private static void invokeSimple() {
+        value = 111;
+    }
+
+    private static void invokeComplicated() {
+        if (flag) {
+            value = 0;
+        } else {
+            value = 42;
+        }
+    }
+
+    private static int readInt1(Object obj) {
+        return readInt(obj, 16);
+    }
+
+    private static int readInt2(Object obj) {
+        invokeSimple();
+        return readInt(obj, 16);
+    }
+
+    private static int readInt3(Object obj) {
+        invokeComplicated();
+        return readInt(obj, 16);
+    }
+
+    private static int readInt4(Object obj, int n) {
+        if (n > 0) {
+            invokeComplicated();
+        }
+        return readInt(obj, 16);
+    }
+
+    public static int doTest(Object obj) {
+        int result = 0;
+        result += readInt1(obj);
+        result += readInt2(obj);
+        result += readInt3(obj);
+        result += readInt4(obj, 2);
+        return result;
+    }
+
+    private static void registerPlugins(InvocationPlugins plugins) {
+        Registration r = new Registration(plugins, PEGraphDecoderTest.class);
+        r.register2("readInt", Object.class, long.class, new InvocationPlugin() {
+            @Override
+            public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, MethodIdMap.Receiver unused, ValueNode obj, ValueNode offset) {
+                LocationNode location = b.add(new ConstantLocationNode(LocationIdentity.any(), offset.asJavaConstant().asLong()));
+                ReadNode read = b.addPush(Kind.Int, new ReadNode(obj, location, StampFactory.forKind(Kind.Int), BarrierType.NONE));
+                read.setGuard(AbstractBeginNode.prevBegin(read));
+                return true;
+            }
+        });
+    }
+
+    class InlineAll implements InlineInvokePlugin {
+        @Override
+        public InlineInfo getInlineInfo(GraphBuilderContext b, ResolvedJavaMethod method, ValueNode[] args, JavaType returnType) {
+            return new InlineInfo(method, false);
+        }
+    }
+
+    @Test
+    public void test() {
+        ResolvedJavaMethod testMethod = getResolvedJavaMethod(PEGraphDecoderTest.class, "doTest", Object.class);
+        StructuredGraph targetGraph = null;
+        try (Debug.Scope scope = Debug.scope("GraphPETest", testMethod)) {
+            GraphBuilderConfiguration graphBuilderConfig = GraphBuilderConfiguration.getEagerDefault(getDefaultGraphBuilderPlugins());
+            registerPlugins(graphBuilderConfig.getPlugins().getInvocationPlugins());
+            CachingPEGraphDecoder decoder = new CachingPEGraphDecoder(getProviders(), graphBuilderConfig, OptimisticOptimizations.NONE, AllowAssumptions.YES, getTarget().arch);
+
+            targetGraph = new StructuredGraph(testMethod, AllowAssumptions.YES);
+            decoder.decode(targetGraph, testMethod, null, null, new InlineAll(), null);
+            Debug.dump(targetGraph, "Target Graph");
+            targetGraph.verify();
+
+            PhaseContext context = new PhaseContext(getProviders());
+            new CanonicalizerPhase().apply(targetGraph, context);
+            targetGraph.verify();
+
+        } catch (Throwable ex) {
+            if (targetGraph != null) {
+                Debug.dump(targetGraph, ex.toString());
+            }
+            Debug.handle(ex);
+        }
+    }
+}
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/PEGraphDecoder.java	Tue May 26 21:22:00 2015 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/PEGraphDecoder.java	Tue May 26 16:19:16 2015 -0700
@@ -390,7 +390,7 @@
             if (graphBuilderContext.lastInstr != null) {
                 registerNode(loopScope, invokeData.invokeOrderId, graphBuilderContext.pushedNode, true, true);
                 invoke.asNode().replaceAtUsages(graphBuilderContext.pushedNode);
-                graphBuilderContext.lastInstr.setNext(nodeAfterInvoke(methodScope, loopScope, invokeData));
+                graphBuilderContext.lastInstr.setNext(nodeAfterInvoke(methodScope, loopScope, invokeData, AbstractBeginNode.prevBegin(graphBuilderContext.lastInstr)));
             } else {
                 assert graphBuilderContext.pushedNode == null : "Why push a node when the invoke does not return anyway?";
                 invoke.asNode().replaceAtUsages(null);
@@ -469,16 +469,16 @@
         ValueNode returnValue;
         List<ReturnNode> returnNodes = inlineScope.returnNodes;
         if (!returnNodes.isEmpty()) {
-            FixedNode n;
-            n = nodeAfterInvoke(methodScope, loopScope, invokeData);
             if (returnNodes.size() == 1) {
                 ReturnNode returnNode = returnNodes.get(0);
                 returnValue = returnNode.result();
+                FixedNode n = nodeAfterInvoke(methodScope, loopScope, invokeData, AbstractBeginNode.prevBegin(returnNode));
                 returnNode.replaceAndDelete(n);
             } else {
                 AbstractMergeNode merge = methodScope.graph.add(new MergeNode());
                 merge.setStateAfter((FrameState) ensureNodeCreated(methodScope, loopScope, invokeData.stateAfterOrderId));
                 returnValue = InliningUtil.mergeReturns(merge, returnNodes, null);
+                FixedNode n = nodeAfterInvoke(methodScope, loopScope, invokeData, merge);
                 merge.setNext(n);
             }
         } else {
@@ -508,9 +508,11 @@
         return true;
     }
 
-    public FixedNode nodeAfterInvoke(PEMethodScope methodScope, LoopScope loopScope, InvokeData invokeData) {
+    public FixedNode nodeAfterInvoke(PEMethodScope methodScope, LoopScope loopScope, InvokeData invokeData, AbstractBeginNode lastBlock) {
+        assert lastBlock.isAlive();
         FixedNode n;
         if (invokeData.invoke instanceof InvokeWithExceptionNode) {
+            registerNode(loopScope, invokeData.nextOrderId, lastBlock, false, false);
             n = makeStubNode(methodScope, loopScope, invokeData.nextNextOrderId);
         } else {
             n = makeStubNode(methodScope, loopScope, invokeData.nextOrderId);