# HG changeset patch
# User Doug Simon <doug.simon@oracle.com>
# Date 1385380005 -3600
# Node ID 3f1c70baa3bd6fcb1bee96114077c2d7c25f8304
# Parent  f9d908fb3492cc097f521f12aaaf1534bbc85161
use separate data structure for canonicalizing ConstantNodes (GRAAL-508)

diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/Constant.java
--- a/graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/Constant.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.api.meta/src/com/oracle/graal/api/meta/Constant.java	Mon Nov 25 12:46:45 2013 +0100
@@ -250,7 +250,7 @@
         if (getKind() == Kind.Object) {
             return System.identityHashCode(object);
         }
-        return (int) primitive;
+        return (int) primitive * getKind().ordinal();
     }
 
     /**
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java	Mon Nov 25 12:46:45 2013 +0100
@@ -75,6 +75,11 @@
     NodeChangedListener usagesDroppedToZeroListener;
     private final HashMap<CacheEntry, Node> cachedNodes = new HashMap<>();
 
+    /**
+     * Determines if external nodes will use {@link Graph}'s canonicalization cache.
+     */
+    public static final boolean CacheExternalNodesInGraph = Boolean.parseBoolean(System.getProperty("graal.cacheExternalNodesInGraph", "false"));
+
     private static final class CacheEntry {
 
         private final Node node;
@@ -343,7 +348,7 @@
     }
 
     /**
-     * Looks for a node <i>similar</i> to {@code node}. If not found, {@code node} is added to the
+     * Looks for a node <i>similar</i> to {@code node}. If not found, {@code node} is added to a
      * cache in this graph used to canonicalize nodes.
      * <p>
      * Note that node must implement {@link ValueNumberable} and must be an
@@ -351,9 +356,12 @@
      * 
      * @return a node similar to {@code node} if one exists, otherwise {@code node}
      */
