# HG changeset patch # User Tom Rodriguez # Date 1404243478 25200 # Node ID 657ff04a7b7369d4c0db941fcd464cd3f9003298 # Parent 67500ef4d102161283af20b766263109f483563e look for original method and substitution when processing snippet graph diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java --- 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); diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ObjectAccessTest.java --- 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 diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/PointerTest.java --- 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 diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/ReplacementsParseTest.java --- /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); + } + } +} diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/WordTest.java --- 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 diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/GraphKit.java --- 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); } diff -r 67500ef4d102 -r 657ff04a7b73 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java --- 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();