# HG changeset patch # User Doug Simon # Date 1336505555 -7200 # Node ID 33b8603f180d8ecb43b8a601e58cc9ebd22cd285 # Parent 827854645d6ce01ec6999b20ef72d33a515786e9 lowering checkcasts with Java snippets diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/InsertStateAfterPlaceholderPhase.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/InsertStateAfterPlaceholderPhase.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/InsertStateAfterPlaceholderPhase.java Tue May 08 21:32:35 2012 +0200 @@ -38,6 +38,11 @@ public void generate(LIRGeneratorTool gen) { // nothing to do } + + @Override + public boolean hasSideEffect() { + return false; + } } @Override diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java --- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java Tue May 08 21:32:35 2012 +0200 @@ -29,6 +29,7 @@ import com.oracle.graal.compiler.*; import com.oracle.graal.compiler.loop.*; import com.oracle.graal.compiler.phases.*; +import com.oracle.graal.compiler.phases.CanonicalizerPhase.IsImmutablePredicate; import com.oracle.graal.cri.*; import com.oracle.graal.debug.*; import com.oracle.graal.graph.*; @@ -909,12 +910,32 @@ * Performs replacement of a node with a snippet graph. * * @param replacee the node that will be replaced - * @param anchor if non-null, then this fixed node is control flow replacee. This is required iff replacee is not a fixed node. + * @param anchor the control flow replacee * @param snippetGraph the graph that the replacee will be replaced with * @param explodeLoops specifies if all the loops in the snippet graph are counted loops that must be completely unrolled * @param args */ - public static void snippetInline(RiRuntime runtime, Node replacee, FixedNode anchor, StructuredGraph snippetGraph, boolean explodeLoops, Object... args) { + public static void inlineSnippet(final RiRuntime runtime, + final Node replacee, + final FixedWithNextNode anchor, + final StructuredGraph snippetGraph, + final boolean explodeLoops, + final IsImmutablePredicate immutabilityPredicate, + final Object... args) { + Debug.scope("InliningSnippet", snippetGraph.method(), new Runnable() { + @Override + public void run() { + inlineSnippet0(runtime, replacee, anchor, snippetGraph, explodeLoops, immutabilityPredicate, args); + } + }); + } + private static void inlineSnippet0(RiRuntime runtime, + Node replacee, + FixedWithNextNode anchor, + StructuredGraph snippetGraph, + boolean explodeLoops, + IsImmutablePredicate immutabilityPredicate, + Object... args) { // Copy snippet graph, replacing parameters with given args in the process StructuredGraph snippetCopy = new StructuredGraph(snippetGraph.name, snippetGraph.method()); IdentityHashMap replacements = new IdentityHashMap<>(); @@ -935,6 +956,9 @@ } assert localCount == args.length : "snippet argument count mismatch"; snippetCopy.addDuplicates(snippetGraph.getNodes(), replacements); + if (!replacements.isEmpty()) { + new CanonicalizerPhase(null, runtime, null, false, immutabilityPredicate).apply(snippetCopy); + } // Explode all loops in the snippet if requested if (explodeLoops && snippetCopy.hasLoops()) { @@ -942,15 +966,17 @@ for (Loop loop : cfg.getLoops()) { LoopBeginNode loopBegin = loop.loopBegin(); SuperBlock wholeLoop = LoopTransformUtil.wholeLoop(loop); + Debug.dump(snippetCopy, "Before exploding loop %s", loopBegin); while (!loopBegin.isDeleted()) { snippetCopy.mark(); LoopTransformUtil.peel(loop, wholeLoop); - new CanonicalizerPhase(null, runtime, null, true, null).apply(snippetCopy); + new CanonicalizerPhase(null, runtime, null, true, immutabilityPredicate).apply(snippetCopy); } + Debug.dump(snippetCopy, "After exploding loop %s", loopBegin); } } - // Inline snippet + // Gather the nodes in the snippets that are to be inlined ArrayList nodes = new ArrayList<>(); ReturnNode returnNode = null; BeginNode entryPointNode = snippetCopy.start(); @@ -975,18 +1001,30 @@ } } + // Inline the gathered snippet nodes StructuredGraph graph = (StructuredGraph) replacee.graph(); Map duplicates = graph.addDuplicates(nodes, replacements); - FixedNode firstCFGNodeDuplicate = (FixedNode) duplicates.get(firstCFGNode); - if (anchor != null) { - assert !(replacee instanceof FixedNode) : "anchor not required when replacing fixed node " + replacee; - anchor.replaceAtPredecessors(firstCFGNodeDuplicate); - } else { - assert replacee.successors().first() != null; - assert replacee.predecessor() != null; - replacee.replaceAtPredecessors(firstCFGNodeDuplicate); + + // Remove all frame states from the inlined snippet graph. Snippets must be atomic (i.e. free + // of side-effects that prevent deoptimizing to a point before the snippet). + for (Node node : duplicates.values()) { + if (node instanceof StateSplit) { + StateSplit stateSplit = (StateSplit) node; + FrameState frameState = stateSplit.stateAfter(); + assert !stateSplit.hasSideEffect() : "snippets cannot contain side-effecting node " + node; + if (frameState != null) { + stateSplit.setStateAfter(null); + } + } } + // Rewire the control flow graph around the replacee + FixedNode firstCFGNodeDuplicate = (FixedNode) duplicates.get(firstCFGNode); + anchor.replaceAtPredecessors(firstCFGNodeDuplicate); + FixedNode next = anchor.next(); + anchor.setNext(null); + + // Replace all usages of the replacee with the value returned by the snippet Node returnValue = null; if (returnNode != null) { if (returnNode.result() instanceof LocalNode) { @@ -996,8 +1034,13 @@ } assert returnValue != null || replacee.usages().isEmpty(); replacee.replaceAtUsages(returnValue); + + Node returnDuplicate = duplicates.get(returnNode); + returnDuplicate.clearInputs(); + returnDuplicate.replaceAndDelete(next); } + // Remove the replacee from its graph replacee.clearInputs(); replacee.replaceAtUsages(null); if (replacee instanceof FixedNode) { diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/CheckCastSnippets.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/CheckCastSnippets.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/CheckCastSnippets.java Tue May 08 21:32:35 2012 +0200 @@ -46,7 +46,7 @@ return object; } } - if (!hintsAreExact && TypeCheckSlowPath.check(objectHub, hub) == null) { + if (hintsAreExact || TypeCheckSlowPath.check(objectHub, hub) == null) { DeoptimizeNode.deopt(RiDeoptAction.InvalidateReprofile, RiDeoptReason.ClassCastException); } return object; @@ -56,9 +56,4 @@ private static int hubOffset() { return CompilerImpl.getInstance().getConfig().hubOffset; } - - @Fold - private static int klassOopOffset() { - return CompilerImpl.getInstance().getConfig().klassOopOffset; - } } diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/LowerCheckCastPhase.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/LowerCheckCastPhase.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/LowerCheckCastPhase.java Tue May 08 21:32:35 2012 +0200 @@ -36,6 +36,9 @@ import com.oracle.max.cri.ci.*; import com.oracle.max.cri.ri.*; +/** + * Lowers a {@link CheckCastNode} by replacing it with the graph of a {@linkplain CheckCastSnippets checkcast snippet}. + */ public class LowerCheckCastPhase extends Phase { private final GraalRuntime runtime; private final RiResolvedMethod checkcast; @@ -49,41 +52,33 @@ } } - private static HotSpotKlassOop klassOop(RiResolvedType resolvedType) { - return ((HotSpotType) resolvedType).klassOop(); - } - @Override protected void run(StructuredGraph graph) { - // TODO (dnsimon) remove this once lowering works in general - if (graph.method() == null || !graph.method().holder().name().contains("LowerCheckCastTest")) { - return; - } - - int hits = 0; - graph.mark(); - IsImmutablePredicate immutabilityPredicate = null; - for (CheckCastNode ccn : graph.getNodes(CheckCastNode.class)) { - ValueNode hub = ccn.targetClassInstruction(); - ValueNode object = ccn.object(); - RiResolvedType[] hints = ccn.hints(); + final Map hintHubsSet = new IdentityHashMap<>(); + IsImmutablePredicate immutabilityPredicate = new IsImmutablePredicate() { + public boolean apply(CiConstant constant) { + return hintHubsSet.containsKey(constant); + } + }; + for (CheckCastNode node : graph.getNodes(CheckCastNode.class)) { + ValueNode hub = node.targetClassInstruction(); + ValueNode object = node.object(); + RiResolvedType[] hints = node.hints(); StructuredGraph snippetGraph = (StructuredGraph) checkcast.compilerStorage().get(Graph.class); - hits++; + assert snippetGraph != null : CheckCastSnippets.class.getSimpleName() + " should be installed"; HotSpotKlassOop[] hintHubs = new HotSpotKlassOop[hints.length]; for (int i = 0; i < hintHubs.length; i++) { - hintHubs[i] = klassOop(hints[i]); + hintHubs[i] = ((HotSpotType) hints[i]).klassOop(); } + assert !node.hintsExact() || hints.length > 0 : "cannot have 0 exact hints!"; final CiConstant hintHubsConst = CiConstant.forObject(hintHubs); - immutabilityPredicate = new IsImmutablePredicate() { - public boolean apply(CiConstant constant) { - return constant == hintHubsConst; - } - }; - Debug.log("Lowering checkcast in %s: ccn=%s, hintsHubs=%s, exact=%b", graph, ccn, Arrays.toString(hints), ccn.hintsExact()); - InliningUtil.snippetInline(runtime, ccn, ccn.anchor(), snippetGraph, true, hub, object, hintHubsConst, CiConstant.forBoolean(ccn.hintsExact())); + hintHubsSet.put(hintHubsConst, hintHubsConst); + Debug.log("Lowering checkcast in %s: node=%s, hintsHubs=%s, exact=%b", graph, node, Arrays.toString(hints), node.hintsExact()); + + InliningUtil.inlineSnippet(runtime, node, (FixedWithNextNode) node.anchor(), snippetGraph, true, immutabilityPredicate, hub, object, hintHubsConst, CiConstant.forBoolean(node.hintsExact())); } - if (hits != 0) { - Debug.log("Lowered %d checkcasts in %s ", hits, graph); + if (!hintHubsSet.isEmpty()) { + Debug.log("Lowered %d checkcasts in %s ", hintHubsSet.size(), graph); new DeadCodeEliminationPhase().apply(graph); new CanonicalizerPhase(null, runtime, null, false, immutabilityPredicate).apply(graph); } diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SafeAccessNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SafeAccessNode.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SafeAccessNode.java Tue May 08 21:32:35 2012 +0200 @@ -25,7 +25,10 @@ import com.oracle.graal.nodes.*; import com.oracle.graal.nodes.type.*; - +/** + * An analog to {@link AccessNode} with the additional semantics of null-checking + * the receiver object before the access. + */ public abstract class SafeAccessNode extends FixedWithNextNode { @Input private ValueNode object; diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippet.java --- a/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippet.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippet.java Tue May 08 21:32:35 2012 +0200 @@ -24,8 +24,17 @@ import java.lang.annotation.*; +import com.oracle.graal.graph.Node.NodeIntrinsic; + +/** + * A snippet is a Graal graph expressed as a Java source method. Graal snippets can be used for: + *
    + *
  • intrinsifying native JDK methods (see {@link ClassSubstitution})
    • + *
  • lowering operations that have runtime dependent semantics (e.g. the {@code CHECKCAST} bytecode)
    • + *
  • replacing a method call with a single graph node (see {@link NodeIntrinsic})
    • + *
