changeset 16239:de84713267fa

use default methods to select Canonicalizable behavior
author Lukas Stadler <lukas.stadler@oracle.com>
date Thu, 26 Jun 2014 10:48:46 +0200
parents db5b41891078
children ff06fc69249e
files graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java graal/com.oracle.graal.graph/src/com/oracle/graal/graph/spi/Canonicalizable.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/BaseReduction.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/EquationalReasoner.java
diffstat 6 files changed, 38 insertions(+), 91 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Thu Jun 26 10:48:46 2014 +0200
@@ -810,17 +810,6 @@
     }
 
     /**
-     * Must be overridden by subclasses that implement {@link Canonicalizable}. The implementation
-     * in {@link Node} exists to obviate the need to cast a node before invoking
-     * {@link Canonicalizable#canonical(CanonicalizerTool)}.
-     *
-     * @param tool
-     */
-    public Node canonical(CanonicalizerTool tool) {
-        throw new UnsupportedOperationException();
-    }
-
-    /**
      * Must be overridden by subclasses that implement {@link Simplifiable}. The implementation in
      * {@link Node} exists to obviate the need to cast a node before invoking
      * {@link Simplifiable#simplify(SimplifierTool)}.
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Thu Jun 26 10:48:46 2014 +0200
@@ -35,7 +35,6 @@
 import com.oracle.graal.graph.Node.Successor;
 import com.oracle.graal.graph.Node.Verbosity;
 import com.oracle.graal.graph.spi.*;
-import com.oracle.graal.graph.spi.Canonicalizable.CanonicalizeMethod;
 
 /**
  * Metadata for every {@link Node} type. The metadata includes:
@@ -103,7 +102,7 @@
     /**
      * Determines if this node type implements {@link Canonicalizable}.
      */
-    private final CanonicalizeMethod canonicalizeMethod;
+    private final boolean isCanonicalizable;
 
     /**
      * Determines if this node type implements {@link Simplifiable}.
@@ -117,16 +116,10 @@
     public NodeClass(Class<?> clazz, CalcOffset calcOffset, int[] presetIterableIds, int presetIterableId) {
         super(clazz);
         assert NODE_CLASS.isAssignableFrom(clazz);
-        if (Canonicalizable.Unary.class.isAssignableFrom(clazz)) {
-            assert !Canonicalizable.Binary.class.isAssignableFrom(clazz) && !Canonicalizable.class.isAssignableFrom(clazz) : clazz;
-            this.canonicalizeMethod = CanonicalizeMethod.UNARY;
-        } else if (Canonicalizable.Binary.class.isAssignableFrom(clazz)) {
-            assert !Canonicalizable.class.isAssignableFrom(clazz) : clazz;
-            this.canonicalizeMethod = CanonicalizeMethod.BINARY;
-        } else if (Canonicalizable.class.isAssignableFrom(clazz)) {
-            this.canonicalizeMethod = CanonicalizeMethod.BASE;
-        } else {
-            this.canonicalizeMethod = null;
+
+        this.isCanonicalizable = Canonicalizable.class.isAssignableFrom(clazz);
+        if (Canonicalizable.Unary.class.isAssignableFrom(clazz) || Canonicalizable.Binary.class.isAssignableFrom(clazz)) {
+            assert Canonicalizable.Unary.class.isAssignableFrom(clazz) ^ Canonicalizable.Binary.class.isAssignableFrom(clazz) : clazz + " should implement either Unary or Binary, not both";
         }
 
         this.isSimplifiable = Simplifiable.class.isAssignableFrom(clazz);
@@ -261,8 +254,8 @@
     /**
      * Determines if this node type implements {@link Canonicalizable}.
      */
