# HG changeset patch # User Christian Wimmer # Date 1432682356 25200 # Node ID 553445b73d99bbe739e05307999d2b163fb21714 # Parent ce585b0ac3e2eef29a0c4423ab9a5c524a331a30 Bugfix for Graph Decoder: ensure that guard dependencies to block begins are correctly re-wired during decoding diff -r ce585b0ac3e2 -r 553445b73d99 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/GraphDecoder.java --- 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); diff -r ce585b0ac3e2 -r 553445b73d99 graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PEGraphDecoderTest.java --- /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); + } + } +} diff -r ce585b0ac3e2 -r 553445b73d99 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/PEGraphDecoder.java --- 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 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);