Mercurial > hg > graal-compiler

changeset 22911:9aadd8e4e5aa

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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);