changeset 3489:a64b615ba630

WIP : convert Conditional (IfOp) to use the new BooleanNode infrastructure, Canonicalize some phi to Conditional, some Conditional to Materialize, remove If useless if nodes
author Gilles Duboscq <gilles.duboscq@oracle.com>
date Mon, 01 Aug 2011 18:30:25 +0200
parents b9f76db5ac53
children ce7cf6d16b38
files graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/gen/LIRGenerator.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Condition.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Conditional.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/If.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/NegateBooleanNode.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Phi.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/ValueVisitor.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRAssembler.java graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRGenerator.java
diffstat 9 files changed, 182 insertions(+), 135 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/gen/LIRGenerator.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/gen/LIRGenerator.java	Mon Aug 01 18:30:25 2011 +0200
@@ -618,32 +618,6 @@
         }
     }
 
-    @Override
-    public void visitIfOp(Conditional i) {
-        Value x = i.x();
-        Value y = i.y();
-        CiKind xtype = x.kind;
-        CiKind ttype = i.trueValue().kind;
-        assert xtype.isInt() || xtype.isObject() : "cannot handle others";
-        assert ttype.isInt() || ttype.isObject() || ttype.isLong() || ttype.isWord() : "cannot handle others";
-        assert ttype.equals(i.falseValue().kind) : "cannot handle others";
-
-        CiValue left = load(x);
-        CiValue right = null;
-        if (!canInlineAsConstant(y)) {
-            right = load(y);
-        } else {
-            right = makeOperand(y);
-        }
-
-        CiValue tVal = makeOperand(i.trueValue());
-        CiValue fVal = makeOperand(i.falseValue());
-        CiValue reg = createResultVariable(i);
-
-        lir.cmp(i.condition(), left, right);
-        lir.cmove(i.condition(), tVal, fVal, reg);
-    }
-
     protected FrameState stateBeforeInvokeReturn(Invoke invoke) {
         return invoke.stateAfter().duplicateModified(invoke.bci, invoke.stateAfter().rethrowException(), invoke.kind);
     }
@@ -1787,7 +1761,7 @@
         }
     }
 
-    protected abstract boolean canInlineAsConstant(Value i);
+    public abstract boolean canInlineAsConstant(Value i);
 
     protected abstract boolean canStoreAsConstant(Value i, CiKind kind);
 
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Condition.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Condition.java	Mon Aug 01 18:30:25 2011 +0200
@@ -129,6 +129,8 @@
             case BE: return AT;
             case AT: return BE;
             case AE: return BT;
+            case OF: return NOF;
+            case NOF: return OF;
         }
         throw new IllegalArgumentException(this.toString());
     }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Conditional.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Conditional.java	Mon Aug 01 18:30:25 2011 +0200
@@ -22,22 +22,25 @@
  */
 package com.oracle.max.graal.compiler.ir;
 
+import com.oracle.max.asm.*;
 import com.oracle.max.graal.compiler.debug.*;
-import com.oracle.max.graal.compiler.util.*;
+import com.oracle.max.graal.compiler.gen.*;
+import com.oracle.max.graal.compiler.gen.LIRGenerator.LIRGeneratorOp;
+import com.oracle.max.graal.compiler.lir.*;
+import com.oracle.max.graal.compiler.phases.CanonicalizerPhase.CanonicalizerOp;
 import com.oracle.max.graal.graph.*;
 import com.sun.cri.bytecode.*;
 import com.sun.cri.ci.*;
 
 /**
- * The {@code IfOp} class represents a comparison that yields one of two values.
+ * The {@code Conditional} class represents a comparison that yields one of two values.
  * Note that these nodes are not built directly from the bytecode but are introduced
  * by conditional expression elimination.
  */
 public final class Conditional extends Binary {
 
-    private static final int INPUT_COUNT = 2;
-    private static final int INPUT_TRUE_VALUE = 0;
-    private static final int INPUT_FALSE_VALUE = 1;
+    private static final int INPUT_COUNT = 1;
+    private static final int INPUT_CONDITION = 0;
 
     private static final int SUCCESSOR_COUNT = 0;
 
@@ -51,32 +54,14 @@
         return super.successorCount() + SUCCESSOR_COUNT;
     }
 
