# HG changeset patch # User Doug Simon # Date 1379975735 -7200 # Node ID 6e734982f89f35b9fa54c1c7592488ef2b679c93 # Parent 095325ccbf9ab4ce1abe2587e137d26afd101ec6 fixed concurrency issue in lowering of MacroNode replacement graphs diff -r 095325ccbf9a -r 6e734982f89f graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ArrayCopyNode.java --- 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) { diff -r 095325ccbf9a -r 6e734982f89f graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/ObjectCloneNode.java --- 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) { diff -r 095325ccbf9a -r 6e734982f89f graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/nodes/MacroNode.java --- 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); } }