Mercurial > hg > graal-compiler
changeset 22911:9aadd8e4e5aa
Fix read elimination for raw read and write nodes.
author | Thomas Wuerthinger <thomas.wuerthinger@oracle.com> |
---|---|
date | Fri, 30 Oct 2015 20:55:32 +0100 |
parents | 2c244f95fcbf |
children | 3c00f45259b6 |
files | graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IntegerEqualsCanonicalizerTest.java graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/EarlyReadEliminationTest.java graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/PEAReadEliminationTest.java graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java |
diffstat | 5 files changed, 106 insertions(+), 104 deletions(-) [+] |
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IntegerEqualsCanonicalizerTest.java Thu Oct 08 17:24:58 2015 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/IntegerEqualsCanonicalizerTest.java Fri Oct 30 20:55:32 2015 +0100 @@ -51,6 +51,25 @@ return 0; } + @Test + public void testSubtractEqualsZeroLong() { + test("testSubtractEqualsZeroLongSnippet", "testSubtractEqualsZeroLongReference"); + } + + public static int testSubtractEqualsZeroLongReference(long a, long b) { + if (a == b) { + return 1; + } + return 0; + } + + public static int testSubtractEqualsZeroLongSnippet(long a, long b) { + if (a - b == 0) { + return 1; + } + return 0; + } + /** * Tests the canonicalization of (x >>> const) == 0 to x |test| (-1 << const). */
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/EarlyReadEliminationTest.java Thu Oct 08 17:24:58 2015 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/EarlyReadEliminationTest.java Fri Oct 30 20:55:32 2015 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -35,15 +35,16 @@ import com.oracle.graal.nodes.ValuePhiNode; import com.oracle.graal.nodes.java.LoadFieldNode; import com.oracle.graal.nodes.java.StoreFieldNode; +import com.oracle.graal.nodes.memory.ReadNode; +import com.oracle.graal.nodes.spi.LoweringTool; import com.oracle.graal.phases.common.CanonicalizerPhase; +import com.oracle.graal.phases.common.LoweringPhase; import com.oracle.graal.phases.common.inlining.InliningPhase; import com.oracle.graal.phases.tiers.HighTierContext; import com.oracle.graal.virtual.phases.ea.EarlyReadEliminationPhase; public class EarlyReadEliminationTest extends GraalCompilerTest { - protected StructuredGraph graph; - public static Object staticField; public static class TestObject { @@ -87,8 +88,15 @@ @Test public void testSimple() { - ValueNode result = getReturn("testSimpleSnippet").result(); - assertTrue(graph.getNodes().filter(LoadFieldNode.class).isEmpty()); + // Test without lowering. + ValueNode result = getReturn("testSimpleSnippet", false).result(); + assertTrue(result.graph().getNodes().filter(LoadFieldNode.class).isEmpty()); + assertTrue(result.isConstant()); + assertDeepEquals(2, result.asJavaConstant().asInt()); + + // Test with lowering. + result = getReturn("testSimpleSnippet", true).result(); + assertTrue(result.graph().getNodes().filter(ReadNode.class).isEmpty()); assertTrue(result.isConstant()); assertDeepEquals(2, result.asJavaConstant().asInt()); } @@ -103,7 +111,7 @@ @Test public void testSimpleConflict() { - ValueNode result = getReturn("testSimpleConflictSnippet").result(); + ValueNode result = getReturn("testSimpleConflictSnippet", false).result(); assertFalse(result.isConstant()); assertTrue(result instanceof LoadFieldNode); } @@ -116,9 +124,9 @@ @Test public void testParam() { - ValueNode result = getReturn("testParamSnippet").result(); - assertTrue(graph.getNodes().filter(LoadFieldNode.class).isEmpty()); - assertDeepEquals(graph.getParameter(1), result); + ValueNode result = getReturn("testParamSnippet", false).result(); + assertTrue(result.graph().getNodes().filter(LoadFieldNode.class).isEmpty()); + assertDeepEquals(result.graph().getParameter(1), result); } @SuppressWarnings("all") @@ -130,9 +138,9 @@ @Test public void testMaterialized() { - ValueNode result = getReturn("testMaterializedSnippet").result(); - assertTrue(graph.getNodes().filter(LoadFieldNode.class).isEmpty()); - assertDeepEquals(graph.getParameter(0), result); + ValueNode result = getReturn("testMaterializedSnippet", false).result(); + assertTrue(result.graph().getNodes().filter(LoadFieldNode.class).isEmpty()); + assertDeepEquals(result.graph().getParameter(0), result); } @SuppressWarnings("all") @@ -146,9 +154,15 @@ @Test public void testSimpleLoop() { - ValueNode result = getReturn("testSimpleLoopSnippet").result(); - assertTrue(graph.getNodes().filter(LoadFieldNode.class).isEmpty()); - assertDeepEquals(graph.getParameter(1), result); + // Test without lowering. + ValueNode result = getReturn("testSimpleLoopSnippet", false).result(); + assertTrue(result.graph().getNodes().filter(LoadFieldNode.class).isEmpty()); + assertDeepEquals(result.graph().getParameter(1), result); + + // Now test with lowering. + result = getReturn("testSimpleLoopSnippet", true).result(); + assertTrue(result.graph().getNodes().filter(ReadNode.class).isEmpty()); + assertDeepEquals(result.graph().getParameter(1), result); } @SuppressWarnings("all") @@ -164,8 +178,8 @@ @Test public void testBadLoop() { - ValueNode result = getReturn("testBadLoopSnippet").result(); - assertDeepEquals(0, graph.getNodes().filter(LoadFieldNode.class).count()); + ValueNode result = getReturn("testBadLoopSnippet", false).result(); + assertDeepEquals(0, result.graph().getNodes().filter(LoadFieldNode.class).count()); assertTrue(result instanceof ProxyNode); assertTrue(((ProxyNode) result).value() instanceof ValuePhiNode); } @@ -182,8 +196,8 @@ @Test public void testBadLoop2() { - ValueNode result = getReturn("testBadLoop2Snippet").result(); - assertDeepEquals(1, graph.getNodes().filter(LoadFieldNode.class).count()); + ValueNode result = getReturn("testBadLoop2Snippet", false).result(); + assertDeepEquals(1, result.graph().getNodes().filter(LoadFieldNode.class).count()); assertTrue(result instanceof LoadFieldNode); } @@ -199,7 +213,7 @@ @Test public void testPhi() { - processMethod("testPhiSnippet"); + StructuredGraph graph = processMethod("testPhiSnippet", false); assertTrue(graph.getNodes().filter(LoadFieldNode.class).isEmpty()); List<ReturnNode> returnNodes = graph.getNodes(ReturnNode.TYPE).snapshot(); assertDeepEquals(2, returnNodes.size()); @@ -217,7 +231,7 @@ @Test public void testSimpleStore() { - processMethod("testSimpleStoreSnippet"); + StructuredGraph graph = processMethod("testSimpleStoreSnippet", false); assertDeepEquals(1, graph.getNodes().filter(StoreFieldNode.class).count()); } @@ -235,20 +249,24 @@ @Test public void testValueProxy() { - processMethod("testValueProxySnippet"); + StructuredGraph graph = processMethod("testValueProxySnippet", false); assertDeepEquals(2, graph.getNodes().filter(LoadFieldNode.class).count()); } - ReturnNode getReturn(String snippet) { - processMethod(snippet); + ReturnNode getReturn(String snippet, boolean doLowering) { + StructuredGraph graph = processMethod(snippet, doLowering); assertDeepEquals(1, graph.getNodes(ReturnNode.TYPE).count()); return graph.getNodes(ReturnNode.TYPE).first(); } - protected void processMethod(String snippet) { - graph = parseEager(getResolvedJavaMethod(snippet), AllowAssumptions.NO); + protected StructuredGraph processMethod(String snippet, boolean doLowering) { + StructuredGraph graph = parseEager(getResolvedJavaMethod(snippet), AllowAssumptions.NO); HighTierContext context = getDefaultHighTierContext(); new InliningPhase(new CanonicalizerPhase()).apply(graph, context); + if (doLowering) { + new LoweringPhase(new CanonicalizerPhase(), LoweringTool.StandardLoweringStage.HIGH_TIER).apply(graph, context); + } new EarlyReadEliminationPhase(new CanonicalizerPhase()).apply(graph, context); + return graph; } }
--- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/PEAReadEliminationTest.java Thu Oct 08 17:24:58 2015 +0200 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/ea/PEAReadEliminationTest.java Fri Oct 30 20:55:32 2015 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,6 +26,7 @@ import sun.misc.Unsafe; +import com.oracle.graal.nodes.StructuredGraph; import com.oracle.graal.nodes.StructuredGraph.AllowAssumptions; import com.oracle.graal.nodes.extended.UnsafeLoadNode; import com.oracle.graal.nodes.java.LoadIndexedNode; @@ -49,7 +50,7 @@ @Test public void testIndexed1() { - processMethod("testIndexed1Snippet"); + StructuredGraph graph = processMethod("testIndexed1Snippet", false); assertDeepEquals(0, graph.getNodes().filter(LoadIndexedNode.class).count()); } @@ -69,7 +70,7 @@ @Test public void testIndexed2() { - processMethod("testIndexed2Snippet"); + StructuredGraph graph = processMethod("testIndexed2Snippet", false); assertDeepEquals(3, graph.getNodes().filter(LoadIndexedNode.class).count()); assertDeepEquals(7, graph.getNodes().filter(StoreIndexedNode.class).count()); } @@ -93,7 +94,7 @@ @Test public void testIndexed3() { - processMethod("testIndexed3Snippet"); + StructuredGraph graph = processMethod("testIndexed3Snippet", false); assertDeepEquals(3, graph.getNodes().filter(LoadIndexedNode.class).count()); } @@ -112,7 +113,7 @@ @Test public void testIndexed4() { - processMethod("testIndexed4Snippet"); + StructuredGraph graph = processMethod("testIndexed4Snippet", false); assertDeepEquals(3, graph.getNodes().filter(LoadIndexedNode.class).count()); } @@ -128,7 +129,7 @@ @Test public void testUnsafe1() { - processMethod("testUnsafe1Snippet"); + StructuredGraph graph = processMethod("testUnsafe1Snippet", false); assertDeepEquals(1, graph.getNodes().filter(UnsafeLoadNode.class).count()); } @@ -141,7 +142,7 @@ @Test public void testUnsafe2() { - processMethod("testUnsafe2Snippet"); + StructuredGraph graph = processMethod("testUnsafe2Snippet", false); assertDeepEquals(3, graph.getNodes().filter(UnsafeLoadNode.class).count()); } @@ -157,7 +158,7 @@ @Test public void testUnsafe3() { - processMethod("testUnsafe3Snippet"); + StructuredGraph graph = processMethod("testUnsafe3Snippet", false); assertDeepEquals(1, graph.getNodes().filter(UnsafeLoadNode.class).count()); } @@ -171,7 +172,7 @@ @Test public void testUnsafe4() { - processMethod("testUnsafe4Snippet"); + StructuredGraph graph = processMethod("testUnsafe4Snippet", false); assertDeepEquals(3, graph.getNodes().filter(UnsafeLoadNode.class).count()); } @@ -187,15 +188,16 @@ @Test public void testUnsafe5() { - processMethod("testUnsafe5Snippet"); + StructuredGraph graph = processMethod("testUnsafe5Snippet", false); assertDeepEquals(1, graph.getNodes().filter(UnsafeLoadNode.class).count()); } @Override - protected void processMethod(final String snippet) { - graph = parseEager(snippet, AllowAssumptions.NO); + protected StructuredGraph processMethod(final String snippet, boolean doLowering) { + StructuredGraph graph = parseEager(snippet, AllowAssumptions.NO); HighTierContext context = getDefaultHighTierContext(); new InliningPhase(new CanonicalizerPhase()).apply(graph, context); new PartialEscapePhase(false, true, new CanonicalizerPhase(), null).apply(graph, context); + return graph; } }
--- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java Thu Oct 08 17:24:58 2015 +0200 +++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationBlockState.java Fri Oct 30 20:55:32 2015 +0100 @@ -112,41 +112,6 @@ } } - static class ReadCacheEntry extends CacheEntry<ValueNode> { - - private final LocationIdentity location; - - public ReadCacheEntry(ValueNode object, ValueNode offset, LocationIdentity location) { - super(object, offset); - this.location = location; - } - - @Override - public CacheEntry<ValueNode> duplicateWithObject(ValueNode newObject) { - return new ReadCacheEntry(newObject, identity, location); - } - - @Override - public boolean conflicts(LocationIdentity other) { - return location.equals(other); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof ReadCacheEntry)) { - return false; - } - - ReadCacheEntry other = (ReadCacheEntry) obj; - return this.location.equals(other.location) && super.equals(other); - } - - @Override - public int hashCode() { - return location.hashCode() * 23 + super.hashCode(); - } - } - public ReadEliminationBlockState() { readCache = CollectionsFactory.newMap(); }
--- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java Thu Oct 08 17:24:58 2015 +0200 +++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/ReadEliminationClosure.java Fri Oct 30 20:55:32 2015 +0100 @@ -55,11 +55,9 @@ import com.oracle.graal.nodes.memory.MemoryCheckpoint; import com.oracle.graal.nodes.memory.ReadNode; import com.oracle.graal.nodes.memory.WriteNode; -import com.oracle.graal.nodes.memory.address.OffsetAddressNode; import com.oracle.graal.nodes.util.GraphUtil; import com.oracle.graal.virtual.phases.ea.ReadEliminationBlockState.CacheEntry; import com.oracle.graal.virtual.phases.ea.ReadEliminationBlockState.LoadCacheEntry; -import com.oracle.graal.virtual.phases.ea.ReadEliminationBlockState.ReadCacheEntry; import com.oracle.graal.virtual.phases.ea.ReadEliminationBlockState.UnsafeLoadCacheEntry; public class ReadEliminationClosure extends EffectsClosure<ReadEliminationBlockState> { @@ -106,46 +104,46 @@ } } else if (node instanceof ReadNode) { ReadNode read = (ReadNode) node; - if (read.getAddress() instanceof OffsetAddressNode) { - OffsetAddressNode address = (OffsetAddressNode) read.getAddress(); - if (address.getOffset().isConstant()) { - ValueNode object = GraphUtil.unproxify(address.getBase()); - ReadCacheEntry identifier = new ReadCacheEntry(object, address.getOffset(), read.getLocationIdentity()); - ValueNode cachedValue = state.getCacheEntry(identifier); - if (cachedValue != null && read.stamp().isCompatible(cachedValue.stamp())) { - // Anchor guard if it is not fixed and different from cachedValue's guard - if (read.getGuard() != null && !(read.getGuard() instanceof FixedNode)) { - if (!(cachedValue instanceof GuardedNode) || ((GuardedNode) cachedValue).getGuard() != read.getGuard()) { - effects.addFixedNodeBefore(new ValueAnchorNode((ValueNode) read.getGuard()), read); - } + if (read.getLocationIdentity().isSingle()) { + ValueNode object = GraphUtil.unproxify(read.getAddress()); + LoadCacheEntry identifier = new LoadCacheEntry(object, read.getLocationIdentity()); + ValueNode cachedValue = state.getCacheEntry(identifier); + if (cachedValue != null && read.stamp().isCompatible(cachedValue.stamp())) { + // Anchor guard if it is not fixed and different from cachedValue's guard such + // that it gets preserved. + if (read.getGuard() != null && !(read.getGuard() instanceof FixedNode)) { + if (!(cachedValue instanceof GuardedNode) || ((GuardedNode) cachedValue).getGuard() != read.getGuard()) { + effects.addFixedNodeBefore(new ValueAnchorNode((ValueNode) read.getGuard()), read); } } + effects.replaceAtUsages(read, cachedValue); + addScalarAlias(read, cachedValue); + deleted = true; + } else { + state.addCacheEntry(identifier, read); } } } else if (node instanceof WriteNode) { WriteNode write = (WriteNode) node; - if (write.getAddress() instanceof OffsetAddressNode) { - OffsetAddressNode address = (OffsetAddressNode) write.getAddress(); - if (address.getOffset().isConstant()) { - ValueNode object = GraphUtil.unproxify(address.getBase()); - ReadCacheEntry identifier = new ReadCacheEntry(object, address.getOffset(), write.getLocationIdentity()); - ValueNode cachedValue = state.getCacheEntry(identifier); + if (write.getLocationIdentity().isSingle()) { + ValueNode object = GraphUtil.unproxify(write.getAddress()); + LoadCacheEntry identifier = new LoadCacheEntry(object, write.getLocationIdentity()); + ValueNode cachedValue = state.getCacheEntry(identifier); - ValueNode value = getScalarAlias(write.value()); - if (GraphUtil.unproxify(value) == GraphUtil.unproxify(cachedValue)) { - effects.deleteNode(write); - deleted = true; - } - processIdentity(state, write.getLocationIdentity()); - state.addCacheEntry(identifier, value); - } else { - processIdentity(state, write.getLocationIdentity()); + ValueNode value = getScalarAlias(write.value()); + if (GraphUtil.unproxify(value) == GraphUtil.unproxify(cachedValue)) { + effects.deleteNode(write); + deleted = true; } + processIdentity(state, write.getLocationIdentity()); + state.addCacheEntry(identifier, value); + } else { + processIdentity(state, write.getLocationIdentity()); } } else if (node instanceof UnsafeAccessNode) { if (node instanceof UnsafeLoadNode) { UnsafeLoadNode load = (UnsafeLoadNode) node; - if (load.offset().isConstant() && !load.getLocationIdentity().equals(LocationIdentity.any())) { + if (load.getLocationIdentity().isSingle()) { ValueNode object = GraphUtil.unproxify(load.object()); UnsafeLoadCacheEntry identifier = new UnsafeLoadCacheEntry(object, load.offset(), load.getLocationIdentity()); ValueNode cachedValue = state.getCacheEntry(identifier); @@ -160,7 +158,7 @@ } else { assert node instanceof UnsafeStoreNode; UnsafeStoreNode write = (UnsafeStoreNode) node; - if (write.offset().isConstant() && !write.getLocationIdentity().equals(LocationIdentity.any())) { + if (write.getLocationIdentity().isSingle()) { ValueNode object = GraphUtil.unproxify(write.object()); UnsafeLoadCacheEntry identifier = new UnsafeLoadCacheEntry(object, write.offset(), write.getLocationIdentity()); ValueNode cachedValue = state.getCacheEntry(identifier);