changeset 21014:c11ea4cb5765

Merge
author Matthias Grimmer <grimmer@ssw.jku.at>
date Mon, 20 Apr 2015 10:58:47 +0200
parents 556b6a4b36b2 (current diff) f6369bd988c7 (diff)
children 87e03398a25e
files graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/NodeIntrinsificationVerificationPhase.java
diffstat 10 files changed, 83 insertions(+), 191 deletions(-) [+]
line wrap: on
line diff
--- 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());
--- 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);
--- 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.
--- 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) {
--- 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
--- 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;
--- 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");
-    }
-}
--- 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) {
--- 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];
--- 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));