+ */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface Snippet { - boolean atomic() default false; } diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippets.java --- a/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippets.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippets.java Tue May 08 21:32:35 2012 +0200 @@ -37,7 +37,6 @@ import com.oracle.graal.nodes.java.*; import com.oracle.max.cri.ci.*; import com.oracle.max.cri.ri.*; -import com.oracle.max.criutils.*; /** * Utilities for snippet installation and management. @@ -64,7 +63,7 @@ } RiResolvedMethod snippetRiMethod = runtime.getRiMethod(snippet); if (snippetRiMethod.compilerStorage().get(Graph.class) == null) { - buildSnippetGraph(snippetRiMethod, runtime, target, method.getAnnotation(Snippet.class).atomic(), pool); + buildSnippetGraph(snippetRiMethod, runtime, target, pool); } } } @@ -83,7 +82,7 @@ throw new RuntimeException("Snippet must not be abstract or native"); } RiResolvedMethod snippetRiMethod = runtime.getRiMethod(snippet); - StructuredGraph graph = buildSnippetGraph(snippetRiMethod, runtime, target, false, pool); + StructuredGraph graph = buildSnippetGraph(snippetRiMethod, runtime, target, pool); runtime.getRiMethod(method).compilerStorage().put(Graph.class, graph); } catch (NoSuchMethodException e) { throw new RuntimeException("Could not resolve method to substitute with: " + snippet.getName(), e); @@ -91,7 +90,7 @@ } } - private static StructuredGraph buildSnippetGraph(final RiResolvedMethod snippetRiMethod, final GraalRuntime runtime, final CiTarget target, final boolean atomic, final BoxingMethodPool pool) { + private static StructuredGraph buildSnippetGraph(final RiResolvedMethod snippetRiMethod, final GraalRuntime runtime, final CiTarget target, final BoxingMethodPool pool) { return Debug.scope("BuildSnippetGraph", snippetRiMethod, new Callable() { @Override @@ -112,7 +111,7 @@ if (holder.isSubtypeOf(runtime.getType(SnippetsInterface.class))) { StructuredGraph targetGraph = (StructuredGraph) targetMethod.compilerStorage().get(Graph.class); if (targetGraph == null) { - targetGraph = buildSnippetGraph(targetMethod, runtime, target, atomic, pool); + targetGraph = buildSnippetGraph(targetMethod, runtime, target, pool); } InliningUtil.inline(invoke, targetGraph, true); if (GraalOptions.OptCanonicalizer) { @@ -129,18 +128,6 @@ new CanonicalizerPhase(target, runtime, null).apply(graph); } - if (atomic) { - for (Node n : graph.getNodes()) { - if (n instanceof StateSplit) { - StateSplit stateSplit = (StateSplit) n; - if (stateSplit.stateAfter() != null) { - TTY.println(graph + ": " + stateSplit); - stateSplit.setStateAfter(null); - } - } - } - } - for (LoopEndNode end : graph.getNodes(LoopEndNode.class)) { end.disableSafepoint(); } diff -r 827854645d6c -r 33b8603f180d graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/LowerCheckCastTest.java --- a/graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/LowerCheckCastTest.java Tue May 08 20:17:30 2012 +0200 +++ b/graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/LowerCheckCastTest.java Tue May 08 21:32:35 2012 +0200 @@ -23,7 +23,6 @@ package com.oracle.graal.compiler.tests; import java.lang.reflect.*; -import java.util.*; import org.junit.*; @@ -32,27 +31,18 @@ import com.oracle.max.cri.ci.*; import com.oracle.max.cri.ri.*; -/** - * In the following tests, the usages of local variable "a" are replaced with the integer constant 0. - * Then canonicalization is applied and it is verified that the resulting graph is equal to the - * graph of the method that just has a "return 1" statement in it. - */ public class LowerCheckCastTest extends GraphTest { - static int warmup() { - Object[] numbers = {76L, (short) 34}; - int result = 0; - for (int i = 0; i < 20; i++) { - Object num = numbers[i % numbers.length]; - result += result + asNumber(num).intValue(); - } - return result; + static { + // Ensure that the methods to be compiled and executed are fully resolved + asNumber(0); + asString("0"); + asNumberExt(0); + asStringExt("0"); } private RiCompiledMethod compile(String name, Class[] hintClasses, boolean exact) { - System.out.println("compiling: " + name + ", hints=" + Arrays.toString(hintClasses) + ", exact=" + exact); - // Ensure that the method is fully resolved - asNumber(0); + //System.out.println("compiling: " + name + ", hints=" + Arrays.toString(hintClasses) + ", exact=" + exact); Method method = getMethod(name); final StructuredGraph graph = parse(method); @@ -72,46 +62,57 @@ return addMethod(riMethod, targetMethod); } - //@Test - public void test1() { - Class[] hints = {}; - RiCompiledMethod compiledMethod = compile("asNumber", hints, false); - Assert.assertEquals(Integer.valueOf(111), compiledMethod.executeVarargs(111)); + private static final boolean EXACT = true; + private static final boolean NOT_EXACT = false; + + private static Class[] hints(Class... classes) { + return classes; } - //@Test - public void test2() { - Class[] hints = {Integer.class}; - RiCompiledMethod compiledMethod = compile("asNumber", hints, false); - Assert.assertEquals(Integer.valueOf(111), compiledMethod.executeVarargs(111)); + private void test(String name, Class[] hints, boolean exact, Object expected, Object... args) { + RiCompiledMethod compiledMethod = compile(name, hints, exact); + Assert.assertEquals(expected, compiledMethod.executeVarargs(args)); + } + + @Test + public void test1() { + test("asNumber", hints(), NOT_EXACT, 111, 111); + test("asNumber", hints(Integer.class), NOT_EXACT, 111, 111); + test("asNumber", hints(Long.class, Short.class), NOT_EXACT, 111, 111); + test("asNumberExt", hints(), NOT_EXACT, 121, 111); + test("asNumberExt", hints(Integer.class), NOT_EXACT, 121, 111); + test("asNumberExt", hints(Long.class, Short.class), NOT_EXACT, 121, 111); } @Test - public void test3() { - Class[] hints = {Long.class, Short.class}; - RiCompiledMethod compiledMethod = compile("asNumber", hints, false); - Assert.assertEquals(Integer.valueOf(111), compiledMethod.executeVarargs(111)); - } + public void test2() { + test("asString", hints(), NOT_EXACT, "111", "111"); + test("asString", hints(String.class), EXACT, "111", "111"); + test("asString", hints(String.class), NOT_EXACT, "111", "111"); - //@Test - public void test4() { - Class[] hints = {}; - RiCompiledMethod compiledMethod = compile("asString", hints, true); - Assert.assertEquals("111", compiledMethod.executeVarargs("111")); + test("asStringExt", hints(), NOT_EXACT, "#111", "111"); + test("asStringExt", hints(String.class), EXACT, "#111", "111"); + test("asStringExt", hints(String.class), NOT_EXACT, "#111", "111"); } - @Test - public void test5() { - Class[] hints = {String.class}; - RiCompiledMethod compiledMethod = compile("asString", hints, true); - Assert.assertEquals("111", compiledMethod.executeVarargs("111")); + @Test(expected = ClassCastException.class) + public void test3() { + test("asNumber", hints(), NOT_EXACT, 111, "111"); } - //@Test(expected = ClassCastException.class) - public void test100() { - Class[] hints = {}; - RiCompiledMethod compiledMethod = compile("asNumber", hints, false); - compiledMethod.executeVarargs("number"); + @Test(expected = ClassCastException.class) + public void test4() { + test("asString", hints(String.class), EXACT, "111", 111); + } + + @Test(expected = ClassCastException.class) + public void test5() { + test("asNumberExt", hints(), NOT_EXACT, 111, "111"); + } + + @Test(expected = ClassCastException.class) + public void test6() { + test("asStringExt", hints(String.class), EXACT, "111", 111); } public static Number asNumber(Object o) { @@ -121,4 +122,14 @@ public static String asString(Object o) { return (String) o; } + + public static Number asNumberExt(Object o) { + Number n = (Number) o; + return n.intValue() + 10; + } + + public static String asStringExt(Object o) { + String s = (String) o; + return "#" + s; + } }