-    public <T extends Node> T uniqueWithoutAdd(T node) {
+    public <T extends Node> T uniqueExternal(T node) {
         assert node.isExternal() : node;
         assert node instanceof ValueNumberable : node;
+        if (!CacheExternalNodesInGraph) {
+            return node;
+        }
         return uniqueHelper(node, false);
     }
 
@@ -380,6 +388,7 @@
     }
 
     Node findNodeInCache(Node node) {
+        assert !node.isExternal() || CacheExternalNodesInGraph;
         CacheEntry key = new CacheEntry(node);
         Node result = cachedNodes.get(key);
         if (result != null && result.isDeleted()) {
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Mon Nov 25 12:46:45 2013 +0100
@@ -712,7 +712,7 @@
     }
 
     /**
-     * Determines if a given node is {@linkplain Graph#uniqueWithoutAdd(Node) unique} within a given
+     * Determines if a given node is {@linkplain Graph#uniqueExternal(Node) unique} within a given
      * graph if the node is non-null and {@linkplain #isExternal() external}.
      * 
      * @param node node to check
@@ -721,7 +721,7 @@
      *         {@link VerificationError}
      */
     public static boolean verifyUniqueIfExternal(Node node, Graph graph) {
-        if (node != null && node.isExternal()) {
+        if (node != null && node.isExternal() && Graph.CacheExternalNodesInGraph) {
             Node cached = graph.findNodeInCache(node);
             node.assertTrue(cached == node, "external node does not match canonical node %s", cached);
         }
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Mon Nov 25 12:46:45 2013 +0100
@@ -1381,7 +1381,7 @@
 
             public Node replacement(Node node, boolean isInput) {
                 if (node.isExternal() && node instanceof ValueNumberable) {
-                    return graph.uniqueWithoutAdd(node);
+                    return graph.uniqueExternal(node);
                 }
                 Node target = newNodes.get(node);
                 if (target == null) {
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/TypeCheckSnippetUtils.java
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/TypeCheckSnippetUtils.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/TypeCheckSnippetUtils.java	Mon Nov 25 12:46:45 2013 +0100
@@ -30,7 +30,6 @@
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.api.meta.*;
-import com.oracle.graal.graph.*;
 import com.oracle.graal.hotspot.meta.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.replacements.*;
@@ -122,7 +121,7 @@
         }
     }
 
-    static Hints createHints(TypeCheckHints hints, MetaAccessProvider metaAccess, boolean positiveOnly, Graph graph) {
+    static Hints createHints(TypeCheckHints hints, MetaAccessProvider metaAccess, boolean positiveOnly, StructuredGraph graph) {
         ConstantNode[] hubs = new ConstantNode[hints.hints.length];
         boolean[] isPositive = new boolean[hints.hints.length];
         int index = 0;
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ConstantNode.java	Mon Nov 25 12:46:45 2013 +0100
@@ -22,6 +22,8 @@
  */
 package com.oracle.graal.nodes;
 
+import static com.oracle.graal.graph.Graph.*;
+
 import java.util.*;
 
 import com.oracle.graal.api.meta.*;
@@ -42,19 +44,19 @@
 
     private final Constant value;
 
-    protected ConstantNode(Constant value) {
-        super(StampFactory.forConstant(value));
-        this.value = value;
-        ConstantNodes.increment();
+    private static ConstantNode createPrimitive(Constant value) {
+        assert value.getKind() != Kind.Object;
+        return new ConstantNode(value, StampFactory.forConstant(value));
     }
 
     /**
-     * Constructs a new ConstantNode representing the specified constant.
+     * Constructs a new node representing the specified constant.
      * 
      * @param value the constant
      */
-    protected ConstantNode(Constant value, MetaAccessProvider metaAccess) {
-        super(StampFactory.forConstant(value, metaAccess));
+    protected ConstantNode(Constant value, Stamp stamp) {
+        super(stamp);
+        assert stamp != null;
         this.value = value;
         ConstantNodes.increment();
     }
@@ -110,9 +112,9 @@
     }
 
     /**
-     * Gathers all the {@link ConstantNode}s that are inputs to the {@linkplain Graph#getNodes()
-     * live nodes} in a given graph. This is an expensive operation that should only be used in
-     * test/verification/AOT code.
+     * Gathers all the {@link ConstantNode}s that are inputs to the
+     * {@linkplain StructuredGraph#getNodes() live nodes} in a given graph. This is an expensive
+     * operation that should only be used in test/verification/AOT code.
      */
     public static NodeIterable<ConstantNode> getConstantNodes(StructuredGraph graph) {
         Map<ConstantNode, ConstantNode> result = new HashMap<>();
@@ -166,20 +168,25 @@
         return true;
     }
 
-    public static ConstantNode forConstant(Constant constant, MetaAccessProvider metaAccess, Graph graph) {
+    public static ConstantNode forConstant(Constant constant, MetaAccessProvider metaAccess, StructuredGraph graph) {
         if (constant.getKind().getStackKind() == Kind.Int && constant.getKind() != Kind.Int) {
             return forInt(constant.asInt(), graph);
-        } else if (constant.getKind() == Kind.Object) {
-            return unique(graph, new ConstantNode(constant, metaAccess));
+        }
+        if (!CacheExternalNodesInGraph) {
+            Stamp stamp = constant.getKind() == Kind.Object ? StampFactory.forConstant(constant, metaAccess) : StampFactory.forConstant(constant);
+            return graph.asConstantNode(constant, stamp);
+        }
+        if (constant.getKind() == Kind.Object) {
+            return unique(graph, new ConstantNode(constant, StampFactory.forConstant(constant, metaAccess)));
         } else {
-            return unique(graph, new ConstantNode(constant));
+            return unique(graph, createPrimitive(constant));
         }
     }
 
     /**
      * Returns a node for a primitive constant.
      */
-    public static ConstantNode forPrimitive(Constant constant, Graph graph) {
+    public static ConstantNode forPrimitive(Constant constant, StructuredGraph graph) {
         assert constant.getKind() != Kind.Object;
         return forConstant(constant, null, graph);
     }
@@ -190,8 +197,11 @@
      * @param d the double value for which to create the instruction
      * @return a node for a double constant
      */
-    public static ConstantNode forDouble(double d, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forDouble(d)));
+    public static ConstantNode forDouble(double d, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forDouble(d), null);
+        }
+        return unique(graph, createPrimitive(Constant.forDouble(d)));
     }
 
     /**
@@ -200,8 +210,11 @@
      * @param f the float value for which to create the instruction
      * @return a node for a float constant
      */
-    public static ConstantNode forFloat(float f, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forFloat(f)));
+    public static ConstantNode forFloat(float f, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forFloat(f), null);
+        }
+        return unique(graph, createPrimitive(Constant.forFloat(f)));
     }
 
     /**
@@ -210,8 +223,11 @@
      * @param i the long value for which to create the instruction
      * @return a node for an long constant
      */
-    public static ConstantNode forLong(long i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forLong(i)));
+    public static ConstantNode forLong(long i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forLong(i), null);
+        }
+        return unique(graph, createPrimitive(Constant.forLong(i)));
     }
 
     /**
@@ -220,8 +236,11 @@
      * @param i the integer value for which to create the instruction
      * @return a node for an integer constant
      */
