changeset 11754:6e734982f89f

fixed concurrency issue in lowering of MacroNode replacement graphs
author Doug Simon <doug.simon@oracle.com>
date Tue, 24 Sep 2013 00:35:35 +0200
parents 095325ccbf9a
children bbcb72443066
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ObjectCloneNode.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java
diffstat 3 files changed, 35 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java	Tue Sep 24 00:29:41 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java	Tue Sep 24 00:35:35 2013 +0200
@@ -100,7 +100,7 @@
     }
 
     @Override
-    protected StructuredGraph getSnippetGraph(final LoweringTool tool) {
+    protected StructuredGraph getLoweredSnippetGraph(final LoweringTool tool) {
         if (!shouldIntrinsify(getTargetMethod())) {
             return null;
         }
@@ -119,10 +119,9 @@
             replaceSnippetInvokes(snippetGraph);
         } else {
             assert snippetGraph != null : "ArrayCopySnippets should be installed";
-
+            snippetGraph = snippetGraph.copy();
             if (getLength().isConstant() && getLength().asConstant().asInt() <= GraalOptions.MaximumEscapeAnalysisArrayLength.getValue()) {
-                final StructuredGraph copy = snippetGraph.copy();
-                snippetGraph = copy;
+                final StructuredGraph copy = snippetGraph;
                 Debug.scope("ArrayCopySnippetSpecialization", snippetGraph.method(), new Runnable() {
 
                     @Override
@@ -130,10 +129,9 @@
                         unrollFixedLengthLoop(copy, getLength().asConstant().asInt(), tool);
                     }
                 });
-
             }
         }
-        return snippetGraph;
+        return lowerReplacement(snippetGraph, tool);
     }
 
     private static boolean checkBounds(int position, int length, VirtualObjectNode virtualObject) {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ObjectCloneNode.java	Tue Sep 24 00:29:41 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ObjectCloneNode.java	Tue Sep 24 00:35:35 2013 +0200
@@ -53,7 +53,7 @@
     }
 
     @Override
-    protected StructuredGraph getSnippetGraph(LoweringTool tool) {
+    protected StructuredGraph getLoweredSnippetGraph(LoweringTool tool) {
         if (!shouldIntrinsify(getTargetMethod())) {
             return null;
         }
@@ -82,7 +82,7 @@
         });
 
         assert snippetGraph != null : "ObjectCloneSnippets should be installed";
-        return snippetGraph;
+        return lowerReplacement(snippetGraph.copy(), tool);
     }
 
     private static boolean isCloneableType(ResolvedJavaType type, MetaAccessProvider metaAccess) {
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java	Tue Sep 24 00:29:41 2013 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java	Tue Sep 24 00:35:35 2013 +0200
@@ -25,6 +25,7 @@
 import java.lang.reflect.*;
 
 import com.oracle.graal.api.meta.*;
+import com.oracle.graal.debug.*;
 import com.oracle.graal.graph.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.extended.*;
@@ -64,33 +65,51 @@
     }
 
     /**
-     * Gets a snippet to be used for lowering this macro node. The returned graph (if non-null) is
-     * not shared and can be modified by the caller.
+     * Gets a snippet to be used for lowering this macro node. The returned graph (if non-null) must
+     * have been {@linkplain #lowerReplacement(StructuredGraph, LoweringTool) lowered}.
      */
     @SuppressWarnings("unused")
-    protected StructuredGraph getSnippetGraph(LoweringTool tool) {
+    protected StructuredGraph getLoweredSnippetGraph(LoweringTool tool) {
         return null;
     }
 
     /**
      * Gets a normal method substitution to be used for lowering this macro node. This is only
-     * called if {@link #getSnippetGraph(LoweringTool)} returns null. The returned graph (if
-     * non-null) is not shared and can be modified by the caller.
+     * called if {@link #getLoweredSnippetGraph(LoweringTool)} returns null. The returned graph (if
+     * non-null) must have been {@linkplain #lowerReplacement(StructuredGraph, LoweringTool)
+     * lowered}.
      */
-    protected StructuredGraph getSubstitutionGraph(LoweringTool tool) {
+    protected StructuredGraph getLoweredSubstitutionGraph(LoweringTool tool) {
         StructuredGraph methodSubstitution = tool.getReplacements().getMethodSubstitution(getTargetMethod());
         if (methodSubstitution != null) {
-            methodSubstitution = methodSubstitution.copy();
+            return lowerReplacement(methodSubstitution.copy(), tool);
         }
-        return methodSubstitution;
+        return null;
+    }
+
+    /**
+     * Applies {@linkplain LoweringPhase lowering} to a replacement graph.
+     * 
+     * @param replacementGraph a replacement (i.e., snippet or method substitution) graph
+     */
+    protected StructuredGraph lowerReplacement(final StructuredGraph replacementGraph, LoweringTool tool) {
+        replacementGraph.setGuardsStage(graph().getGuardsStage());
+        final PhaseContext c = new PhaseContext(tool.getRuntime(), tool.assumptions(), tool.getReplacements());
+        Debug.scope("LoweringReplacement", replacementGraph, new Runnable() {
+
+            public void run() {
+                new LoweringPhase(new CanonicalizerPhase(true)).apply(replacementGraph, c);
+            }
+        });
+        return replacementGraph;
     }
 
     @Override
     public void lower(LoweringTool tool) {
         boolean nullCheck = false;
-        StructuredGraph replacementGraph = getSnippetGraph(tool);
+        StructuredGraph replacementGraph = getLoweredSnippetGraph(tool);
         if (replacementGraph == null) {
-            replacementGraph = getSubstitutionGraph(tool);
+            replacementGraph = getLoweredSubstitutionGraph(tool);
             nullCheck = true;
         }
 
@@ -98,10 +117,6 @@
         assert invoke.verify();
 
         if (replacementGraph != null) {
-            // Lower the (non-shared) replacement graph
-            replacementGraph.setGuardsStage(graph().getGuardsStage());
-            PhaseContext c = new PhaseContext(tool.getRuntime(), tool.assumptions(), tool.getReplacements());
-            new LoweringPhase(new CanonicalizerPhase(true)).apply(replacementGraph, c);
             InliningUtil.inline(invoke, replacementGraph, nullCheck);
         }
     }