# HG changeset patch # User Gilles Duboscq # Date 1312216225 -7200 # Node ID a64b615ba63092f6e95cc716643b461364459704 # Parent b9f76db5ac535e3f122b89e7d92132d7f5a0c4a1 WIP : convert Conditional (IfOp) to use the new BooleanNode infrastructure, Canonicalize some phi to Conditional, some Conditional to Materialize, remove If useless if nodes diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/gen/LIRGenerator.java --- 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); diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Condition.java --- 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()); } diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Conditional.java --- 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 lookup(Class 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); + } + } + }; } diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/If.java --- 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; } }; diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/NegateBooleanNode.java --- 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; } diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/Phi.java --- 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()); } }; } diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/ir/ValueVisitor.java --- 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); diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRAssembler.java --- 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()); } diff -r b9f76db5ac53 -r a64b615ba630 graal/com.oracle.max.graal.compiler/src/com/oracle/max/graal/compiler/target/amd64/AMD64LIRGenerator.java --- 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(); }