-
-    /**
-     * The instruction that produces the value if the comparison is true.
-     */
-    public Value trueValue() {
-        return (Value) inputs().get(super.inputCount() + INPUT_TRUE_VALUE);
-    }
-
-    public Value setTrueValue(Value n) {
-        return (Value) inputs().set(super.inputCount() + INPUT_TRUE_VALUE, n);
+    public BooleanNode condition() {
+        return (BooleanNode) inputs().get(super.inputCount() + INPUT_CONDITION);
     }
 
-    /**
-     * The instruction that produces the value if the comparison is false.
-     */
-    public Value falseValue() {
-        return (Value) inputs().get(super.inputCount() + INPUT_FALSE_VALUE);
+    public void setCondition(BooleanNode n) {
+        inputs().set(super.inputCount() + INPUT_CONDITION, n);
     }
 
-    public Value setFalseValue(Value n) {
-        return (Value) inputs().set(super.inputCount() + INPUT_FALSE_VALUE, n);
-    }
-
-
-    Condition condition;
-
     /**
      * Constructs a new IfOp.
      * @param x the instruction producing the first value to be compared
@@ -85,51 +70,29 @@
      * @param trueValue the value produced if the condition is true
      * @param falseValue the value produced if the condition is false
      */
-    public Conditional(Value x, Condition condition, Value y, Value trueValue, Value falseValue, Graph graph) {
+    public Conditional(BooleanNode condition, Value trueValue, Value falseValue, Graph graph) {
         // TODO: return the appropriate bytecode IF_ICMPEQ, etc
-        super(trueValue.kind.meet(falseValue.kind), Bytecodes.ILLEGAL, x, y, INPUT_COUNT, SUCCESSOR_COUNT, graph);
-        this.condition = condition;
-        setTrueValue(trueValue);
-        setFalseValue(falseValue);
+        super(trueValue.kind.meet(falseValue.kind), Bytecodes.ILLEGAL, trueValue, falseValue, INPUT_COUNT, SUCCESSOR_COUNT, graph);
+        setCondition(condition);
     }
 
     // for copying
-    private Conditional(CiKind kind, Condition cond, Graph graph) {
+    private Conditional(CiKind kind, Graph graph) {
         super(kind, Bytecodes.ILLEGAL, null, null, INPUT_COUNT, SUCCESSOR_COUNT, graph);
-        this.condition = cond;
-    }
-
-    /**
-     * Gets the condition of this if operation.
-     * @return the condition
-     */
-    public Condition condition() {
-        return condition;
     }
 
-    /**
-     * Checks whether this comparison operator is commutative (i.e. it is either == or !=).
-     * @return {@code true} if this comparison is commutative
-     */
-    public boolean isCommutative() {
-        return condition == Condition.EQ || condition == Condition.NE;
+    public Value trueValue() {
+        return x();
     }
 
-    @Override
-    public void accept(ValueVisitor v) {
-        v.visitIfOp(this);
-    }
-
-    @Override
-    public int valueNumber() {
-        return Util.hash4(condition.hashCode(), x(), y(), trueValue(), falseValue());
+    public Value falseValue() {
+        return y();
     }
 
     @Override
     public boolean valueEqual(Node i) {
         if (i instanceof Conditional) {
-            Conditional o = (Conditional) i;
-            return opcode == o.opcode && x() == o.x() && y() == o.y() && trueValue() == o.trueValue() && falseValue() == o.falseValue();
+            return super.valueEqual(i);
         }
         return false;
     }
@@ -138,7 +101,7 @@
     public void print(LogStream out) {
         out.print(x()).
         print(' ').
-        print(condition().operator).
+        print(condition()).
         print(' ').
         print(y()).
         print(" ? ").
@@ -149,7 +112,102 @@
 
     @Override
     public Node copy(Graph into) {
-        Conditional x = new Conditional(kind, condition, into);
+        Conditional x = new Conditional(kind, into);
         return x;
     }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public <T extends Op> T lookup(Class<T> clazz) {
+        if (clazz == CanonicalizerOp.class) {
+            return (T) CANONICALIZER;
+        }
+        if (clazz == LIRGeneratorOp.class) {
+            return (T) LIRGEN;
+        }
+        return super.lookup(clazz);
+    }
+
+    private static final CanonicalizerOp CANONICALIZER = new CanonicalizerOp() {
+        @Override
+        public Node canonical(Node node) {
+            Conditional conditional = (Conditional) node;
+            BooleanNode condition = conditional.condition();
+            Value trueValue = conditional.trueValue();
+            Value falseValue = conditional.falseValue();
+            if (condition instanceof Constant) {
+                Constant c = (Constant) condition;
+                if (c.asConstant().asBoolean()) {
+                    return trueValue;
+                } else {
+                    return falseValue;
+                }
+            }
+            if (trueValue == falseValue) {
+                return trueValue;
+            }
+            if (trueValue instanceof Constant && falseValue instanceof Constant
+                            && trueValue.kind == CiKind.Int && falseValue.kind == CiKind.Int) {
+                int trueInt = trueValue.asConstant().asInt();
+                int falseInt = falseValue.asConstant().asInt();
+                if (trueInt == 0 && falseInt == 1) {
+                    TTY.println("> Conditional canon'ed to Materialize");
+                    return new MaterializeNode(new NegateBooleanNode(condition, node.graph()), node.graph());
+                } else if (trueInt == 1 && falseInt == 0) {
+                    TTY.println("> Conditional canon'ed to Materialize");
+                    return new MaterializeNode(condition, node.graph());
+                }
+            }
+            return conditional;
+        }
+    };
+
+    private static final LIRGeneratorOp LIRGEN = new LIRGeneratorOp() {
+
+        @Override
+        public void generate(Node n, LIRGenerator generator) {
+            Conditional conditional = (Conditional) n;
+            BooleanNode condition = conditional.condition();
+
+            CiVariable result = generator.createResultVariable(conditional);
+            // try to use cmp + cmov first
+            Condition cond = null;
+            CiValue left = null;
+            CiValue right = null;
+            if (condition instanceof Compare) {
+                Compare compare = (Compare) condition;
+                Value x = compare.x();
+                Value y = compare.y();
+                left = generator.load(x);
+                if (!generator.canInlineAsConstant(y)) {
+                    right = generator.load(y);
+                } else {
+                    right = generator.makeOperand(y);
+                }
+                cond = compare.condition;
+            } else if (condition instanceof IsNonNull) {
+                IsNonNull isNonNull = (IsNonNull) condition;
+                left = generator.load(isNonNull.object());
+                right = CiConstant.NULL_OBJECT;
+                cond = Condition.NE;
+            }
+            CiValue tVal = generator.makeOperand(conditional.trueValue());
+            CiValue fVal = generator.makeOperand(conditional.falseValue());
+            if (cond != null) {
+                assert left != null && right != null;
+                generator.lir().cmp(cond, left, right);
+                generator.lir().cmove(cond, tVal, fVal, result);
+            } else {
+                LIRBlock trueSuccessor = new LIRBlock(new Label(), null);
+                generator.emitBooleanBranch(condition, trueSuccessor, null, null);
+                LIRList lir = generator.lir();
+                lir.move(fVal, result);
+                Label label = new Label();
+                lir.branch(Condition.TRUE, label);
+                lir.branchDestination(trueSuccessor.label);
+                lir.move(tVal, result);
+                lir.branchDestination(label);
+            }
+        }
+    };
 }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/If.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/If.java	Mon Aug 01 18:30:25 2011 +0200
@@ -52,7 +52,7 @@
     /**
      * The instruction that produces the first input to this comparison.
      */
-     public BooleanNode compare() {
+    public BooleanNode compare() {
         return (BooleanNode) inputs().get(super.inputCount() + INPUT_COMPARE);
     }
 
@@ -162,6 +162,21 @@
                     return ifNode.falseSuccessor();
                 }
             }
+            if (ifNode.trueSuccessor() instanceof EndNode && ifNode.falseSuccessor() instanceof EndNode) {
+                EndNode trueEnd = (EndNode) ifNode.trueSuccessor();
+                EndNode falseEnd = (EndNode) ifNode.falseSuccessor();
+                if (trueEnd.merge() == falseEnd.merge() && trueEnd.merge().phis().size() == 0) {
+                    if (ifNode.compare().usages().size() == 1 && /*ifNode.compare().hasSideEffets()*/ true) { // TODO (gd) ifNode.compare().hasSideEffets() ?
+                        TTY.println("> Useless if with side effects Canon'ed to guard");
+                        FixedGuard guard = new FixedGuard(ifNode.compare(), node.graph());
+                        guard.setNext(trueEnd.merge().next());
+                        return guard;
+                    } else {
+                        TTY.println("> Useless if Canon'ed away");
+                        return trueEnd.merge().next();
+                    }
+                }
+            }
             return ifNode;
         }
     };
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/NegateBooleanNode.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/NegateBooleanNode.java	Mon Aug 01 18:30:25 2011 +0200
@@ -96,6 +96,10 @@
                 return ((NegateBooleanNode) value).value();
             } else if (value instanceof Constant) {
                 return Constant.forBoolean(!value.asConstant().asBoolean(), node.graph());
+            } else if (value instanceof Compare) {
+                Compare compare = (Compare) value;
+                compare.condition = compare.condition.negate();
+                return compare;
             }
             return negateNode;
         }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Phi.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Phi.java	Mon Aug 01 18:30:25 2011 +0200
@@ -190,46 +190,32 @@
         @Override
         public Node canonical(Node node) {
             Phi phiNode = (Phi) node;
-//            if (phiNode.valueCount() != 2 || phiNode.merge().endCount() != 2 || phiNode.merge().phis().size() != 1) {
-//                return phiNode;
-//            }
-//            if (!(phiNode.valueAt(0) instanceof Constant && phiNode.valueAt(1) instanceof Constant)) {
-//                return phiNode;
-//            }
-//            Merge merge = phiNode.merge();
-//            Node end0 = merge.endAt(0);
-//            Node end1 = merge.endAt(1);
-//            if (end0.predecessors().size() != 1 || end1.predecessors().size() != 1) {
-//                return phiNode;
-//            }
-//            Node endPred0 = end0.predecessors().get(0);
-//            Node endPred1 = end1.predecessors().get(0);
-//            if (endPred0 != endPred1 || !(endPred0 instanceof If)) {
-//                return phiNode;
-//            }
-//            If ifNode = (If) endPred0;
-//            if (ifNode.predecessors().size() != 1) {
-//                return phiNode;
-//            }
-//            boolean inverted = ((If) endPred0).trueSuccessor() == end1;
-//            CiConstant trueValue = phiNode.valueAt(inverted ? 1 : 0).asConstant();
-//            CiConstant falseValue = phiNode.valueAt(inverted ? 0 : 1).asConstant();
-//            if (trueValue.kind != CiKind.Int || falseValue.kind != CiKind.Int) {
-//                return phiNode;
-//            }
-//            if ((trueValue.asInt() == 0 || trueValue.asInt() == 1) && (falseValue.asInt() == 0 || falseValue.asInt() == 1) && (trueValue.asInt() != falseValue.asInt())) {
-//                MaterializeNode result;
-//                if (trueValue.asInt() == 1) {
-//                    result = new MaterializeNode(ifNode.compare(), phiNode.graph());
-//                } else {
-//                    result = new MaterializeNode(new NegateBooleanNode(ifNode.compare(), phiNode.graph()), phiNode.graph());
-//                }
-//                Node next = merge.next();
-//                merge.setNext(null);
-//                ifNode.predecessors().get(0).successors().replace(ifNode, next);
-//                return result;
-//            }
-            return phiNode;
+            if (phiNode.valueCount() != 2 || phiNode.merge().endCount() != 2) {
+                return phiNode;
+            }
+            Merge merge = phiNode.merge();
+            Node end0 = merge.endAt(0);
+            Node end1 = merge.endAt(1);
+            if (end0.predecessors().size() != 1 || end1.predecessors().size() != 1) {
+                return phiNode;
+            }
+            Node endPred0 = end0.predecessors().get(0);
+            Node endPred1 = end1.predecessors().get(0);
+            if (endPred0 != endPred1 || !(endPred0 instanceof If)) {
+                return phiNode;
+            }
+            If ifNode = (If) endPred0;
+            if (ifNode.predecessors().size() != 1) {
+                return phiNode;
+            }
+            boolean inverted = ifNode.trueSuccessor() == end1;
+            Value trueValue = phiNode.valueAt(inverted ? 1 : 0);
+            Value falseValue = phiNode.valueAt(inverted ? 0 : 1);
+            if (trueValue.kind != CiKind.Int || falseValue.kind != CiKind.Int) {
+                return phiNode;
+            }
+            TTY.println("> Phi canon'ed to Conditional");
+            return new Conditional(ifNode.compare(), trueValue, falseValue, node.graph());
         }
     };
 }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/ValueVisitor.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/ValueVisitor.java	Mon Aug 01 18:30:25 2011 +0200
