changeset 5414:d3dec1a05a80

fix bug in FloatingReadPhase that leads to wrong ordering of phi inputs on memory phis
author Lukas Stadler <lukas.stadler@jku.at>
date Tue, 22 May 2012 11:11:48 +0200
parents 098c5eba749d
children 44c378aa4c47 068cc464e0cf
files graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/FloatingReadPhase.java graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/PhiNode.java
diffstat 2 files changed, 19 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/FloatingReadPhase.java	Wed May 16 13:24:39 2012 +0200
+++ b/graal/com.oracle.graal.compiler/src/com/oracle/graal/compiler/phases/FloatingReadPhase.java	Tue May 22 11:11:48 2012 +0200
@@ -50,7 +50,7 @@
             map.putAll(other.map);
         }
 
-        public void mergeLoopEntryWith(MemoryMap otherMemoryMap, LoopBeginNode begin) {
+        public void mergeLoopEntryWith(MemoryMap otherMemoryMap, LoopBeginNode begin, EndNode pred) {
             for (Object keyInOther : otherMemoryMap.map.keySet()) {
                 assert loopEntryMap.containsKey(keyInOther) || map.get(keyInOther) == otherMemoryMap.map.get(keyInOther) : keyInOther + ", " + map.get(keyInOther) + " vs " + otherMemoryMap.map.get(keyInOther) + " " + begin;
             }
@@ -65,7 +65,10 @@
                     other = otherMemoryMap.map.get(LocationNode.ANY_LOCATION);
                 }
 
-                phiNode.addInput((ValueNode) other);
+                // this explicitly honors the phi input index, since the iteration order will not always adhere to the end index ordering.
+                // TODO(ls) check for instances of this problem in other places.
+                int index = begin.phiPredecessorIndex(pred);
+                phiNode.initializeValueAt(index, (ValueNode) other);
             }
         }
 
@@ -114,6 +117,7 @@
                 assert phi.valueCount() <= phi.merge().forwardEndCount() : phi.merge();
             } else {
                 PhiNode phi = m.graph().unique(new PhiNode(CiKind.Illegal, m, PhiType.Memory));
+                // TODO(ls) how does this work? add documentation ...
                 for (int i = 0; i < mergeOperationCount + 1; ++i) {
                     phi.addInput((ValueNode) original);
                 }
@@ -293,7 +297,7 @@
             MemoryMap memoryMap = memoryMaps[beginBlock.getId()];
             assert memoryMap != null;
             assert memoryMap.getLoopEntryMap() != null;
-            memoryMap.mergeLoopEntryWith(map, begin);
+            memoryMap.mergeLoopEntryWith(map, begin, (EndNode) b.getEndNode());
         }
     }
 
--- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/PhiNode.java	Wed May 16 13:24:39 2012 +0200
+++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/PhiNode.java	Tue May 22 11:11:48 2012 +0200
@@ -93,6 +93,18 @@
         return values.get(i);
     }
 
+    /**
+     * Sets the value at the given index and makes sure that the values list is large enough.
+     * @param i the index at which to set the value
+     * @param x the new phi input value for the given location
+     */
+    public void initializeValueAt(int i, ValueNode x) {
+        while (values().size() <= i) {
+            values.add(null);
+        }
+        values.set(i, x);
+    }
+
     public void setValueAt(int i, ValueNode x) {
         values.set(i, x);
     }