-    public CanonicalizeMethod getCanonicalizeMethod() {
-        return canonicalizeMethod;
+    public boolean isCanonicalizable() {
+        return isCanonicalizable;
     }
 
     /**
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/spi/Canonicalizable.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/spi/Canonicalizable.java	Thu Jun 26 10:48:46 2014 +0200
@@ -26,25 +26,27 @@
 
 public interface Canonicalizable {
 
-    public enum CanonicalizeMethod {
-        BASE,
-        UNARY,
-        BINARY
-    }
-
     Node canonical(CanonicalizerTool tool);
 
-    public interface Unary<T extends Node> {
-        T canonical(CanonicalizerTool tool, T forValue);
+    public interface Unary<T extends Node> extends Canonicalizable {
+        Node canonical(CanonicalizerTool tool, T forValue);
 
         T getValue();
+
+        default Node canonical(CanonicalizerTool tool) {
+            return canonical(tool, getValue());
+        }
     }
 
-    public interface Binary<T extends Node> {
-        T canonical(CanonicalizerTool tool, T forX, T forY);
+    public interface Binary<T extends Node> extends Canonicalizable {
+        Node canonical(CanonicalizerTool tool, T forX, T forY);
 
         T getX();
 
         T getY();
+
+        default Node canonical(CanonicalizerTool tool) {
+            return canonical(tool, getX(), getY());
+        }
     }
 }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Thu Jun 26 10:48:46 2014 +0200
@@ -24,15 +24,12 @@
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.api.meta.*;
-import com.oracle.graal.compiler.common.*;
 import com.oracle.graal.debug.*;
 import com.oracle.graal.debug.Debug.Scope;
 import com.oracle.graal.graph.*;
 import com.oracle.graal.graph.Graph.Mark;
 import com.oracle.graal.graph.Graph.NodeChangedListener;
 import com.oracle.graal.graph.spi.*;
-import com.oracle.graal.graph.spi.Canonicalizable.Binary;
-import com.oracle.graal.graph.spi.Canonicalizable.Unary;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.calc.*;
 import com.oracle.graal.nodes.util.*;
@@ -235,7 +232,7 @@
         private static AutoCloseable getCanonicalizeableContractAssertion(Node node) {
             boolean needsAssertion = false;
             assert (needsAssertion = true) == true;
-            if (needsAssertion) {
+            if (needsAssertion && (node instanceof Canonicalizable.Unary<?> || node instanceof Canonicalizable.Binary<?>)) {
                 Mark mark = node.graph().getMark();
                 return () -> {
                     assert mark.equals(node.graph().getMark()) : "new node created while canonicalizing " + node.getClass().getSimpleName() + " " + node + ": " +
@@ -251,30 +248,12 @@
         }
 
         public boolean baseTryCanonicalize(final Node node, NodeClass nodeClass) {
-            if (nodeClass.getCanonicalizeMethod() != null) {
+            if (nodeClass.isCanonicalizable()) {
                 METRIC_CANONICALIZATION_CONSIDERED_NODES.increment();
                 try (Scope s = Debug.scope("CanonicalizeNode", node)) {
                     Node canonical;
-                    switch (nodeClass.getCanonicalizeMethod()) {
-                        case BASE:
-                            canonical = node.canonical(tool);
-                            break;
-                        case UNARY:
-                            try (AutoCloseable verify = getCanonicalizeableContractAssertion(node)) {
-                                @SuppressWarnings("unchecked")
-                                Canonicalizable.Unary<Node> unary = (Unary<Node>) node;
-                                canonical = unary.canonical(tool, unary.getValue());
-                            }
-                            break;
-                        case BINARY:
-                            try (AutoCloseable verify = getCanonicalizeableContractAssertion(node)) {
-                                @SuppressWarnings("unchecked")
-                                Canonicalizable.Binary<Node> binary = (Binary<Node>) node;
-                                canonical = binary.canonical(tool, binary.getX(), binary.getY());
-                            }
-                            break;
-                        default:
-                            throw GraalInternalError.shouldNotReachHere("unexpected CanonicalizeMethod");
+                    try (AutoCloseable verify = getCanonicalizeableContractAssertion(node)) {
+                        canonical = ((Canonicalizable) node).canonical(tool);
                     }
                     if (performReplacement(node, canonical)) {
                         return true;
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/BaseReduction.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/BaseReduction.java	Thu Jun 26 10:48:46 2014 +0200
@@ -27,7 +27,7 @@
 import com.oracle.graal.debug.Debug;
 import com.oracle.graal.debug.DebugMetric;
 import com.oracle.graal.graph.Node;
-import com.oracle.graal.graph.spi.CanonicalizerTool;
+import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.compiler.common.type.ObjectStamp;
 import com.oracle.graal.phases.graph.SinglePassNodeIterator;
@@ -140,9 +140,8 @@
      * One of the promises of
      * {@link com.oracle.graal.phases.common.cfs.EquationalReasoner#deverbosify(com.oracle.graal.graph.Node)}
      * is that a "maximally reduced" node is returned. That is achieved in part by leveraging
-     * {@link com.oracle.graal.graph.Node#canonical(com.oracle.graal.graph.spi.CanonicalizerTool)}.
-     * Doing so, in turn, requires this subclass of
-     * {@link com.oracle.graal.graph.spi.CanonicalizerTool}.
+     * {@link Canonicalizable#canonical(com.oracle.graal.graph.spi.CanonicalizerTool)}. Doing so, in
+     * turn, requires this subclass of {@link com.oracle.graal.graph.spi.CanonicalizerTool}.
      * </p>
      */
     public final class Tool implements CanonicalizerTool {
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/EquationalReasoner.java	Thu Jun 26 10:16:19 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/EquationalReasoner.java	Thu Jun 26 10:48:46 2014 +0200
@@ -27,12 +27,10 @@
 import java.util.*;
 
 import com.oracle.graal.api.meta.*;
-import com.oracle.graal.compiler.common.*;
 import com.oracle.graal.compiler.common.type.*;
 import com.oracle.graal.debug.*;
 import com.oracle.graal.graph.*;
 import com.oracle.graal.graph.spi.*;
-import com.oracle.graal.graph.spi.Canonicalizable.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.calc.*;
 import com.oracle.graal.nodes.extended.*;
@@ -335,32 +333,19 @@
         FlowUtil.inferStampAndCheck(changed);
         added.add(changed);
 
-        Node canon;
-        switch (changed.getNodeClass().getCanonicalizeMethod()) {
-            case BASE:
-                canon = changed.canonical(tool);
-                break;
-            case UNARY:
-                @SuppressWarnings("unchecked")
-                Canonicalizable.Unary<Node> unary = (Unary<Node>) changed;
-                canon = unary.canonical(tool, unary.getValue());
-                break;
-            case BINARY:
-                @SuppressWarnings("unchecked")
-                Canonicalizable.Binary<Node> binary = (Binary<Node>) changed;
-                canon = binary.canonical(tool, binary.getX(), binary.getY());
-                break;
-            default:
-                throw GraalInternalError.shouldNotReachHere("unexpected CanonicalizeMethod");
+        if (changed instanceof Canonicalizable) {
+            ValueNode canon = (ValueNode) ((Canonicalizable) changed).canonical(tool);
+            if (canon != null && !canon.isAlive()) {
+                assert !canon.isDeleted();
+                canon = graph.addOrUniqueWithInputs(canon);
+            }
+            // might be already in `added`, no problem adding it again.
+            added.add(canon);
+            rememberSubstitution(f, canon);
+            return canon;
+        } else {
+            return changed;
         }
-        if (canon != null && !canon.isAlive()) {
-            assert !canon.isDeleted();
-            canon = graph.addOrUniqueWithInputs(canon);
-        }
-        // might be already in `added`, no problem adding it again.
-        added.add((ValueNode) canon);
-        rememberSubstitution(f, (ValueNode) canon);
-        return canon;
     }
 
     /**