@@ -43,7 +43,6 @@
     public abstract void visitFrameState(FrameState i);
     public abstract void visitAnchor(Anchor i);
     public abstract void visitIf(If i);
-    public abstract void visitIfOp(Conditional i);
     public abstract void visitInvoke(Invoke i);
     public abstract void visitLoadField(LoadField i);
     public abstract void visitLoadIndexed(LoadIndexed i);
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRAssembler.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRAssembler.java	Mon Aug 01 18:30:25 2011 +0200
@@ -754,6 +754,14 @@
                 acond = ConditionFlag.above;
                 ncond = ConditionFlag.belowEqual;
                 break;
+            case OF:
+                acond = ConditionFlag.overflow;
+                ncond = ConditionFlag.noOverflow;
+                break;
+            case NOF:
+                acond = ConditionFlag.noOverflow;
+                ncond = ConditionFlag.overflow;
+                break;
             default:
                 throw Util.shouldNotReachHere();
         }
@@ -782,10 +790,11 @@
 
         if (!other.isConstant()) {
             // optimized version that does not require a branch
+            TTY.println("> emitting cmov");
             if (other.isRegister()) {
                 assert other.asRegister() != result.asRegister() : "other already overwritten by previous move";
                 if (other.kind.isInt()) {
-                    masm.cmovq(ncond, result.asRegister(), other.asRegister());
+                    masm.cmovl(ncond, result.asRegister(), other.asRegister());
                 } else {
                     masm.cmovq(ncond, result.asRegister(), other.asRegister());
                 }
--- a/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRGenerator.java	Mon Aug 01 13:56:56 2011 +0200
+++ b/graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRGenerator.java	Mon Aug 01 18:30:25 2011 +0200
@@ -71,12 +71,12 @@
     }
 
     @Override
-    protected boolean canInlineAsConstant(Value v) {
+    public boolean canInlineAsConstant(Value v) {
+        if (!v.isConstant()) {
+            return false;
+        }
         if (v.kind == CiKind.Long) {
-            if (v.isConstant() && NumUtil.isInt(v.asConstant().asLong())) {
-                return true;
-            }
-            return false;
+            return NumUtil.isInt(v.asConstant().asLong());
         }
         return v.kind != CiKind.Object || v.isNullConstant();
     }