# HG changeset patch # User Christos Kotselidis # Date 1364477291 -3600 # Node ID 04b002b7077fd4a4925dcbbaf6fd9ed39a645d48 # Parent 59dab34ba44a27644a08fb00cd9f350af446792f# Parent da674936800c9ed467a2f2b298d0a63f184c6fa2 -Merge diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/DegeneratedLoopsTest.java --- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/DegeneratedLoopsTest.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/DegeneratedLoopsTest.java Thu Mar 28 14:28:11 2013 +0100 @@ -27,6 +27,7 @@ import com.oracle.graal.api.code.*; import com.oracle.graal.debug.*; import com.oracle.graal.nodes.*; +import com.oracle.graal.phases.*; import com.oracle.graal.phases.common.*; /** @@ -80,13 +81,11 @@ public void run() { StructuredGraph graph = parse(snippet); + new InliningPhase(runtime(), null, new Assumptions(false), null, getDefaultPhasePlan(), OptimisticOptimizations.ALL).apply(graph); + new CanonicalizerPhase(runtime(), new Assumptions(false)).apply(graph); Debug.dump(graph, "Graph"); - for (Invoke invoke : graph.getInvokes()) { - invoke.intrinsify(null); - } - new CanonicalizerPhase(runtime(), new Assumptions(false)).apply(graph); StructuredGraph referenceGraph = parse(REFERENCE_SNIPPET); - Debug.dump(referenceGraph, "Graph"); + Debug.dump(referenceGraph, "ReferenceGraph"); assertEquals(referenceGraph, graph); } }); diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraphScheduleTest.java --- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraphScheduleTest.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/GraphScheduleTest.java Thu Mar 28 14:28:11 2013 +0100 @@ -47,11 +47,11 @@ Block block = bBlock; while (block != null) { if (block == aBlock) { - break; + return; } block = block.getDominator(); } - Assert.assertSame(block, aBlock); + Assert.fail("block of A doesn't dominate the block of B"); } } } diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ReadAfterCheckCast.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ReadAfterCheckCast.java Thu Mar 28 14:28:11 2013 +0100 @@ -0,0 +1,130 @@ +/* + * Copyright (c) 2013, 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.compiler.test; + +import java.util.*; + +import org.junit.*; + +import com.oracle.graal.api.code.*; +import com.oracle.graal.debug.*; +import com.oracle.graal.graph.*; +import com.oracle.graal.nodes.*; +import com.oracle.graal.nodes.calc.*; +import com.oracle.graal.nodes.extended.*; +import com.oracle.graal.phases.common.*; + +/* consider + * B b = (B) a; + * return b.x10; + * + * With snippets a typecheck is performed and if it was successful, a UnsafeCastNode is created. + * For the read node, however, there is only a dependency to the UnsafeCastNode, but not to the + * typecheck itself. With special crafting, it's possible to get the scheduler moving the + * FloatingReadNode before the typecheck. Assuming the object is of the wrong type (here for + * example A), an invalid field read is done. + * + * In order to avoid this situation, an anchor node is introduced in CheckCastSnippts. + */ + +public class ReadAfterCheckCast extends GraphScheduleTest { + + public static long foo = 0; + + public static class A { + + public long x1; + } + + public static class B extends A { + + public long x10; + } + + public static long test1Snippet(A a) { + if (foo > 4) { + B b = (B) a; + b.x10 += 1; + return b.x10; + } else { + B b = (B) a; + b.x10 += 1; + return b.x10; + } + } + + @Test + public void test1() { + test("test1Snippet"); + } + + private void test(final String snippet) { + Debug.scope("FloatingReadTest", new DebugDumpScope(snippet), new Runnable() { + + // check shape of graph, with lots of assumptions. will probably fail if graph + // structure changes significantly + public void run() { + StructuredGraph graph = parse(snippet); + new LoweringPhase(null, runtime(), new Assumptions(false)).apply(graph); + new FloatingReadPhase().apply(graph); + new EliminatePartiallyRedundantGuardsPhase(true, false).apply(graph); + new ReadEliminationPhase().apply(graph); + new CanonicalizerPhase(runtime(), null).apply(graph); + + Debug.dump(graph, "After lowering"); + + ArrayList merges = new ArrayList<>(); + ArrayList reads = new ArrayList<>(); + for (Node n : graph.getNodes()) { + if (n instanceof MergeNode) { + // check shape + MergeNode merge = (MergeNode) n; + + if (merge.inputs().count() == 2) { + for (EndNode m : merge.forwardEnds()) { + if (m.predecessor() != null && m.predecessor() instanceof BeginNode && m.predecessor().predecessor() instanceof IfNode) { + IfNode o = (IfNode) m.predecessor().predecessor(); + if (o.falseSuccessor().next() instanceof DeoptimizeNode) { + merges.add(merge); + } + } + } + } + } + if (n instanceof IntegerAddNode) { + IntegerAddNode ian = (IntegerAddNode) n; + + Assert.assertTrue(ian.y() instanceof ConstantNode); + Assert.assertTrue(ian.x() instanceof FloatingReadNode); + reads.add((FloatingReadNode) ian.x()); + } + } + + Assert.assertTrue(merges.size() >= reads.size()); + for (int i = 0; i < reads.size(); i++) { + assertOrderedAfterSchedule(graph, merges.get(i), reads.get(i)); + } + } + }); + } +} diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/GraalCompiler.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/GraalCompiler.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/GraalCompiler.java Thu Mar 28 14:28:11 2013 +0100 @@ -200,6 +200,10 @@ new EliminatePartiallyRedundantGuardsPhase(true, true).apply(graph); } + if (GraalOptions.OptCanonicalizer) { + new CanonicalizerPhase(runtime, assumptions).apply(graph); + } + plan.runPhases(PhasePosition.MID_LEVEL, graph); plan.runPhases(PhasePosition.LOW_LEVEL, graph); diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CheckCastSnippets.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CheckCastSnippets.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/CheckCastSnippets.java Thu Mar 28 14:28:11 2013 +0100 @@ -81,7 +81,11 @@ } exactHit.inc(); } - return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic()); + /** + * make sure that the unsafeCast is done *after* the check above, + * cf. {@link ReadAfterCheckCast}*/ + BeginNode anchorNode = BeginNode.anchor(StampFactory.forNodeIntrinsic()); + return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic(), anchorNode); } /** @@ -109,7 +113,8 @@ } displayHit.inc(); } - return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic()); + BeginNode anchorNode = BeginNode.anchor(StampFactory.forNodeIntrinsic()); + return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic(), anchorNode); } /** @@ -132,14 +137,16 @@ Word hintHub = hints[i]; if (hintHub.equal(objectHub)) { hintsHit.inc(); - return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic()); + BeginNode anchorNode = BeginNode.anchor(StampFactory.forNodeIntrinsic()); + return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic(), anchorNode); } } if (!checkSecondarySubType(hub, objectHub)) { DeoptimizeNode.deopt(InvalidateReprofile, ClassCastException); } } - return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic()); + BeginNode anchorNode = BeginNode.anchor(StampFactory.forNodeIntrinsic()); + return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic(), anchorNode); } /** @@ -160,7 +167,8 @@ DeoptimizeNode.deopt(InvalidateReprofile, ClassCastException); } } - return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic()); + BeginNode anchorNode = BeginNode.anchor(StampFactory.forNodeIntrinsic()); + return unsafeCast(verifyOop(object), StampFactory.forNodeIntrinsic(), anchorNode); } // @formatter:on diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/BeginNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/BeginNode.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/BeginNode.java Thu Mar 28 14:28:11 2013 +0100 @@ -184,4 +184,7 @@ throw new UnsupportedOperationException(); } } + + @NodeIntrinsic + public static native T anchor(@ConstantNodeParameter Stamp stamp); } diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/InvokeNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/InvokeNode.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/InvokeNode.java Thu Mar 28 14:28:11 2013 +0100 @@ -160,20 +160,15 @@ StateSplit stateSplit = (StateSplit) node; stateSplit.setStateAfter(stateAfter); } - if (node == null) { - assert kind() == Kind.Void && usages().isEmpty(); - ((StructuredGraph) graph()).removeFixed(this); + if (node instanceof FixedWithNextNode) { + ((StructuredGraph) graph()).replaceFixedWithFixed(this, (FixedWithNextNode) node); + } else if (node instanceof DeoptimizeNode) { + this.replaceAtPredecessor(node); + this.replaceAtUsages(null); + GraphUtil.killCFG(this); + return; } else { - if (node instanceof FixedWithNextNode) { - ((StructuredGraph) graph()).replaceFixedWithFixed(this, (FixedWithNextNode) node); - } else if (node instanceof DeoptimizeNode) { - this.replaceAtPredecessor(node); - this.replaceAtUsages(null); - GraphUtil.killCFG(this); - return; - } else { - ((StructuredGraph) graph()).replaceFixed(this, node); - } + ((StructuredGraph) graph()).replaceFixed(this, node); } call.safeDelete(); if (stateAfter.usages().isEmpty()) { diff -r 59dab34ba44a -r 04b002b7077f graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeCastNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeCastNode.java Thu Mar 28 14:26:38 2013 +0100 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/UnsafeCastNode.java Thu Mar 28 14:28:11 2013 +0100 @@ -44,6 +44,11 @@ this.object = object; } + public UnsafeCastNode(ValueNode object, Stamp stamp, ValueNode anchor) { + super(stamp, anchor); + this.object = object; + } + public UnsafeCastNode(ValueNode object, ResolvedJavaType toType, boolean exactType, boolean nonNull) { this(object, toType.getKind() == Kind.Object ? StampFactory.object(toType, exactType, nonNull || object.stamp().nonNull()) : StampFactory.forKind(toType.getKind())); } @@ -109,6 +114,9 @@ @NodeIntrinsic public static native T unsafeCast(Object object, @ConstantNodeParameter Stamp stamp); + @NodeIntrinsic + public static native T unsafeCast(Object object, @ConstantNodeParameter Stamp stamp, ValueNode anchor); + @SuppressWarnings("unused") @NodeIntrinsic public static T unsafeCast(Object object, @ConstantNodeParameter Class toType, @ConstantNodeParameter boolean exactType, @ConstantNodeParameter boolean nonNull) {