-    public static ConstantNode forInt(int i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forInt(i)));
+    public static ConstantNode forInt(int i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forInt(i), null);
+        }
+        return unique(graph, createPrimitive(Constant.forInt(i)));
     }
 
     /**
@@ -230,8 +249,11 @@
      * @param i the boolean value for which to create the instruction
      * @return a node representing the boolean
      */
-    public static ConstantNode forBoolean(boolean i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forInt(i ? 1 : 0)));
+    public static ConstantNode forBoolean(boolean i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(i ? Constant.INT_1 : Constant.INT_0, null);
+        }
+        return unique(graph, createPrimitive(Constant.forInt(i ? 1 : 0)));
     }
 
     /**
@@ -240,8 +262,11 @@
      * @param i the byte value for which to create the instruction
      * @return a node representing the byte
      */
-    public static ConstantNode forByte(byte i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forInt(i)));
+    public static ConstantNode forByte(byte i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forInt(i), null);
+        }
+        return unique(graph, createPrimitive(Constant.forInt(i)));
     }
 
     /**
@@ -250,8 +275,11 @@
      * @param i the char value for which to create the instruction
      * @return a node representing the char
      */
-    public static ConstantNode forChar(char i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forInt(i)));
+    public static ConstantNode forChar(char i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forInt(i), null);
+        }
+        return unique(graph, createPrimitive(Constant.forInt(i)));
     }
 
     /**
@@ -260,8 +288,11 @@
      * @param i the short value for which to create the instruction
      * @return a node representing the short
      */
-    public static ConstantNode forShort(short i, Graph graph) {
-        return unique(graph, new ConstantNode(Constant.forInt(i)));
+    public static ConstantNode forShort(short i, StructuredGraph graph) {
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(Constant.forInt(i), null);
+        }
+        return unique(graph, createPrimitive(Constant.forInt(i)));
     }
 
     /**
@@ -270,16 +301,21 @@
      * @param o the object value for which to create the instruction
      * @return a node representing the object
      */
