Mercurial > hg > truffle
changeset 19951:a252927dfbfd
Fix an issue when the result of canonicalization is appended in the graph builder. Introduce GraphBuilderPhase#recursiveAppend.
| author | Thomas Wuerthinger <thomas.wuerthinger@oracle.com> |
|---|---|
| date | Wed, 18 Mar 2015 23:26:23 +0100 |
| parents | 5ee90d1bf6cd |
| children | a9145e7b14b4 |
| files | graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/GraphBuilderContext.java graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/StandardGraphBuilderPlugins.java |
| diffstat | 5 files changed, 65 insertions(+), 79 deletions(-) [+] |
line wrap: on
line diff
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Wed Mar 18 21:36:35 2015 +0100 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Wed Mar 18 23:26:23 2015 +0100 @@ -847,12 +847,19 @@ protected void afterClone(@SuppressWarnings("unused") Node other) { } + public boolean verifyInputs() { + for (Node input : inputs()) { + assertFalse(input.isDeleted(), "input was deleted"); + assertTrue(input.isAlive(), "input is not alive yet, i.e., it was not yet added to the graph"); + assertTrue(input.usages().contains(this), "missing usage in input %s", input); + } + return true; + } + public boolean verify() { assertTrue(isAlive(), "cannot verify inactive nodes (id=%d)", id); assertTrue(graph() != null, "null graph"); - for (Node input : inputs()) { - assertTrue(input.usages().contains(this), "missing usage in input %s", input); - } + verifyInputs(); for (Node successor : successors()) { assertTrue(successor.predecessor() == this, "missing predecessor in %s (actual: %s)", successor, successor.predecessor()); assertTrue(successor.graph() == graph(), "mismatching graph in successor %s", successor); @@ -864,7 +871,7 @@ while (iterator.hasNext()) { Position pos = iterator.nextPosition(); if (pos.get(usage) == this && pos.getInputType() != InputType.Unchecked) { - assert isAllowedUsageType(pos.getInputType()) : "invalid input of type " + pos.getInputType() + " from " + usage + " to " + this + " (" + pos.getName() + ")"; + assertTrue(isAllowedUsageType(pos.getInputType()), "invalid input of type " + pos.getInputType() + " from " + usage + " to " + this + " (" + pos.getName() + ")"); } } }
--- a/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/GraphBuilderContext.java Wed Mar 18 21:36:35 2015 +0100 +++ b/graal/com.oracle.graal.graphbuilderconf/src/com/oracle/graal/graphbuilderconf/GraphBuilderContext.java Wed Mar 18 23:26:23 2015 +0100 @@ -26,7 +26,6 @@ import com.oracle.graal.api.meta.*; import com.oracle.graal.api.replacements.*; import com.oracle.graal.nodes.*; -import com.oracle.graal.nodes.calc.*; import com.oracle.graal.nodes.spi.*; /** @@ -60,15 +59,15 @@ boolean isIntrinsic(); } - <T extends ControlSinkNode> T append(T fixed); - - <T extends ControlSplitNode> T append(T fixed); + <T extends ValueNode> T append(T value); - <T extends FixedWithNextNode> T append(T fixed); - - <T extends FloatingNode> T append(T value); - - <T extends ValueNode> T append(T value); + /** + * Adds the given floating node to the graph and also adds recursively all referenced inputs. + * + * @param value the floating node to be added to the graph + * @return either the node added or an equivalent node + */ + <T extends ValueNode> T recursiveAppend(T value); StampProvider getStampProvider();
--- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java Wed Mar 18 21:36:35 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/AbstractBytecodeParser.java Wed Mar 18 23:26:23 2015 +0100 @@ -1033,7 +1033,7 @@ protected abstract ValueNode appendConstant(JavaConstant constant); - protected abstract ValueNode append(ValueNode v); + protected abstract <T extends ValueNode> T append(T v); protected boolean isNeverExecutedCode(double probability) { return probability == 0 && optimisticOpts.removeNeverExecutedCode();
--- a/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Wed Mar 18 21:36:35 2015 +0100 +++ b/graal/com.oracle.graal.java/src/com/oracle/graal/java/GraphBuilderPhase.java Wed Mar 18 23:26:23 2015 +0100 @@ -1444,61 +1444,41 @@ return ConstantNode.forConstant(constant, metaAccess, currentGraph); } - @SuppressWarnings("unchecked") @Override - public ValueNode append(ValueNode v) { + public <T extends ValueNode> T append(T v) { if (v.graph() != null) { - // This node was already appended to the graph. return v; } - if (v instanceof ControlSinkNode) { - return append((ControlSinkNode) v); - } - if (v instanceof ControlSplitNode) { - return append((ControlSplitNode) v); - } - if (v instanceof FixedWithNextNode) { - return append((FixedWithNextNode) v); + T added = currentGraph.addOrUnique(v); + if (added == v) { + updateLastInstruction(v); } - if (v instanceof FloatingNode) { - return append((FloatingNode) v); - } - throw GraalInternalError.shouldNotReachHere("Can not append Node of type: " + v.getClass().getName()); - } - - public <T extends ControlSinkNode> T append(T fixed) { - assert !fixed.isAlive() && !fixed.isDeleted() : "instruction should not have been appended yet"; - assert lastInstr.next() == null : "cannot append instruction to instruction which isn't end (" + lastInstr + "->" + lastInstr.next() + ")"; - T added = currentGraph.add(fixed); - lastInstr.setNext(added); - lastInstr = null; return added; } - public <T extends ControlSplitNode> T append(T fixed) { - assert !fixed.isAlive() && !fixed.isDeleted() : "instruction should not have been appended yet"; - assert lastInstr.next() == null : "cannot append instruction to instruction which isn't end (" + lastInstr + "->" + lastInstr.next() + ")"; - T added = currentGraph.add(fixed); - lastInstr.setNext(added); - lastInstr = null; + public <T extends ValueNode> T recursiveAppend(T v) { + if (v.graph() != null) { + return v; + } + T added = currentGraph.addOrUniqueWithInputs(v); + if (added == v) { + updateLastInstruction(v); + } return added; } - public <T extends FixedWithNextNode> T append(T fixed) { - assert !fixed.isAlive() && !fixed.isDeleted() : "instruction should not have been appended yet"; - assert lastInstr.next() == null : "cannot append instruction to instruction which isn't end (" + lastInstr + "->" + lastInstr.next() + ")"; - T added = currentGraph.add(fixed); - lastInstr.setNext(added); - lastInstr = added; - return added; - } - - public <T extends FloatingNode> T append(T v) { - if (v.graph() != null) { - return v; + private <T extends ValueNode> void updateLastInstruction(T v) { + if (v instanceof FixedNode) { + FixedNode fixedNode = (FixedNode) v; + lastInstr.setNext(fixedNode); + if (fixedNode instanceof FixedWithNextNode) { + FixedWithNextNode fixedWithNextNode = (FixedWithNextNode) fixedNode; + assert fixedWithNextNode.next() == null : "cannot append instruction to instruction which isn't end"; + lastInstr = fixedWithNextNode; + } else { + lastInstr = null; + } } - T added = currentGraph.unique(v); - return added; } private Target checkLoopExit(FixedNode target, BciBlock targetBlock, HIRFrameStateBuilder state) {
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/StandardGraphBuilderPlugins.java Wed Mar 18 21:36:35 2015 +0100 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/StandardGraphBuilderPlugins.java Wed Mar 18 23:26:23 2015 +0100 @@ -151,25 +151,25 @@ Registration r = new Registration(plugins, declaringClass); r.register1("reverseBytes", type, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(kind, b.append(new ReverseBytesNode(value).canonical(null, value))); + b.push(kind, b.recursiveAppend(new ReverseBytesNode(value).canonical(null, value))); return true; } }); r.register1("bitCount", type, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Int, b.append(new BitCountNode(value).canonical(null, value))); + b.push(Kind.Int, b.recursiveAppend(new BitCountNode(value).canonical(null, value))); return true; } }); r.register2("divideUnsigned", type, type, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode dividend, ValueNode divisor) { - b.push(kind, b.append(new UnsignedDivNode(dividend, divisor).canonical(null, dividend, divisor))); + b.push(kind, b.recursiveAppend(new UnsignedDivNode(dividend, divisor).canonical(null, dividend, divisor))); return true; } }); r.register2("remainderUnsigned", type, type, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode dividend, ValueNode divisor) { - b.push(kind, b.append(new UnsignedDivNode(dividend, divisor).canonical(null, dividend, divisor))); + b.push(kind, b.recursiveAppend(new UnsignedDivNode(dividend, divisor).canonical(null, dividend, divisor))); return true; } }); @@ -183,7 +183,7 @@ ReverseBytesNode reverse = b.append(new ReverseBytesNode(value)); RightShiftNode rightShift = b.append(new RightShiftNode(reverse, b.append(ConstantNode.forInt(16)))); ZeroExtendNode charCast = b.append(new ZeroExtendNode(b.append(new NarrowNode(rightShift, 16)), 32)); - b.push(Kind.Char.getStackKind(), b.append(charCast.canonical(null, value))); + b.push(Kind.Char.getStackKind(), b.recursiveAppend(charCast.canonical(null, value))); return true; } }); @@ -197,7 +197,7 @@ ReverseBytesNode reverse = b.append(new ReverseBytesNode(value)); RightShiftNode rightShift = b.append(new RightShiftNode(reverse, b.append(ConstantNode.forInt(16)))); SignExtendNode charCast = b.append(new SignExtendNode(b.append(new NarrowNode(rightShift, 16)), 32)); - b.push(Kind.Short.getStackKind(), b.append(charCast.canonical(null, value))); + b.push(Kind.Short.getStackKind(), b.recursiveAppend(charCast.canonical(null, value))); return true; } }); @@ -207,13 +207,13 @@ Registration r = new Registration(plugins, Float.class); r.register1("floatToRawIntBits", float.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Int, b.append(new ReinterpretNode(Kind.Int, value).canonical(null, value))); + b.push(Kind.Int, b.recursiveAppend(new ReinterpretNode(Kind.Int, value).canonical(null, value))); return true; } }); r.register1("intBitsToFloat", int.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Float, b.append(new ReinterpretNode(Kind.Float, value).canonical(null, value))); + b.push(Kind.Float, b.recursiveAppend(new ReinterpretNode(Kind.Float, value).canonical(null, value))); return true; } }); @@ -223,13 +223,13 @@ Registration r = new Registration(plugins, Double.class); r.register1("doubleToRawLongBits", double.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Long, b.append(new ReinterpretNode(Kind.Long, value).canonical(null, value))); + b.push(Kind.Long, b.recursiveAppend(new ReinterpretNode(Kind.Long, value).canonical(null, value))); return true; } }); r.register1("longBitsToDouble", long.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Double, b.append(new ReinterpretNode(Kind.Double, value).canonical(null, value))); + b.push(Kind.Double, b.recursiveAppend(new ReinterpretNode(Kind.Double, value).canonical(null, value))); return true; } }); @@ -239,32 +239,32 @@ Registration r = new Registration(plugins, Math.class); r.register1("abs", Float.TYPE, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Float, b.append(new AbsNode(value).canonical(null, value))); + b.push(Kind.Float, b.recursiveAppend(new AbsNode(value).canonical(null, value))); return true; } }); r.register1("abs", Double.TYPE, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Double, b.append(new AbsNode(value).canonical(null, value))); + b.push(Kind.Double, b.recursiveAppend(new AbsNode(value).canonical(null, value))); return true; } }); r.register1("sqrt", Double.TYPE, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Double, b.append(new SqrtNode(value).canonical(null, value))); + b.push(Kind.Double, b.recursiveAppend(new SqrtNode(value).canonical(null, value))); return true; } }); if (getAndSetEnabled(arch)) { r.register1("log", Double.TYPE, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Double, b.append(MathIntrinsicNode.create(value, LOG))); + b.push(Kind.Double, b.recursiveAppend(MathIntrinsicNode.create(value, LOG))); return true; } }); r.register1("log10", Double.TYPE, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode value) { - b.push(Kind.Double, b.append(MathIntrinsicNode.create(value, LOG10))); + b.push(Kind.Double, b.recursiveAppend(MathIntrinsicNode.create(value, LOG10))); return true; } }); @@ -313,25 +313,25 @@ r.register2("belowOrEqual", long.class, long.class, new UnsignedMathPlugin(Condition.BE)); r.register2("divide", int.class, int.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode x, ValueNode y) { - b.push(Kind.Int, b.append(new UnsignedDivNode(x, y).canonical(null, x, y))); + b.push(Kind.Int, b.recursiveAppend(new UnsignedDivNode(x, y).canonical(null, x, y))); return true; } }); r.register2("divide", long.class, long.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode x, ValueNode y) { - b.push(Kind.Long, b.append(new UnsignedDivNode(x, y).canonical(null, x, y))); + b.push(Kind.Long, b.recursiveAppend(new UnsignedDivNode(x, y).canonical(null, x, y))); return true; } }); r.register2("remainder", int.class, int.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode x, ValueNode y) { - b.push(Kind.Int, b.append(new UnsignedRemNode(x, y).canonical(null, x, y))); + b.push(Kind.Int, b.recursiveAppend(new UnsignedRemNode(x, y).canonical(null, x, y))); return true; } }); r.register2("remainder", long.class, long.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode x, ValueNode y) { - b.push(Kind.Long, b.append(new UnsignedRemNode(x, y).canonical(null, x, y))); + b.push(Kind.Long, b.recursiveAppend(new UnsignedRemNode(x, y).canonical(null, x, y))); return true; } }); @@ -366,14 +366,14 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode type, ValueNode object) { ValueNode nullCheckedType = nullCheckedValue(b, type); LogicNode condition = b.append(InstanceOfDynamicNode.create(b.getConstantReflection(), nullCheckedType, object)); - b.push(Kind.Boolean.getStackKind(), b.append(new ConditionalNode(condition).canonical(null))); + b.push(Kind.Boolean.getStackKind(), b.recursiveAppend(new ConditionalNode(condition).canonical(null))); return true; } }); r.register2("isAssignableFrom", Receiver.class, Class.class, new InvocationPlugin() { public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, ValueNode type, ValueNode otherType) { - ClassIsAssignableFromNode condition = b.append(new ClassIsAssignableFromNode(nullCheckedValue(b, type), otherType)); - b.push(Kind.Boolean.getStackKind(), b.append(new ConditionalNode(condition).canonical(null))); + ClassIsAssignableFromNode condition = b.recursiveAppend(new ClassIsAssignableFromNode(nullCheckedValue(b, type), otherType)); + b.push(Kind.Boolean.getStackKind(), b.recursiveAppend(new ConditionalNode(condition).canonical(null))); return true; } });