changeset 5365:33b8603f180d

lowering checkcasts with Java snippets
author Doug Simon <doug.simon@oracle.com>
date Tue, 08 May 2012 21:32:35 +0200
parents 827854645d6c
children 67e63e8dcbd2
files graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/InsertStateAfterPlaceholderPhase.java graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/util/InliningUtil.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/CheckCastSnippets.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/snippets/LowerCheckCastPhase.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/extended/SafeAccessNode.java graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippet.java graal/com.oracle.graal.snippets/src/com/oracle/graal/snippets/Snippets.java graal/com.oracle.graal.tests/src/com/oracle/graal/compiler/tests/LowerCheckCastTest.java
diffstat 8 files changed, 159 insertions(+), 111 deletions(-) [+]
line wrap: on
line diff
--- 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
--- 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<Node, Node> 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<Node> 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<Node, Node> 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) {
--- 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;
-    }
 }
--- 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<CiConstant, CiConstant> 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);
         }
--- 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;
--- 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:
+ * <ul>
+ * <li>intrinsifying native JDK methods (see {@link ClassSubstitution})</li>
+ * <li>lowering operations that have runtime dependent semantics (e.g. the {@code CHECKCAST} bytecode) </li>
+ * <li>replacing a method call with a single graph node (see {@link NodeIntrinsic})</li>
+ * </ul>
+ */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.METHOD)
 public @interface Snippet {
-    boolean atomic() default false;
 }
--- 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<StructuredGraph>() {
 
             @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();
                 }
--- 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;
+    }
 }