-    public static ConstantNode forObject(Object o, MetaAccessProvider metaAccess, Graph graph) {
+    public static ConstantNode forObject(Object o, MetaAccessProvider metaAccess, StructuredGraph graph) {
         assert !(o instanceof Constant) : "wrapping a Constant into a Constant";
-        return unique(graph, new ConstantNode(Constant.forObject(o), metaAccess));
+        Constant constant = Constant.forObject(o);
+        if (!CacheExternalNodesInGraph) {
+            return graph.asConstantNode(constant, StampFactory.forConstant(constant, metaAccess));
+        }
+        return unique(graph, new ConstantNode(constant, StampFactory.forConstant(constant, metaAccess)));
     }
 
-    private static ConstantNode unique(Graph graph, ConstantNode node) {
-        return graph.uniqueWithoutAdd(node);
+    private static ConstantNode unique(StructuredGraph graph, ConstantNode node) {
+        assert CacheExternalNodesInGraph;
+        return graph.uniqueExternal(node);
     }
 
-    public static ConstantNode forIntegerKind(Kind kind, long value, Graph graph) {
+    public static ConstantNode forIntegerKind(Kind kind, long value, StructuredGraph graph) {
         switch (kind) {
             case Byte:
             case Short:
@@ -292,7 +328,7 @@
         }
     }
 
-    public static ConstantNode forFloatingKind(Kind kind, double value, Graph graph) {
+    public static ConstantNode forFloatingKind(Kind kind, double value, StructuredGraph graph) {
         switch (kind) {
             case Float:
                 return ConstantNode.forFloat((float) value, graph);
@@ -303,7 +339,7 @@
         }
     }
 
-    public static ConstantNode defaultForKind(Kind kind, Graph graph) {
+    public static ConstantNode defaultForKind(Kind kind, StructuredGraph graph) {
         switch (kind) {
             case Boolean:
             case Byte:
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/StructuredGraph.java
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/StructuredGraph.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/StructuredGraph.java	Mon Nov 25 12:46:45 2013 +0100
@@ -29,6 +29,7 @@
 import com.oracle.graal.graph.*;
 import com.oracle.graal.nodes.calc.*;
 import com.oracle.graal.nodes.java.*;
+import com.oracle.graal.nodes.type.*;
 import com.oracle.graal.nodes.util.*;
 
 /**
@@ -81,6 +82,38 @@
     private boolean isAfterFloatingReadPhase = false;
 
     /**
+     * Used to create canonical {@link ConstantNode}s for {@link Constant}s in this graph.
+     */
+    private Map<Constant, ConstantNode> constants;
+
+    /**
+     * Gets a node for a given constant that is unique/canonical within this graph.
+     * 
+     * @param stamp the stamp for an {@link Kind#Object} constant (ignored otherwise)
+     */
+    public ConstantNode asConstantNode(Constant constant, Stamp stamp) {
+        ConstantNode node;
+        if (constants == null) {
+            constants = new HashMap<>();
+            node = null;
+        } else {
+            node = constants.get(constant);
+        }
+        if (node == null) {
+            node = new ConstantNode(constant, stamp == null ? StampFactory.forConstant(constant) : stamp);
+            constants.put(constant, node);
+        }
+        return node;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public <T extends Node> T uniqueExternal(T node) {
+        ConstantNode cn = (ConstantNode) node;
+        return (T) asConstantNode(cn.asConstant(), cn.stamp());
+    }
+
+    /**
      * Creates a new Graph containing a single {@link AbstractBeginNode} as the {@link #start()
      * start} node.
      */
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java	Mon Nov 25 12:46:45 2013 +0100
@@ -1447,7 +1447,7 @@
                 if (!returnValue.isExternal()) {
                     returnValue = duplicates.get(returnValue);
                 } else if (returnValue instanceof ValueNumberable) {
-                    returnValue = graph.uniqueWithoutAdd(returnValue);
+                    returnValue = graph.uniqueExternal(returnValue);
                 }
             }
             invoke.asNode().replaceAtUsages(returnValue);
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/NewMultiArrayTest.java
--- a/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/NewMultiArrayTest.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.replacements.test/src/com/oracle/graal/replacements/test/NewMultiArrayTest.java	Mon Nov 25 12:46:45 2013 +0100
@@ -59,7 +59,7 @@
             int rank = dimensions.length;
             ValueNode[] dimensionNodes = new ValueNode[rank];
             for (int i = 0; i < rank; i++) {
-                dimensionNodes[i] = graph.unique(ConstantNode.forInt(dimensions[i], graph));
+                dimensionNodes[i] = ConstantNode.forInt(dimensions[i], graph);
             }
 
             NewMultiArrayNode repl = graph.add(new NewMultiArrayNode(arrayType, dimensionNodes));
diff -r f9d908fb3492 -r 3f1c70baa3bd graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluatorCanonicalizer.java
--- a/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluatorCanonicalizer.java	Sat Nov 23 23:20:03 2013 +0100
+++ b/graal/com.oracle.graal.truffle/src/com/oracle/graal/truffle/PartialEvaluatorCanonicalizer.java	Mon Nov 25 12:46:45 2013 +0100
@@ -53,7 +53,7 @@
                                 loadFieldNode.field().getAnnotation(CompilerDirectives.CompilationFinal.class) != null) {
                     Constant constant = loadFieldNode.field().readValue(loadFieldNode.object().asConstant());
                     assert verifyFieldValue(loadFieldNode.field(), constant);
-                    return ConstantNode.forConstant(constant, metaAccess, node.graph());
+                    return ConstantNode.forConstant(constant, metaAccess, loadFieldNode.graph());
                 }
             }
         } else if (node instanceof LoadIndexedNode) {