changeset 16230:5f72421928e0

converted MemoryMap to an interface and provided new MemoryMapNode implementation that is also a Node allowing the map and it's node entries to survive DeadCodeElimination by normal graph reachability rules
author Doug Simon <doug.simon@oracle.com>
date Wed, 25 Jun 2014 16:56:45 +0200
parents d837c02aba58
children a47528fb2ea0
files graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/MemoryMap.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/MemoryMapNode.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ReturnNode.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java
diffstat 5 files changed, 119 insertions(+), 72 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/MemoryMap.java	Wed Jun 25 16:53:09 2014 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/MemoryMap.java	Wed Jun 25 16:56:45 2014 +0200
@@ -25,21 +25,21 @@
 import java.util.*;
 
 import com.oracle.graal.api.meta.*;
-import com.oracle.graal.compiler.common.type.*;
-import com.oracle.graal.graph.*;
-import com.oracle.graal.nodes.calc.*;
 import com.oracle.graal.nodes.extended.*;
 
-@NodeInfo(allowedUsageTypes = {InputType.Extension})
-public abstract class MemoryMap extends FloatingNode {
-
-    public MemoryMap() {
-        super(StampFactory.forVoid());
-    }
+/**
+ * Maps a {@linkplain LocationIdentity location} to the last node that (potentially) wrote to the
+ * location.
+ */
+public interface MemoryMap {
 
-    public abstract MemoryNode getLastLocationAccess(LocationIdentity locationIdentity);
+    /**
+     * Gets the last node that that (potentially) wrote to {@code locationIdentity}.
+     */
+    MemoryNode getLastLocationAccess(LocationIdentity locationIdentity);
 
-    public abstract Set<LocationIdentity> getLocations();
-
-    public abstract boolean replaceLastLocationAccess(MemoryNode oldNode, MemoryNode newNode);
+    /**
+     * Gets the location identities in the domain of this map.
+     */
+    Collection<LocationIdentity> getLocations();
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/MemoryMapNode.java	Wed Jun 25 16:56:45 2014 +0200
@@ -0,0 +1,76 @@
+package com.oracle.graal.nodes;
+
+import static com.oracle.graal.api.meta.LocationIdentity.*;
+
+import java.util.*;
+
+import com.oracle.graal.api.meta.*;
+import com.oracle.graal.compiler.common.type.*;
+import com.oracle.graal.graph.*;
+import com.oracle.graal.nodes.calc.*;
+import com.oracle.graal.nodes.extended.*;
+import com.oracle.graal.nodes.spi.*;
+
+@NodeInfo(allowedUsageTypes = {InputType.Extension})
+public class MemoryMapNode extends FloatingNode implements MemoryMap, LIRLowerable {
+
+    private final List<LocationIdentity> locationIdentities;
+    @Input(InputType.Memory) private final NodeInputList<ValueNode> nodes;
+
+    private boolean checkOrder(Map<LocationIdentity, MemoryNode> mmap) {
+        for (int i = 0; i < locationIdentities.size(); i++) {
+            LocationIdentity locationIdentity = locationIdentities.get(i);
+            ValueNode n = nodes.get(i);
+            assertTrue(mmap.get(locationIdentity) == n, "iteration order of keys differs from values in input map");
+        }
+        return true;
+    }
+
+    public MemoryMapNode(Map<LocationIdentity, MemoryNode> mmap) {
+        super(StampFactory.forVoid());
+        locationIdentities = new ArrayList<>(mmap.keySet());
+        nodes = new NodeInputList<>(this, mmap.values());
+        assert checkOrder(mmap);
+    }
+
+    public boolean isEmpty() {
+        if (locationIdentities.isEmpty()) {
+            return true;
+        }
+        if (locationIdentities.size() == 1) {
+            if (nodes.get(0) instanceof StartNode) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public MemoryNode getLastLocationAccess(LocationIdentity locationIdentity) {
+        if (locationIdentity == FINAL_LOCATION) {
+            return null;
+        } else {
+            int index = locationIdentities.indexOf(locationIdentity);
+            if (index == -1) {
+                index = locationIdentities.indexOf(ANY_LOCATION);
+            }
+            assert index != -1;
+            return (MemoryNode) nodes.get(index);
+        }
+    }
+
+    public Collection<LocationIdentity> getLocations() {
+        return locationIdentities;
+    }
+
+    public Map<LocationIdentity, MemoryNode> toMap() {
+        HashMap<LocationIdentity, MemoryNode> res = new HashMap<>(locationIdentities.size());
+        for (int i = 0; i < nodes.size(); i++) {
+            res.put(locationIdentities.get(i), (MemoryNode) nodes.get(i));
+        }
+        return res;
+    }
+
+    public void generate(NodeLIRBuilderTool generator) {
+        // nothing to do...
+    }
+}
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ReturnNode.java	Wed Jun 25 16:53:09 2014 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/ReturnNode.java	Wed Jun 25 16:56:45 2014 +0200
@@ -29,7 +29,7 @@
 public final class ReturnNode extends ControlSinkNode implements LIRLowerable, IterableNodeType {
 
     @Input private ValueNode result;
-    @Input(InputType.Extension) private MemoryMap memoryMap;
+    @Input(InputType.Extension) private MemoryMapNode memoryMap;
 
     public ValueNode result() {
         return result;
@@ -45,7 +45,7 @@
         this(result, null);
     }
 
-    public ReturnNode(ValueNode result, MemoryMap memoryMap) {
+    public ReturnNode(ValueNode result, MemoryMapNode memoryMap) {
         super(StampFactory.forVoid());
         this.result = result;
         this.memoryMap = memoryMap;
@@ -65,12 +65,12 @@
         }
     }
 
-    public void setMemoryMap(MemoryMap memoryMap) {
+    public void setMemoryMap(MemoryMapNode memoryMap) {
         updateUsages(this.memoryMap, memoryMap);
         this.memoryMap = memoryMap;
     }
 
-    public MemoryMap getMemoryMap() {
+    public MemoryMapNode getMemoryMap() {
         return memoryMap;
     }
 }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java	Wed Jun 25 16:53:09 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/FloatingReadPhase.java	Wed Jun 25 16:56:45 2014 +0200
@@ -42,7 +42,7 @@
         CREATE_FLOATING_READS
     }
 
-    public static class MemoryMapImpl extends MemoryMap {
+    public static class MemoryMapImpl implements MemoryMap {
 
         private final Map<LocationIdentity, MemoryNode> lastMemorySnapshot;
 
@@ -74,33 +74,13 @@
             }
         }
 
-        public boolean isEmpty() {
-            if (lastMemorySnapshot.size() == 0) {
-                return true;
-            }
-            if (lastMemorySnapshot.size() == 1) {
-                if (lastMemorySnapshot.get(ANY_LOCATION) instanceof StartNode) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         @Override
-        public Set<LocationIdentity> getLocations() {
+        public Collection<LocationIdentity> getLocations() {
             return lastMemorySnapshot.keySet();
         }
 
-        @Override
-        public boolean replaceLastLocationAccess(MemoryNode oldNode, MemoryNode newNode) {
-            boolean replaced = false;
-            for (Map.Entry<LocationIdentity, MemoryNode> entry : lastMemorySnapshot.entrySet()) {
-                if (entry.getValue() == oldNode) {
-                    entry.setValue(newNode);
-                    replaced = true;
-                }
-            }
-            return replaced;
+        public Map<LocationIdentity, MemoryNode> getMap() {
+            return lastMemorySnapshot;
         }
     }
 
@@ -243,7 +223,7 @@
             assert MemoryCheckpoint.TypeAssertion.correctType(node) : node;
 
             if (execmode == ExecutionMode.ANALYSIS_ONLY && node instanceof ReturnNode) {
-                ((ReturnNode) node).setMemoryMap(node.graph().unique(new MemoryMapImpl(state)));
+                ((ReturnNode) node).setMemoryMap(node.graph().unique(new MemoryMapNode(state.lastMemorySnapshot)));
             }
             return state;
         }
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Wed Jun 25 16:53:09 2014 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Wed Jun 25 16:56:45 2014 +0200
@@ -35,6 +35,7 @@
 import java.util.*;
 import java.util.concurrent.*;
 import java.util.concurrent.atomic.*;
+import java.util.stream.*;
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.api.meta.*;
@@ -677,38 +678,33 @@
 
         Debug.dump(snippet, "SnippetTemplate after fixing memory anchoring");
 
-        List<ReturnNode> returnNodes = new ArrayList<>(4);
-        List<MemoryMap> memMaps = new ArrayList<>(4);
         StartNode entryPointNode = snippet.start();
-        boolean anchorUsed = false;
-        for (ReturnNode retNode : snippet.getNodes(ReturnNode.class)) {
-            MemoryMap memMap = retNode.getMemoryMap();
-            anchorUsed |= memMap.replaceLastLocationAccess(snippetCopy.start(), memoryAnchor);
-            memMaps.add(memMap);
-            retNode.setMemoryMap(null);
-            returnNodes.add(retNode);
-            if (memMap.usages().isEmpty()) {
-                memMap.safeDelete();
-            }
-        }
-        if (memoryAnchor.usages().isEmpty() && !anchorUsed) {
+        if (memoryAnchor.usages().isEmpty()) {
             memoryAnchor.safeDelete();
         } else {
             snippetCopy.addAfterFixed(snippetCopy.start(), memoryAnchor);
         }
-        assert snippet.getNodes().filter(MemoryMap.class).isEmpty();
+        List<ReturnNode> returnNodes = snippet.getNodes(ReturnNode.class).snapshot();
         if (returnNodes.isEmpty()) {
             this.returnNode = null;
             this.memoryMap = null;
         } else if (returnNodes.size() == 1) {
             this.returnNode = returnNodes.get(0);
-            this.memoryMap = memMaps.get(0);
+            this.memoryMap = returnNode.getMemoryMap();
         } else {
             MergeNode merge = snippet.add(new MergeNode());
+            List<MemoryMapNode> memMaps = returnNodes.stream().map(n -> n.getMemoryMap()).collect(Collectors.toList());
             ValueNode returnValue = InliningUtil.mergeReturns(merge, returnNodes, null);
             this.returnNode = snippet.add(new ReturnNode(returnValue));
-            this.memoryMap = FloatingReadPhase.mergeMemoryMaps(merge, memMaps);
+            MemoryMapImpl mmap = FloatingReadPhase.mergeMemoryMaps(merge, memMaps);
+            this.memoryMap = snippet.unique(new MemoryMapNode(mmap.getMap()));
             merge.setNext(this.returnNode);
+
+            for (MemoryMapNode mm : memMaps) {
+                if (mm.isAlive()) {
+                    mm.safeDelete();
+                }
+            }
         }
 
         this.sideEffectNodes = curSideEffectNodes;
@@ -798,9 +794,9 @@
     private final ArrayList<Node> nodes;
 
     /**
-     * map of killing locations to memory checkpoints (nodes).
+     * Map of killing locations to memory checkpoints (nodes).
      */
-    private final MemoryMap memoryMap;
+    private final MemoryMapNode memoryMap;
 
     /**
      * Times instantiations of this template.
@@ -961,12 +957,12 @@
             // no floating reads yet, ignore locations created while lowering
             return true;
         }
-        if (memoryMap == null || ((MemoryMapImpl) memoryMap).isEmpty()) {
-            // there're no kills in the snippet graph
+        if (memoryMap == null || memoryMap.isEmpty()) {
+            // there are no kills in the snippet graph
             return true;
         }
 
-        Set<LocationIdentity> kills = new HashSet<>(((MemoryMapImpl) memoryMap).getLocations());
+        Set<LocationIdentity> kills = new HashSet<>(memoryMap.getLocations());
 
         if (replacee instanceof MemoryCheckpoint.Single) {
             // check if some node in snippet graph also kills the same location
@@ -1010,10 +1006,10 @@
         return true;
     }
 
-    private class DuplicateMapper extends MemoryMap {
+    private class DuplicateMapper implements MemoryMap {
 
         private final Map<Node, Node> duplicates;
-        @Input private StartNode replaceeStart;
+        private StartNode replaceeStart;
 
         public DuplicateMapper(Map<Node, Node> duplicates, StartNode replaceeStart) {
             this.duplicates = duplicates;
@@ -1033,14 +1029,9 @@
         }
 
         @Override
-        public Set<LocationIdentity> getLocations() {
+        public Collection<LocationIdentity> getLocations() {
             return memoryMap.getLocations();
         }
-
-        @Override
-        public boolean replaceLastLocationAccess(MemoryNode oldNode, MemoryNode newNode) {
-            throw GraalInternalError.shouldNotReachHere();
-        }
     }
 
     /**