changeset 16383:657ff04a7b73

look for original method and substitution when processing snippet graph
author Tom Rodriguez <tom.rodriguez@oracle.com>
date Tue, 01 Jul 2014 12:37:58 -0700
parents 67500ef4d102
children 5d7b90ab9787
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ReplacementsParseTest.java graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java
diffstat 7 files changed, 177 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java	Tue Jul 01 12:37:58 2014 -0700
@@ -111,7 +111,7 @@
         ResolvedJavaMethod initMethod = null;
         try {
             Class<?> rjm = ResolvedJavaMethod.class;
-            makeGraphMethod = metaAccess.lookupJavaMethod(ReplacementsImpl.class.getDeclaredMethod("makeGraph", rjm, rjm, rjm, SnippetInliningPolicy.class, FrameStateProcessing.class));
+            makeGraphMethod = metaAccess.lookupJavaMethod(ReplacementsImpl.class.getDeclaredMethod("makeGraph", rjm, rjm, SnippetInliningPolicy.class, FrameStateProcessing.class));
             initMethod = metaAccess.lookupJavaMethod(SnippetTemplate.AbstractTemplates.class.getDeclaredMethod("template", Arguments.class));
         } catch (NoSuchMethodException | SecurityException e) {
             throw new GraalInternalError(e);
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java	Tue Jul 01 12:37:58 2014 -0700
@@ -55,7 +55,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
+        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
     }
 
     @Test
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java	Tue Jul 01 12:37:58 2014 -0700
@@ -61,7 +61,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
+        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
     }
 
     @Test
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ReplacementsParseTest.java	Tue Jul 01 12:37:58 2014 -0700
@@ -0,0 +1,126 @@
+/*
+ * Copyright (c) 2014, 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.replacements.*;
+import com.oracle.graal.api.runtime.*;
+import com.oracle.graal.compiler.test.*;
+import com.oracle.graal.nodes.spi.*;
+import com.oracle.graal.runtime.*;
+
+public class ReplacementsParseTest extends GraalCompilerTest {
+
+    static class TestMethods {
+        static double next(double v) {
+            return Math.nextAfter(v, 1.0);
+        }
+
+        static double next2(double v) {
+            return Math.nextAfter(v, 1.0);
+        }
+
+        static double nextAfter(double x, double d) {
+            return Math.nextAfter(x, d);
+        }
+
+    }
+
+    @ClassSubstitution(TestMethods.class)
+    static class TestMethodsSubstitutions {
+
+        @MethodSubstitution(isStatic = true)
+        static double next(double v) {
+            return TestMethods.next(v);
+        }
+
+        @MethodSubstitution(isStatic = true)
+        static double next2(double v) {
+            return next2(v);
+        }
+
+        @MethodSubstitution(isStatic = true)
+        static double nextAfter(double x, double d) {
+            double xx = (x == -0.0 ? 0.0 : x);
+            assert !Double.isNaN(xx);
+            return Math.nextAfter(xx, d);
+        }
+    }
+
+    private static boolean substitutionsInstalled;
+
+    public ReplacementsParseTest() {
+        if (!substitutionsInstalled) {
+            Replacements replacements = Graal.getRequiredCapability(RuntimeProvider.class).getHostBackend().getProviders().getReplacements();
+            replacements.registerSubstitutions(TestMethods.class, TestMethodsSubstitutions.class);
+            substitutionsInstalled = true;
+        }
+    }
+
+    /**
+     * Ensure that calling the original method from the substitution binds correctly.
+     */
+    @Test
+    public void test1() {
+        test("test1Snippet", 1.0);
+    }
+
+    public double test1Snippet(double d) {
+        return TestMethods.next(d);
+    }
+
+    /**
+     * Ensure that calling the substitution method binds to the original method properly.
+     */
+    @Test
+    public void test2() {
+        test("test2Snippet", 1.0);
+    }
+
+    public double test2Snippet(double d) {
+        return TestMethods.next2(d);
+    }
+
+    /**
+     * Ensure that substitution methods with assertions in them don't complain when the exception
+     * constructor is deleted.
+     */
+
+    @Test
+    public void testNextAfter() {
+        double[] inArray = new double[1024];
+        double[] outArray = new double[1024];
+        for (int i = 0; i < inArray.length; i++) {
+            inArray[i] = -0.0;
+        }
+        test("doNextAfter", inArray, outArray);
+    }
+
+    public void doNextAfter(double[] outArray, double[] inArray) {
+        for (int i = 0; i < inArray.length; i++) {
+            double direction = (i & 1) == 0 ? Double.POSITIVE_INFINITY : -Double.NEGATIVE_INFINITY;
+            outArray[i] = TestMethods.nextAfter(inArray[i], direction);
+        }
+    }
+}
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java	Tue Jul 01 12:37:58 2014 -0700
@@ -51,7 +51,7 @@
     @Override
     protected StructuredGraph parse(Method m) {
         ResolvedJavaMethod resolvedMethod = getMetaAccess().lookupJavaMethod(m);
-        return installer.makeGraph(resolvedMethod, null, resolvedMethod, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
+        return installer.makeGraph(resolvedMethod, null, inliningPolicy.get(), FrameStateProcessing.CollapseFrameForSingleSideEffect);
     }
 
     @Test
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java	Tue Jul 01 12:37:58 2014 -0700
@@ -207,7 +207,7 @@
     public void inline(InvokeNode invoke, SnippetReflectionProvider snippetReflection) {
         ResolvedJavaMethod method = ((MethodCallTargetNode) invoke.callTarget()).targetMethod();
         ReplacementsImpl repl = new ReplacementsImpl(providers, snippetReflection, new Assumptions(false), providers.getCodeCache().getTarget());
-        StructuredGraph calleeGraph = repl.makeGraph(method, null, method, null, FrameStateProcessing.CollapseFrameForSingleSideEffect);
+        StructuredGraph calleeGraph = repl.makeGraph(method, null, null, FrameStateProcessing.CollapseFrameForSingleSideEffect);
         InliningUtil.inline(invoke, calleeGraph, false, null);
     }
 
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Tue Jul 01 12:37:14 2014 -0700
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java	Tue Jul 01 12:37:58 2014 -0700
@@ -242,7 +242,7 @@
             try (TimerCloseable a = SnippetPreparationTime.start()) {
                 FrameStateProcessing frameStateProcessing = method.getAnnotation(Snippet.class).removeAllFrameStates() ? FrameStateProcessing.Removal
                                 : FrameStateProcessing.CollapseFrameForSingleSideEffect;
-                StructuredGraph newGraph = makeGraph(method, recursiveEntry, recursiveEntry, inliningPolicy(method), frameStateProcessing);
+                StructuredGraph newGraph = makeGraph(method, recursiveEntry, inliningPolicy(method), frameStateProcessing);
                 Debug.metric("SnippetNodeCount[%#s]", method).add(newGraph.getNodeCount());
                 if (!UseSnippetGraphCache) {
                     return newGraph;
@@ -278,7 +278,7 @@
         }
         StructuredGraph graph = graphs.get(substitute);
         if (graph == null) {
-            graph = makeGraph(substitute, original, substitute, inliningPolicy(substitute), FrameStateProcessing.None);
+            graph = makeGraph(substitute, original, inliningPolicy(substitute), FrameStateProcessing.None);
             graph.freeze();
             graphs.putIfAbsent(substitute, graph);
             graph = graphs.get(substitute);
@@ -405,15 +405,15 @@
      * @param policy the inlining policy to use during preprocessing
      * @param frameStateProcessing controls how {@link FrameState FrameStates} should be handled.
      */
-    public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, SnippetInliningPolicy policy, FrameStateProcessing frameStateProcessing) {
-        return createGraphMaker(method, original, recursiveEntry, frameStateProcessing).makeGraph(policy);
+    public StructuredGraph makeGraph(ResolvedJavaMethod method, ResolvedJavaMethod original, SnippetInliningPolicy policy, FrameStateProcessing frameStateProcessing) {
+        return createGraphMaker(method, original, frameStateProcessing).makeGraph(policy);
     }
 
     /**
      * Can be overridden to return an object that specializes various parts of graph preprocessing.
      */
-    protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, ResolvedJavaMethod recursiveEntry, FrameStateProcessing frameStateProcessing) {
-        return new GraphMaker(substitute, original, recursiveEntry, frameStateProcessing);
+    protected GraphMaker createGraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod original, FrameStateProcessing frameStateProcessing) {
+        return new GraphMaker(substitute, original, frameStateProcessing);
     }
 
     /**
@@ -437,24 +437,20 @@
         protected final ResolvedJavaMethod method;
 
         /**
-         * The method which is used when a call to {@link #recursiveEntry} is found.
+         * The original method which {@link #method} is substituting. Calls to {@link #method} or
+         * {@link #substitutedMethod} will be replaced with a forced inline of
+         * {@link #substitutedMethod}.
          */
         protected final ResolvedJavaMethod substitutedMethod;
 
         /**
-         * The method which is used to detect a recursive call.
-         */
-        protected final ResolvedJavaMethod recursiveEntry;
-
-        /**
          * Controls how FrameStates are processed.
          */
         private FrameStateProcessing frameStateProcessing;
 
-        protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, ResolvedJavaMethod recursiveEntry, FrameStateProcessing frameStateProcessing) {
+        protected GraphMaker(ResolvedJavaMethod substitute, ResolvedJavaMethod substitutedMethod, FrameStateProcessing frameStateProcessing) {
             this.method = substitute;
             this.substitutedMethod = substitutedMethod;
-            this.recursiveEntry = recursiveEntry;
             this.frameStateProcessing = frameStateProcessing;
         }
 
@@ -484,9 +480,9 @@
                 NodeIntrinsificationVerificationPhase.verify(graph);
             }
             int sideEffectCount = 0;
-            assert (sideEffectCount = graph.getNodes().filter(e -> e instanceof StateSplit && ((StateSplit) e).hasSideEffect()).count()) >= 0;
+            assert (sideEffectCount = graph.getNodes().filter(e -> hasSideEffect(e)).count()) >= 0;
             new ConvertDeoptimizeToGuardPhase().apply(graph);
-            assert sideEffectCount == graph.getNodes().filter(e -> e instanceof StateSplit && ((StateSplit) e).hasSideEffect()).count() : "deleted side effecting node";
+            assert sideEffectCount == graph.getNodes().filter(e -> hasSideEffect(e)).count() : "deleted side effecting node";
 
             switch (frameStateProcessing) {
                 case Removal:
@@ -503,6 +499,35 @@
             new DeadCodeEliminationPhase().apply(graph);
         }
 
+        /**
+         * Filter nodes has side effects and shouldn't be deleted from snippets when converting
+         * deoptimizations to guards. Currently this only allows exception constructors to be
+         * eliminated to cover the case when Java assertions are in the inlined code.
+         *
+         * @param node
+         * @return true for nodes that have side effects and are unsafe to delete
+         */
+        private boolean hasSideEffect(Node node) {
+            if (node instanceof StateSplit) {
+                if (((StateSplit) node).hasSideEffect()) {
+                    if (node instanceof Invoke) {
+                        CallTargetNode callTarget = ((Invoke) node).callTarget();
+                        if (callTarget instanceof MethodCallTargetNode) {
+                            ResolvedJavaMethod targetMethod = ((MethodCallTargetNode) callTarget).targetMethod();
+                            if (targetMethod.isConstructor()) {
+                                ResolvedJavaType throwableType = providers.getMetaAccess().lookupJavaType(Throwable.class);
+                                return !throwableType.isAssignableFrom(targetMethod.getDeclaringClass());
+                            }
+                        }
+                    }
+                    // Not an exception constructor call
+                    return true;
+                }
+            }
+            // Not a StateSplit
+            return false;
+        }
+
         private static final int MAX_GRAPH_INLINING_DEPTH = 100; // more than enough
 
         private StructuredGraph parseGraph(final ResolvedJavaMethod methodToParse, final SnippetInliningPolicy policy, int inliningDepth) {
@@ -585,7 +610,11 @@
                         continue;
                     }
                     ResolvedJavaMethod callee = callTarget.targetMethod();
-                    if (callee.equals(recursiveEntry)) {
+                    if (substitutedMethod != null && (callee.equals(method) || callee.equals(substitutedMethod))) {
+                        /*
+                         * Ensure that calls to the original method inside of a substitution ends up
+                         * calling it instead of the Graal substitution.
+                         */
                         if (isInlinable(substitutedMethod)) {
                             final StructuredGraph originalGraph = buildInitialGraph(substitutedMethod);
                             Mark mark = graph.getMark();