diff src/share/vm/opto/memnode.cpp @ 8116:6931f425c517

8007294: ReduceFieldZeroing doesn't check for dependent load and can lead to incorrect execution Summary: InitializeNode::can_capture_store() must check that the captured store doesn't overwrite a memory location that is loaded before the store. Reviewed-by: kvn
author roland
date Mon, 25 Feb 2013 14:13:04 +0100
parents b30b3c2a0cf2
children ff55877839bc
line wrap: on
line diff
--- a/src/share/vm/opto/memnode.cpp	Fri Feb 22 10:12:00 2013 -0800
+++ b/src/share/vm/opto/memnode.cpp	Mon Feb 25 14:13:04 2013 +0100
@@ -320,6 +320,9 @@
 
   if (mem != old_mem) {
     set_req(MemNode::Memory, mem);
+    if (can_reshape && old_mem->outcnt() == 0) {
+        igvn->_worklist.push(old_mem);
+    }
     if (phase->type( mem ) == Type::TOP) return NodeSentinel;
     return this;
   }
@@ -2319,9 +2322,9 @@
   if (ReduceFieldZeroing && /*can_reshape &&*/
       mem->is_Proj() && mem->in(0)->is_Initialize()) {
     InitializeNode* init = mem->in(0)->as_Initialize();
-    intptr_t offset = init->can_capture_store(this, phase);
+    intptr_t offset = init->can_capture_store(this, phase, can_reshape);
     if (offset > 0) {
-      Node* moved = init->capture_store(this, offset, phase);
+      Node* moved = init->capture_store(this, offset, phase, can_reshape);
       // If the InitializeNode captured me, it made a raw copy of me,
       // and I need to disappear.
       if (moved != NULL) {
@@ -3134,7 +3137,7 @@
 // an initialization.  Returns zero if a check fails.
 // On success, returns the (constant) offset to which the store applies,
 // within the initialized memory.
-intptr_t InitializeNode::can_capture_store(StoreNode* st, PhaseTransform* phase) {
+intptr_t InitializeNode::can_capture_store(StoreNode* st, PhaseTransform* phase, bool can_reshape) {
   const int FAIL = 0;
   if (st->req() != MemNode::ValueIn + 1)
     return FAIL;                // an inscrutable StoreNode (card mark?)
@@ -3156,6 +3159,91 @@
   if (!detect_init_independence(val, true, complexity_count))
     return FAIL;                // stored value must be 'simple enough'
 
+  // The Store can be captured only if nothing after the allocation
+  // and before the Store is using the memory location that the store
+  // overwrites.
+  bool failed = false;
+  // If is_complete_with_arraycopy() is true the shape of the graph is
+  // well defined and is safe so no need for extra checks.
+  if (!is_complete_with_arraycopy()) {
+    // We are going to look at each use of the memory state following
+    // the allocation to make sure nothing reads the memory that the
+    // Store writes.
+    const TypePtr* t_adr = phase->type(adr)->isa_ptr();
+    int alias_idx = phase->C->get_alias_index(t_adr);
+    ResourceMark rm;
+    Unique_Node_List mems;
+    mems.push(mem);
+    Node* unique_merge = NULL;
+    for (uint next = 0; next < mems.size(); ++next) {
+      Node *m  = mems.at(next);
+      for (DUIterator_Fast jmax, j = m->fast_outs(jmax); j < jmax; j++) {
+        Node *n = m->fast_out(j);
+        if (n->outcnt() == 0) {
+          continue;
+        }
+        if (n == st) {
+          continue;
+        } else if (n->in(0) != NULL && n->in(0) != ctl) {
+          // If the control of this use is different from the control
+          // of the Store which is right after the InitializeNode then
+          // this node cannot be between the InitializeNode and the
+          // Store.
+          continue;
+        } else if (n->is_MergeMem()) {
+          if (n->as_MergeMem()->memory_at(alias_idx) == m) {
+            // We can hit a MergeMemNode (that will likely go away
+            // later) that is a direct use of the memory state
+            // following the InitializeNode on the same slice as the
+            // store node that we'd like to capture. We need to check
+            // the uses of the MergeMemNode.
+            mems.push(n);
+          }
+        } else if (n->is_Mem()) {
+          Node* other_adr = n->in(MemNode::Address);
+          if (other_adr == adr) {
+            failed = true;
+            break;
+          } else {
+            const TypePtr* other_t_adr = phase->type(other_adr)->isa_ptr();
+            if (other_t_adr != NULL) {
+              int other_alias_idx = phase->C->get_alias_index(other_t_adr);
+              if (other_alias_idx == alias_idx) {
+                // A load from the same memory slice as the store right
+                // after the InitializeNode. We check the control of the
+                // object/array that is loaded from. If it's the same as
+                // the store control then we cannot capture the store.
+                assert(!n->is_Store(), "2 stores to same slice on same control?");
+                Node* base = other_adr;
+                assert(base->is_AddP(), err_msg_res("should be addp but is %s", base->Name()));
+                base = base->in(AddPNode::Base);
+                if (base != NULL) {
+                  base = base->uncast();
+                  if (base->is_Proj() && base->in(0) == alloc) {
+                    failed = true;
+                    break;
+                  }
+                }
+              }
+            }
+          }
+        } else {
+          failed = true;
+          break;
+        }
+      }
+    }
+  }
+  if (failed) {
+    if (!can_reshape) {
+      // We decided we couldn't capture the store during parsing. We
+      // should try again during the next IGVN once the graph is
+      // cleaner.
+      phase->C->record_for_igvn(st);
+    }
+    return FAIL;
+  }
+
   return offset;                // success
 }
 
@@ -3266,11 +3354,11 @@
 //                      rawstore1 rawstore2)
 //
 Node* InitializeNode::capture_store(StoreNode* st, intptr_t start,
-                                    PhaseTransform* phase) {
+                                    PhaseTransform* phase, bool can_reshape) {
   assert(stores_are_sane(phase), "");
 
   if (start < 0)  return NULL;
-  assert(can_capture_store(st, phase) == start, "sanity");
+  assert(can_capture_store(st, phase, can_reshape) == start, "sanity");
 
   Compile* C = phase->C;
   int size_in_bytes = st->memory_size();