6701887: JDK7 server VM in endless loop in Node::dominates
authorkvn
Thu, 15 May 2008 22:43:11 -0700
changeset 581 02338c8a1bcf
parent 580 aed902488ae4
child 582 9e32e601f40c
child 588 96be33305a55
6701887: JDK7 server VM in endless loop in Node::dominates Summary: The method Node::dominates loops in the dead code which does not have a Region node. Reviewed-by: jrose, never
hotspot/src/share/vm/opto/c2_globals.hpp
hotspot/src/share/vm/opto/memnode.cpp
hotspot/src/share/vm/opto/node.cpp
--- a/hotspot/src/share/vm/opto/c2_globals.hpp	Thu May 15 22:40:43 2008 -0700
+++ b/hotspot/src/share/vm/opto/c2_globals.hpp	Thu May 15 22:43:11 2008 -0700
@@ -390,5 +390,8 @@
                                                                             \
   product(intx, MaxLabelRootDepth, 1100,                                    \
           "Maximum times call Label_Root to prevent stack overflow")        \
+                                                                            \
+  diagnostic(intx, DominatorSearchLimit, 1000,                              \
+          "Iterations limit in Node::dominates")                            \
 
 C2_FLAGS(DECLARE_DEVELOPER_FLAG, DECLARE_PD_DEVELOPER_FLAG, DECLARE_PRODUCT_FLAG, DECLARE_PD_PRODUCT_FLAG, DECLARE_DIAGNOSTIC_FLAG, DECLARE_NOTPRODUCT_FLAG)
--- a/hotspot/src/share/vm/opto/memnode.cpp	Thu May 15 22:40:43 2008 -0700
+++ b/hotspot/src/share/vm/opto/memnode.cpp	Thu May 15 22:43:11 2008 -0700
@@ -256,7 +256,7 @@
   if (dom == NULL || dom->is_top())
     return false; // Conservative answer for dead code
 
-  if (dom->is_Start() || dom->is_Root() || dom == sub)
+  if (dom->is_Con() || dom->is_Start() || dom->is_Root() || dom == sub)
     return true;
 
   // 'dom' dominates 'sub' if its control edge and control edges
@@ -298,7 +298,7 @@
           return false; // Conservative answer for dead code
         assert(n->is_CFG(), "expecting control");
       }
-      if (n->is_Start() || n->is_Root()) {
+      if (n->is_Con() || n->is_Start() || n->is_Root()) {
         only_dominating_controls = true;
       } else if (n->is_CFG()) {
         if (n->dominates(sub, nlist))
@@ -308,12 +308,11 @@
       } else {
         // First, own control edge.
         Node* m = n->find_exact_control(n->in(0));
-        if (m == NULL)
-          continue;
-        if (m->is_top())
-          return false; // Conservative answer for dead code
-        dom_list.push(m);
-
+        if (m != NULL) {
+          if (m->is_top())
+            return false; // Conservative answer for dead code
+          dom_list.push(m);
+        }
         // Now, the rest of edges.
         uint cnt = n->req();
         for (uint i = 1; i < cnt; i++) {
--- a/hotspot/src/share/vm/opto/node.cpp	Thu May 15 22:40:43 2008 -0700
+++ b/hotspot/src/share/vm/opto/node.cpp	Thu May 15 22:43:11 2008 -0700
@@ -1043,6 +1043,9 @@
   assert(this->is_CFG(), "expecting control");
   assert(sub != NULL && sub->is_CFG(), "expecting control");
 
+  // detect dead cycle without regions
+  int iterations_without_region_limit = DominatorSearchLimit;
+
   Node* orig_sub = sub;
   nlist.clear();
   bool this_dominates = false;
@@ -1057,6 +1060,7 @@
         // Region nodes were visited. Continue walk up to Start or Root
         // to make sure that it did not walk in a cycle.
         this_dominates = true; // first time meet
+        iterations_without_region_limit = DominatorSearchLimit; // Reset
       } else {
         return false;          // already met before: walk in a cycle
       }
@@ -1069,19 +1073,20 @@
       return false; // Conservative answer for dead code
 
     if (sub == up && sub->is_Loop()) {
-      up = sub->in(0); // in(LoopNode::EntryControl);
-    } else if (sub == up && sub->is_Region()) {
+      up = sub->in(1); // in(LoopNode::EntryControl);
+    } else if (sub == up && sub->is_Region() && sub->req() == 3) {
+      iterations_without_region_limit = DominatorSearchLimit; // Reset
       uint i = 1;
-      if (nlist.size() == 0) {
+      uint size = nlist.size();
+      if (size == 0) {
         // No Region nodes (except Loops) were visited before.
         // Take first valid path on the way up to 'this'.
-      } else if (nlist.at(nlist.size() - 1) == sub) {
+      } else if (nlist.at(size - 1) == sub) {
         // This Region node was just visited. Take other path.
         i = region_input + 1;
         nlist.pop();
       } else {
         // Was this Region node visited before?
-        uint size = nlist.size();
         for (uint j = 0; j < size; j++) {
           if (nlist.at(j) == sub) {
             return false; // The Region node was visited before. Give up.
@@ -1104,8 +1109,9 @@
     }
     if (sub == up)
       return false;    // some kind of tight cycle
-    if (orig_sub == up)
-      return false;    // walk in a cycle
+
+    if (--iterations_without_region_limit < 0)
+      return false;    // dead cycle
 
     sub = up;
   }