changeset 20836:aff67777d2ea

Better verification in assertSnippetKills.
author Roland Schatz <roland.schatz@oracle.com>
date Thu, 09 Apr 2015 15:04:09 +0200
parents 220ecaa0cc9b
children 5e9dc1535b62
files graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/NewObjectSnippets.java graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java
diffstat 2 files changed, 21 insertions(+), 19 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/NewObjectSnippets.java	Thu Apr 09 16:13:32 2015 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/replacements/NewObjectSnippets.java	Thu Apr 09 15:04:09 2015 +0200
@@ -407,7 +407,7 @@
                         TLAB_END_LOCATION);
         private final SnippetInfo allocateInstanceDynamic = snippet(NewObjectSnippets.class, "allocateInstanceDynamic", INIT_LOCATION, MARK_WORD_LOCATION, HUB_WRITE_LOCATION, TLAB_TOP_LOCATION,
                         TLAB_END_LOCATION);
-        private final SnippetInfo newmultiarray = snippet(NewObjectSnippets.class, "newmultiarray", INIT_LOCATION);
+        private final SnippetInfo newmultiarray = snippet(NewObjectSnippets.class, "newmultiarray", INIT_LOCATION, TLAB_TOP_LOCATION, TLAB_END_LOCATION);
         private final SnippetInfo verifyHeap = snippet(NewObjectSnippets.class, "verifyHeap");
 
         public Templates(HotSpotProviders providers, TargetDescription target) {
--- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Thu Apr 09 16:13:32 2015 +0200
+++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/SnippetTemplate.java	Thu Apr 09 15:04:09 2015 +0200
@@ -1003,14 +1003,14 @@
             LocationIdentity locationIdentity = ((MemoryCheckpoint.Single) replacee).getLocationIdentity();
             if (locationIdentity.isAny()) {
                 assert !(memoryMap.getLastLocationAccess(any()) instanceof MemoryAnchorNode) : replacee + " kills ANY_LOCATION, but snippet does not";
+                // if the replacee kills ANY_LOCATION, the snippet can kill arbitrary locations
+                return true;
             }
             assert kills.contains(locationIdentity) : replacee + " kills " + locationIdentity + ", but snippet doesn't contain a kill to this location";
-            return true;
+            kills.remove(locationIdentity);
         }
         assert !(replacee instanceof MemoryCheckpoint.Multi) : replacee + " multi not supported (yet)";
 
-        Debug.log("WARNING: %s is not a MemoryCheckpoint, but the snippet graph contains kills (%s). You might want %s to be a MemoryCheckpoint", replacee, kills, replacee);
-
         // remove ANY_LOCATION if it's just a kill by the start node
         if (memoryMap.getLastLocationAccess(any()) instanceof MemoryAnchorNode) {
             kills.remove(any());
@@ -1019,24 +1019,26 @@
         // node can only lower to a ANY_LOCATION kill if the replacee also kills ANY_LOCATION
         assert !kills.contains(any()) : "snippet graph contains a kill to ANY_LOCATION, but replacee (" + replacee + ") doesn't kill ANY_LOCATION.  kills: " + kills;
 
+        if (SnippetCounters.getValue()) {
+            /*
+             * accesses to snippet counters are artificially introduced and violate the memory
+             * semantics.
+             */
+            kills.remove(SnippetCounter.SNIPPET_COUNTER_LOCATION);
+        }
+
         /*
-         * kills to other locations than ANY_LOCATION can be still inserted if there aren't any
-         * floating reads accessing this locations. Example: In HotSpot, InstanceOfNode is lowered
-         * to a snippet containing a write to SECONDARY_SUPER_CACHE_LOCATION. This is runtime
-         * specific, so the runtime independent InstanceOfNode can not kill this location. However,
-         * if no FloatingReadNode is reading from this location, the kill to this location is fine.
+         * Kills to private locations are safe, since there can be no floating read to these
+         * locations except reads that are introduced by the snippet itself or related snippets in
+         * the same lowering round. These reads are anchored to a MemoryAnchor at the beginning of
+         * their snippet, so they can not float above a kill in another instance of the same
+         * snippet.
          */
-        for (FloatingReadNode frn : replacee.graph().getNodes().filter(FloatingReadNode.class)) {
-            LocationIdentity locationIdentity = frn.location().getLocationIdentity();
-            if (SnippetCounters.getValue()) {
-                // accesses to snippet counters are artificially introduced and violate the memory
-                // semantics.
-                if (locationIdentity.equals(SnippetCounter.SNIPPET_COUNTER_LOCATION)) {
-                    continue;
-                }
-            }
-            assert !kills.contains(locationIdentity) : frn + " reads from location \"" + locationIdentity + "\" but " + replacee + " does not kill this location";
+        for (LocationIdentity p : this.info.privateLocations) {
+            kills.remove(p);
         }
+
+        assert kills.isEmpty() : "snippet graph kills non-private locations " + Arrays.toString(kills.toArray()) + " that replacee (" + replacee + ") doesn't kill";
         return true;
     }