# HG changeset patch # User Lukas Stadler # Date 1403772526 -7200 # Node ID de84713267faaaa92568efb4b3dc8eb32f009720 # Parent db5b41891078dd25731a2b47240728009c8f0ba3 use default methods to select Canonicalizable behavior diff -r db5b41891078 -r de84713267fa 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 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)}. diff -r db5b41891078 -r de84713267fa 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 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; } /** diff -r db5b41891078 -r de84713267fa graal/com.oracle.graal.graph/src/com/oracle/graal/graph/spi/Canonicalizable.java --- 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 canonical(CanonicalizerTool tool, T forValue); + public interface Unary extends Canonicalizable { + Node canonical(CanonicalizerTool tool, T forValue); T getValue(); + + default Node canonical(CanonicalizerTool tool) { + return canonical(tool, getValue()); + } } - public interface Binary { - T canonical(CanonicalizerTool tool, T forX, T forY); + public interface Binary extends Canonicalizable { + Node canonical(CanonicalizerTool tool, T forX, T forY); T getX(); T getY(); + + default Node canonical(CanonicalizerTool tool) { + return canonical(tool, getX(), getY()); + } } } diff -r db5b41891078 -r de84713267fa graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java --- 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 unary = (Unary) node; - canonical = unary.canonical(tool, unary.getValue()); - } - break; - case BINARY: - try (AutoCloseable verify = getCanonicalizeableContractAssertion(node)) { - @SuppressWarnings("unchecked") - Canonicalizable.Binary binary = (Binary) 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; diff -r db5b41891078 -r de84713267fa graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/BaseReduction.java --- 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}. *

*/ public final class Tool implements CanonicalizerTool { diff -r db5b41891078 -r de84713267fa graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/EquationalReasoner.java --- 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 unary = (Unary) changed; - canon = unary.canonical(tool, unary.getValue()); - break; - case BINARY: - @SuppressWarnings("unchecked") - Canonicalizable.Binary binary = (Binary) 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; } /**