# HG changeset patch # User Matthias Grimmer # Date 1429520327 -7200 # Node ID c11ea4cb57653098d2ab40435aa546141acbfcf9 # Parent 556b6a4b36b26c06edfe2204914b3601dba89567# Parent f6369bd988c75b8181f38f397cbc755510af07bf Merge diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IfCanonicalizerTest.java --- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IfCanonicalizerTest.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IfCanonicalizerTest.java Mon Apr 20 10:58:47 2015 +0200 @@ -179,6 +179,17 @@ return v - 1; } + @Test + public void test9() { + testCombinedIf("test9Snippet", 2); + test("test9Snippet", -1); + test("test9Snippet", 1025); + } + + public static int test9Snippet(int n) { + return (n < 0) ? 1 : (n >= 1024) ? 1024 : n + 1; + } + private void testCombinedIf(String snippet, int count) { StructuredGraph graph = parseEager(snippet, AllowAssumptions.YES); PhaseContext context = new PhaseContext(getProviders()); diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java --- a/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.hotspot.test/src/com/oracle/graal/hotspot/test/WriteBarrierAdditionTest.java Mon Apr 20 10:58:47 2015 +0200 @@ -49,7 +49,6 @@ import com.oracle.graal.phases.common.inlining.*; import com.oracle.graal.phases.common.inlining.policy.*; import com.oracle.graal.phases.tiers.*; -import com.oracle.graal.replacements.*; /** * The following unit tests assert the presence of write barriers for both Serial and G1 GCs. @@ -251,7 +250,6 @@ StructuredGraph graph = parseEager(snippet, AllowAssumptions.NO); HighTierContext highContext = getDefaultHighTierContext(); MidTierContext midContext = new MidTierContext(getProviders(), getCodeCache().getTarget(), OptimisticOptimizations.ALL, graph.method().getProfilingInfo(), null); - new NodeIntrinsificationPhase(getMetaAccess(), getConstantReflection(), getSnippetReflection(), getProviders().getForeignCalls(), getProviders().getStampProvider()).apply(graph); new InliningPhase(new InlineEverythingPolicy(), new CanonicalizerPhase()).apply(graph, highContext); new CanonicalizerPhase().apply(graph, highContext); new LoweringPhase(new CanonicalizerPhase(), LoweringTool.StandardLoweringStage.HIGH_TIER).apply(graph, highContext); diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java --- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Mon Apr 20 10:58:47 2015 +0200 @@ -404,6 +404,9 @@ } LoopBeginNode loopBegin = (LoopBeginNode) ((EndNode) merge.next()).merge(); LoopEndNode loopEnd = graph.add(new LoopEndNode(loopBegin)); + if (parsingReplacement()) { + loopEnd.disableSafepoint(); + } endNode.replaceAndDelete(loopEnd); } else if (visited.contains(n)) { // Normal merge into a branch we are already exploring. diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/IfNode.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/IfNode.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/IfNode.java Mon Apr 20 10:58:47 2015 +0200 @@ -340,7 +340,12 @@ EndNode end1 = (EndNode) next1; EndNode end2 = (EndNode) next2; if (end1.merge() == end2.merge()) { - // They go to the same MergeNode + for (PhiNode phi : end1.merge().phis()) { + if (phi.valueAt(end1) != phi.valueAt(end2)) { + return false; + } + } + // They go to the same MergeNode and merge the same values return true; } } else if (next1 instanceof DeoptimizeNode && next2 instanceof DeoptimizeNode) { diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/spi/Replacements.java Mon Apr 20 10:58:47 2015 +0200 @@ -59,15 +59,6 @@ void registerSnippet(ResolvedJavaMethod method); /** - * Notifies this object during snippet specialization once the specialized snippet's constant - * parameters have been replaced with constant values. - * - * @param specializedSnippet the snippet in the process of being specialized. This is a copy of - * the unspecialized snippet graph created during snippet preparation. - */ - void notifyAfterConstantsBound(StructuredGraph specializedSnippet); - - /** * Gets a graph that is a substitution for a given method. * * @param invokeBci the call site BCI if this request is made for inlining a substitute diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/DefaultGenericInvocationPlugin.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/DefaultGenericInvocationPlugin.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/DefaultGenericInvocationPlugin.java Mon Apr 20 10:58:47 2015 +0200 @@ -29,6 +29,7 @@ import com.oracle.graal.api.meta.*; import com.oracle.graal.api.replacements.*; +import com.oracle.graal.compiler.*; import com.oracle.graal.compiler.common.*; import com.oracle.graal.compiler.common.type.*; import com.oracle.graal.graph.Node.NodeIntrinsic; @@ -56,6 +57,21 @@ this.structuralInputType = metaAccess.lookupJavaType(StructuralInput.class); } + /** + * Calls in replacements to methods matching one of these filters are elided. Only void methods + * are considered for elision. The use of "snippets" in name of the variable and system property + * is purely for legacy reasons. + */ + private static final MethodFilter[] MethodsElidedInSnippets = getMethodsElidedInSnippets(); + + private static MethodFilter[] getMethodsElidedInSnippets() { + String commaSeparatedPatterns = System.getProperty("graal.MethodsElidedInSnippets"); + if (commaSeparatedPatterns != null) { + return MethodFilter.parse(commaSeparatedPatterns); + } + return null; + } + public boolean apply(GraphBuilderContext b, ResolvedJavaMethod method, ValueNode[] args) { if (b.parsingReplacement() && wordOperationPlugin.apply(b, method, args)) { return true; @@ -93,6 +109,13 @@ } return true; } + } else if (MethodsElidedInSnippets != null) { + if (MethodFilter.matches(MethodsElidedInSnippets, method)) { + if (method.getSignature().getReturnKind() != Kind.Void) { + throw new GraalInternalError("Cannot elide non-void method " + method.format("%H.%n(%p)")); + } + return true; + } } } return false; diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/NodeIntrinsificationVerificationPhase.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/NodeIntrinsificationVerificationPhase.java Mon Apr 20 10:58:35 2015 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,66 +0,0 @@ -/* - * Copyright (c) 2011, 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; - -import com.oracle.graal.api.meta.*; -import com.oracle.graal.api.replacements.*; -import com.oracle.graal.compiler.common.*; -import com.oracle.graal.graph.*; -import com.oracle.graal.graph.Node.ConstantNodeParameter; -import com.oracle.graal.graph.Node.NodeIntrinsic; -import com.oracle.graal.nodes.*; -import com.oracle.graal.nodes.java.*; -import com.oracle.graal.phases.*; - -/** - * Checks that a graph contains no calls to {@link NodeIntrinsic} or {@link Fold} methods. - */ -public class NodeIntrinsificationVerificationPhase extends Phase { - - public static void verify(StructuredGraph graph) { - new NodeIntrinsificationVerificationPhase().apply(graph); - } - - @Override - protected void run(StructuredGraph graph) { - for (MethodCallTargetNode n : graph.getNodes(MethodCallTargetNode.TYPE)) { - checkInvoke(n); - } - } - - private static void checkInvoke(MethodCallTargetNode n) { - ResolvedJavaMethod target = n.targetMethod(); - if (target.getAnnotation(Node.NodeIntrinsic.class) != null) { - error(n, "Intrinsification"); - } else if (target.getAnnotation(Fold.class) != null) { - error(n, "Folding"); - } - } - - private static void error(MethodCallTargetNode n, String failedAction) throws GraalInternalError { - String context = n.graph().method().format("%H.%n"); - String target = n.invoke().callTarget().targetName(); - throw new GraalInternalError(failedAction + " of call to '" + target + "' in '" + context + "' failed, most likely due to a parameter annotated with @" + - ConstantNodeParameter.class.getSimpleName() + " not being resolvable to a constant during compilation"); - } -} diff -r 556b6a4b36b2 -r c11ea4cb5765 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 Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Mon Apr 20 10:58:47 2015 +0200 @@ -38,7 +38,6 @@ import com.oracle.graal.api.code.*; import com.oracle.graal.api.meta.*; import com.oracle.graal.api.replacements.*; -import com.oracle.graal.compiler.*; import com.oracle.graal.compiler.common.*; import com.oracle.graal.debug.*; import com.oracle.graal.debug.Debug.Scope; @@ -64,7 +63,6 @@ public final Providers providers; public final SnippetReflectionProvider snippetReflection; public final TargetDescription target; - public final NodeIntrinsificationPhase nodeIntrinsificationPhase; private GraphBuilderConfiguration.Plugins graphBuilderPlugins; /** @@ -78,7 +76,7 @@ } protected boolean hasGenericInvocationPluginAnnotation(ResolvedJavaMethod method) { - return nodeIntrinsificationPhase.getIntrinsic(method) != null || method.getAnnotation(Word.Operation.class) != null || nodeIntrinsificationPhase.isFoldable(method); + return method.getAnnotation(Node.NodeIntrinsic.class) != null || method.getAnnotation(Word.Operation.class) != null || method.getAnnotation(Fold.class) != null; } private static final int MAX_GRAPH_INLINING_DEPTH = 100; // more than enough @@ -112,7 +110,7 @@ // Force inlining when parsing replacements return new InlineInfo(method, true, true); } else { - assert nodeIntrinsificationPhase.getIntrinsic(method) == null : String.format("@%s method %s must only be called from within a replacement%n%s", NodeIntrinsic.class.getSimpleName(), + assert method.getAnnotation(NodeIntrinsic.class) == null : String.format("@%s method %s must only be called from within a replacement%n%s", NodeIntrinsic.class.getSimpleName(), method.format("%h.%n"), b); } return null; @@ -265,7 +263,6 @@ this.target = target; this.graphs = new ConcurrentHashMap<>(); this.snippetTemplateCache = CollectionsFactory.newMap(); - this.nodeIntrinsificationPhase = createNodeIntrinsificationPhase(); } private static final boolean UseSnippetGraphCache = Boolean.parseBoolean(System.getProperty("graal.useSnippetGraphCache", "true")); @@ -320,20 +317,6 @@ } @Override - public void notifyAfterConstantsBound(StructuredGraph specializedSnippet) { - - // Do deferred intrinsification of node intrinsics - - nodeIntrinsificationPhase.apply(specializedSnippet); - new CanonicalizerPhase().apply(specializedSnippet, new PhaseContext(providers)); - NodeIntrinsificationVerificationPhase.verify(specializedSnippet); - } - - protected NodeIntrinsificationPhase createNodeIntrinsificationPhase() { - return new NodeIntrinsificationPhase(providers.getMetaAccess(), providers.getConstantReflection(), snippetReflection, providers.getForeignCalls(), providers.getStampProvider()); - } - - @Override public StructuredGraph getSubstitution(ResolvedJavaMethod original, boolean fromBytecodeOnly, int invokeBci) { ResolvedJavaMethod substitute = null; if (!fromBytecodeOnly) { @@ -491,20 +474,6 @@ } /** - * Calls in snippets to methods matching one of these filters are elided. Only void methods are - * considered for elision. - */ - private static final MethodFilter[] MethodsElidedInSnippets = getMethodsElidedInSnippets(); - - private static MethodFilter[] getMethodsElidedInSnippets() { - String commaSeparatedPatterns = System.getProperty("graal.MethodsElidedInSnippets"); - if (commaSeparatedPatterns != null) { - return MethodFilter.parse(commaSeparatedPatterns); - } - return null; - } - - /** * Creates and preprocesses a graph for a replacement. */ public static class GraphMaker { @@ -558,10 +527,6 @@ * Does final processing of a snippet graph. */ protected void finalizeGraph(StructuredGraph graph) { - replacements.nodeIntrinsificationPhase.apply(graph); - if (!SnippetTemplate.hasConstantParameter(method)) { - NodeIntrinsificationVerificationPhase.verify(graph); - } int sideEffectCount = 0; assert (sideEffectCount = graph.getNodes().filter(e -> hasSideEffect(e)).count()) >= 0; new ConvertDeoptimizeToGuardPhase().apply(graph, null); @@ -612,20 +577,15 @@ } private StructuredGraph parseGraph(final ResolvedJavaMethod methodToParse, Object[] args) { - StructuredGraph graph = args == null ? replacements.graphCache.get(methodToParse) : null; + assert methodToParse.hasBytecodes() : methodToParse; + if (args != null) { + return buildInitialGraph(methodToParse, args); + } + StructuredGraph graph = replacements.graphCache.get(methodToParse); if (graph == null) { - StructuredGraph newGraph = null; - try (Scope s = Debug.scope("ParseGraph", methodToParse)) { - newGraph = buildGraph(methodToParse, args); - } catch (Throwable e) { - throw Debug.handle(e); - } - if (args == null) { - replacements.graphCache.putIfAbsent(methodToParse, newGraph); - graph = replacements.graphCache.get(methodToParse); - } else { - graph = newGraph; - } + StructuredGraph newGraph = buildInitialGraph(methodToParse, args); + replacements.graphCache.putIfAbsent(methodToParse, newGraph); + graph = replacements.graphCache.get(methodToParse); assert graph != null; } return graph; @@ -645,17 +605,12 @@ try (Scope s = Debug.scope("buildInitialGraph", graph)) { MetaAccessProvider metaAccess = replacements.providers.getMetaAccess(); - if (MethodsElidedInSnippets != null && methodToParse.getSignature().getReturnKind() == Kind.Void && MethodFilter.matches(MethodsElidedInSnippets, methodToParse)) { - graph.addAfterFixed(graph.start(), graph.add(new ReturnNode(null))); - } else { - Plugins plugins = new Plugins(replacements.graphBuilderPlugins); - GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); - if (args != null) { - plugins.setParameterPlugin(new ConstantBindingParameterPlugin(args, plugins.getParameterPlugin(), metaAccess, replacements.snippetReflection)); - } - createGraphBuilder(metaAccess, replacements.providers.getStampProvider(), replacements.providers.getConstantReflection(), config, OptimisticOptimizations.NONE).apply(graph); + Plugins plugins = new Plugins(replacements.graphBuilderPlugins); + GraphBuilderConfiguration config = GraphBuilderConfiguration.getSnippetDefault(plugins); + if (args != null) { + plugins.setParameterPlugin(new ConstantBindingParameterPlugin(args, plugins.getParameterPlugin(), metaAccess, replacements.snippetReflection)); } - afterParsing(graph); + createGraphBuilder(metaAccess, replacements.providers.getStampProvider(), replacements.providers.getConstantReflection(), config, OptimisticOptimizations.NONE).apply(graph); if (OptCanonicalizer.getValue()) { new CanonicalizerPhase().apply(graph, new PhaseContext(replacements.providers)); @@ -679,56 +634,6 @@ } return new GraphBuilderPhase.Instance(metaAccess, stampProvider, constantReflection, graphBuilderConfig, optimisticOpts, initialReplacementContext); } - - /** - * @param graph - */ - protected void afterParsing(StructuredGraph graph) { - } - - protected Object beforeInline(@SuppressWarnings("unused") MethodCallTargetNode callTarget, @SuppressWarnings("unused") StructuredGraph callee) { - return null; - } - - /** - * Called after a graph is inlined. - * - * @param caller the graph into which {@code callee} was inlined - * @param callee the graph that was inlined into {@code caller} - * @param beforeInlineData value returned by {@link #beforeInline}. - */ - protected void afterInline(StructuredGraph caller, StructuredGraph callee, Object beforeInlineData) { - if (OptCanonicalizer.getValue()) { - new CanonicalizerPhase().apply(caller, new PhaseContext(replacements.providers)); - } - } - - /** - * Called after all inlining for a given graph is complete. - */ - protected void afterInlining(StructuredGraph graph) { - replacements.nodeIntrinsificationPhase.apply(graph); - new DeadCodeEliminationPhase(Optional).apply(graph); - if (OptCanonicalizer.getValue()) { - new CanonicalizerPhase().apply(graph, new PhaseContext(replacements.providers)); - } - } - - private StructuredGraph buildGraph(final ResolvedJavaMethod methodToParse, Object[] args) { - assert methodToParse.hasBytecodes() : methodToParse; - final StructuredGraph graph = buildInitialGraph(methodToParse, args); - try (Scope s = Debug.scope("buildGraph", graph)) { - - for (LoopEndNode end : graph.getNodes(LoopEndNode.TYPE)) { - end.disableSafepoint(); - } - - new DeadCodeEliminationPhase(Required).apply(graph); - } catch (Throwable e) { - throw Debug.handle(e); - } - return graph; - } } private static String originalName(Method substituteMethod, String methodSubstitution) { diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java Mon Apr 20 10:58:47 2015 +0200 @@ -612,9 +612,6 @@ snippetCopy.addDuplicates(snippetGraph.getNodes(), snippetGraph, snippetGraph.getNodeCount(), nodeReplacements); Debug.dump(snippetCopy, "Before specialization"); - if (!nodeReplacements.isEmpty()) { - providers.getReplacements().notifyAfterConstantsBound(snippetCopy); - } // Gather the template parameters parameters = new Object[parameterCount]; diff -r 556b6a4b36b2 -r c11ea4cb5765 graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/MethodGuardsTest.java --- a/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/MethodGuardsTest.java Mon Apr 20 10:58:35 2015 +0200 +++ b/graal/com.oracle.truffle.api.dsl.test/src/com/oracle/truffle/api/dsl/test/MethodGuardsTest.java Mon Apr 20 10:58:47 2015 +0200 @@ -50,6 +50,7 @@ import com.oracle.truffle.api.dsl.test.MethodGuardsTestFactory.GuardNotTestFactory; import com.oracle.truffle.api.dsl.test.MethodGuardsTestFactory.GuardOrTestFactory; import com.oracle.truffle.api.dsl.test.MethodGuardsTestFactory.GuardStaticFieldTestFactory; +import com.oracle.truffle.api.dsl.test.MethodGuardsTestFactory.GuardStaticFinalFieldCompareTestFactory; import com.oracle.truffle.api.dsl.test.MethodGuardsTestFactory.GuardUnboundMethodTestFactory; import com.oracle.truffle.api.dsl.test.TypeSystemTest.ValueNode; @@ -385,6 +386,30 @@ } @Test + public void testGuardStaticFinalFieldCompare() { + CallTarget root = createCallTarget(GuardStaticFinalFieldCompareTestFactory.getInstance()); + GuardStaticFieldTest.field = true; + assertEquals("do1", root.call(1)); + assertEquals("do2", root.call(2)); + } + + @NodeChild + static class GuardStaticFinalFieldCompareTest extends ValueNode { + + protected static final int FIELD = 1; + + @Specialization(guards = "value == FIELD") + static String do1(int value) { + return "do1"; + } + + @Specialization(guards = "value != FIELD") + static String do2(int value) { + return "do2"; + } + } + + @Test public void testGuardMethod() { CallTarget root = createCallTarget(GuardMethodTestFactory.getInstance()); assertEquals("do1", root.call(1));