7024475: loop doesn't terminate when compiled
authornever
Sun, 27 Mar 2011 00:00:14 -0700
changeset 8870 119881dc9d0b
parent 8869 dd30e4318b8d
child 8871 5c3b26c4119e
7024475: loop doesn't terminate when compiled Reviewed-by: kvn
hotspot/src/share/vm/compiler/compileBroker.cpp
hotspot/src/share/vm/opto/idealGraphPrinter.cpp
hotspot/src/share/vm/opto/idealGraphPrinter.hpp
hotspot/src/share/vm/opto/loopTransform.cpp
hotspot/src/share/vm/opto/loopnode.cpp
hotspot/src/share/vm/opto/node.cpp
hotspot/src/share/vm/runtime/globals.hpp
hotspot/test/compiler/7024475/Test7024475.java
--- a/hotspot/src/share/vm/compiler/compileBroker.cpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/compiler/compileBroker.cpp	Sun Mar 27 00:00:14 2011 -0700
@@ -874,6 +874,14 @@
     return;
   }
 
+#ifndef PRODUCT
+  if (osr_bci != -1 && !FLAG_IS_DEFAULT(OSROnlyBCI)) {
+    if ((OSROnlyBCI > 0) ? (OSROnlyBCI != osr_bci) : (-OSROnlyBCI == osr_bci)) {
+      // Positive OSROnlyBCI means only compile that bci.  Negative means don't compile that BCI.
+      return;
+    }
+  }
+#endif
 
   // If this method is already in the compile queue, then
   // we do not block the current thread.
--- a/hotspot/src/share/vm/opto/idealGraphPrinter.cpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/opto/idealGraphPrinter.cpp	Sun Mar 27 00:00:14 2011 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -599,11 +599,35 @@
 
     if (caller != NULL) {
       stringStream bciStream;
+      ciMethod* last = NULL;
+      int last_bci;
       while(caller) {
+        if (caller->has_method()) {
+          last = caller->method();
+          last_bci = caller->bci();
+        }
         bciStream.print("%d ", caller->bci());
         caller = caller->caller();
       }
       print_prop("bci", bciStream.as_string());
+      if (last != NULL && last->has_linenumber_table() && last_bci >= 0) {
+        print_prop("line", last->line_number_from_bci(last_bci));
+      }
+    }
+
+    if (node->debug_orig() != NULL) {
+      stringStream dorigStream;
+      Node* dorig = node->debug_orig();
+      if (dorig) {
+        dorigStream.print("%d ", dorig->_idx);
+        Node* first = dorig;
+        dorig = first->debug_orig();
+        while (dorig && dorig != first) {
+          dorigStream.print("%d ", dorig->_idx);
+          dorig = dorig->debug_orig();
+        }
+      }
+      print_prop("debug_orig", dorigStream.as_string());
     }
 
     if (_chaitin && _chaitin != (PhaseChaitin *)0xdeadbeef) {
@@ -628,6 +652,17 @@
   GrowableArray<Node *> nodeStack(Thread::current()->resource_area(), 0, 0, NULL);
   nodeStack.push(start);
   visited.test_set(start->_idx);
+  if (C->cfg() != NULL) {
+    // once we have a CFG there are some nodes that aren't really
+    // reachable but are in the CFG so add them here.
+    for (uint i = 0; i < C->cfg()->_blocks.size(); i++) {
+      Block *b = C->cfg()->_blocks[i];
+      for (uint s = 0; s < b->_nodes.size(); s++) {
+        nodeStack.push(b->_nodes[s]);
+      }
+    }
+  }
+
   while(nodeStack.length() > 0) {
 
     Node *n = nodeStack.pop();
@@ -686,16 +721,23 @@
       end_head();
 
       head(SUCCESSORS_ELEMENT);
-      for (uint s = 0; s < C->cfg()->_blocks[i]->_num_succs; s++) {
+      for (uint s = 0; s < b->_num_succs; s++) {
         begin_elem(SUCCESSOR_ELEMENT);
         print_attr(BLOCK_NAME_PROPERTY, b->_succs[s]->_pre_order);
         end_elem();
       }
       tail(SUCCESSORS_ELEMENT);
 
+      head(NODES_ELEMENT);
+      for (uint s = 0; s < b->_nodes.size(); s++) {
+        begin_elem(NODE_ELEMENT);
+        print_attr(NODE_ID_PROPERTY, get_node_id(b->_nodes[s]));
+        end_elem();
+      }
+      tail(NODES_ELEMENT);
+
       tail(BLOCK_ELEMENT);
     }
-
     tail(CONTROL_FLOW_ELEMENT);
   }
   tail(GRAPH_ELEMENT);
--- a/hotspot/src/share/vm/opto/idealGraphPrinter.hpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/opto/idealGraphPrinter.hpp	Sun Mar 27 00:00:14 2011 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -45,15 +45,6 @@
 {
 private:
 
-  enum State
-  {
-    Invalid,
-    Valid,
-    New
-  };
-
-private:
-
   static const char *INDENT;
   static const char *TOP_ELEMENT;
   static const char *GROUP_ELEMENT;
--- a/hotspot/src/share/vm/opto/loopTransform.cpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/opto/loopTransform.cpp	Sun Mar 27 00:00:14 2011 -0700
@@ -1608,15 +1608,7 @@
     return false; // Malformed loop
   if (!phase->is_member(this, phase->get_ctrl(cl->loopexit()->in(CountedLoopEndNode::TestValue))))
     return false;             // Infinite loop
-#ifndef PRODUCT
-  if (PrintOpto) {
-    tty->print("Removing empty loop");
-    this->dump_head();
-  } else if (TraceLoopOpts) {
-    tty->print("Empty        ");
-    this->dump_head();
-  }
-#endif
+
 #ifdef ASSERT
   // Ensure only one phi which is the iv.
   Node* iv = NULL;
@@ -1629,6 +1621,43 @@
   }
   assert(iv == cl->phi(), "Wrong phi" );
 #endif
+
+  // main and post loops have explicitly created zero trip guard
+  bool needs_guard = !cl->is_main_loop() && !cl->is_post_loop();
+  if (needs_guard) {
+    // Check for an obvious zero trip guard.
+    Node* inctrl = cl->in(LoopNode::EntryControl);
+    if (inctrl->Opcode() == Op_IfTrue) {
+      // The test should look like just the backedge of a CountedLoop
+      Node* iff = inctrl->in(0);
+      if (iff->is_If()) {
+        Node* bol = iff->in(1);
+        if (bol->is_Bool() && bol->as_Bool()->_test._test == cl->loopexit()->test_trip()) {
+          Node* cmp = bol->in(1);
+          if (cmp->is_Cmp() && cmp->in(1) == cl->init_trip() && cmp->in(2) == cl->limit()) {
+            needs_guard = false;
+          }
+        }
+      }
+    }
+  }
+
+#ifndef PRODUCT
+  if (PrintOpto) {
+    tty->print("Removing empty loop with%s zero trip guard", needs_guard ? "out" : "");
+    this->dump_head();
+  } else if (TraceLoopOpts) {
+    tty->print("Empty with%s zero trip guard   ", needs_guard ? "out" : "");
+    this->dump_head();
+  }
+#endif
+
+  if (needs_guard) {
+    // Peel the loop to ensure there's a zero trip guard
+    Node_List old_new;
+    phase->do_peeling(this, old_new);
+  }
+
   // Replace the phi at loop head with the final value of the last
   // iteration.  Then the CountedLoopEnd will collapse (backedge never
   // taken) and all loop-invariant uses of the exit values will be correct.
--- a/hotspot/src/share/vm/opto/loopnode.cpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/opto/loopnode.cpp	Sun Mar 27 00:00:14 2011 -0700
@@ -1064,8 +1064,6 @@
   // Cache parts in locals for easy
   PhaseIterGVN &igvn = phase->_igvn;
 
-  phase->C->print_method("Before beautify loops", 3);
-
   igvn.hash_delete(_head);      // Yank from hash before hacking edges
 
   // Check for multiple fall-in paths.  Peel off a landing pad if need be.
@@ -1547,6 +1545,7 @@
   ResourceMark rm;
 
   int old_progress = C->major_progress();
+  uint orig_worklist_size = _igvn._worklist.size();
 
   // Reset major-progress flag for the driver's heuristics
   C->clear_major_progress();
@@ -1610,6 +1609,7 @@
   // Split shared headers and insert loop landing pads.
   // Do not bother doing this on the Root loop of course.
   if( !_verify_me && !_verify_only && _ltree_root->_child ) {
+    C->print_method("Before beautify loops", 3);
     if( _ltree_root->_child->beautify_loops( this ) ) {
       // Re-build loop tree!
       _ltree_root->_child = NULL;
@@ -1694,7 +1694,7 @@
     for (int i = 0; i < old_progress; i++)
       C->set_major_progress();
     assert(C->unique() == unique, "verification mode made Nodes? ? ?");
-    assert(_igvn._worklist.size() == 0, "shouldn't push anything");
+    assert(_igvn._worklist.size() == orig_worklist_size, "shouldn't push anything");
     return;
   }
 
--- a/hotspot/src/share/vm/opto/node.cpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/opto/node.cpp	Sun Mar 27 00:00:14 2011 -0700
@@ -1373,12 +1373,12 @@
 //------------------------------find------------------------------------------
 // Find a neighbor of this Node with the given _idx
 // If idx is negative, find its absolute value, following both _in and _out.
-static void find_recur( Node* &result, Node *n, int idx, bool only_ctrl,
-                        VectorSet &old_space, VectorSet &new_space ) {
+static void find_recur(Compile* C,  Node* &result, Node *n, int idx, bool only_ctrl,
+                        VectorSet* old_space, VectorSet* new_space ) {
   int node_idx = (idx >= 0) ? idx : -idx;
   if (NotANode(n))  return;  // Gracefully handle NULL, -1, 0xabababab, etc.
-  // Contained in new_space or old_space?
-  VectorSet *v = Compile::current()->node_arena()->contains(n) ? &new_space : &old_space;
+  // Contained in new_space or old_space?   Check old_arena first since it's mostly empty.
+  VectorSet *v = C->old_arena()->contains(n) ? old_space : new_space;
   if( v->test(n->_idx) ) return;
   if( (int)n->_idx == node_idx
       debug_only(|| n->debug_idx() == node_idx) ) {
@@ -1390,19 +1390,23 @@
   v->set(n->_idx);
   for( uint i=0; i<n->len(); i++ ) {
     if( only_ctrl && !(n->is_Region()) && (n->Opcode() != Op_Root) && (i != TypeFunc::Control) ) continue;
-    find_recur( result, n->in(i), idx, only_ctrl, old_space, new_space );
+    find_recur(C, result, n->in(i), idx, only_ctrl, old_space, new_space );
   }
   // Search along forward edges also:
   if (idx < 0 && !only_ctrl) {
     for( uint j=0; j<n->outcnt(); j++ ) {
-      find_recur( result, n->raw_out(j), idx, only_ctrl, old_space, new_space );
+      find_recur(C, result, n->raw_out(j), idx, only_ctrl, old_space, new_space );
     }
   }
 #ifdef ASSERT
-  // Search along debug_orig edges last:
-  for (Node* orig = n->debug_orig(); orig != NULL && n != orig; orig = orig->debug_orig()) {
-    if (NotANode(orig))  break;
-    find_recur( result, orig, idx, only_ctrl, old_space, new_space );
+  // Search along debug_orig edges last, checking for cycles
+  Node* orig = n->debug_orig();
+  if (orig != NULL) {
+    do {
+      if (NotANode(orig))  break;
+      find_recur(C, result, orig, idx, only_ctrl, old_space, new_space );
+      orig = orig->debug_orig();
+    } while (orig != NULL && orig != n->debug_orig());
   }
 #endif //ASSERT
 }
@@ -1417,7 +1421,7 @@
   ResourceArea *area = Thread::current()->resource_area();
   VectorSet old_space(area), new_space(area);
   Node* result = NULL;
-  find_recur( result, (Node*) this, idx, false, old_space, new_space );
+  find_recur(Compile::current(), result, (Node*) this, idx, false, &old_space, &new_space );
   return result;
 }
 
@@ -1427,7 +1431,7 @@
   ResourceArea *area = Thread::current()->resource_area();
   VectorSet old_space(area), new_space(area);
   Node* result = NULL;
-  find_recur( result, (Node*) this, idx, true, old_space, new_space );
+  find_recur(Compile::current(), result, (Node*) this, idx, true, &old_space, &new_space );
   return result;
 }
 #endif
--- a/hotspot/src/share/vm/runtime/globals.hpp	Sat Mar 26 08:31:45 2011 -0700
+++ b/hotspot/src/share/vm/runtime/globals.hpp	Sun Mar 27 00:00:14 2011 -0700
@@ -2377,6 +2377,9 @@
   develop(intx, CICloneLoopTestLimit, 100,                                  \
           "size limit for blocks heuristically cloned in ciTypeFlow")       \
                                                                             \
+  develop(intx, OSROnlyBCI, -1,                                             \
+          "OSR only at this bci.  Negative values mean exclude that bci")   \
+                                                                            \
   /* temp diagnostics */                                                    \
                                                                             \
   diagnostic(bool, TraceRedundantCompiles, false,                           \
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/7024475/Test7024475.java	Sun Mar 27 00:00:14 2011 -0700
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/**
+ * @test
+ * @bug 7024475
+ * @summary loop doesn't terminate when compiled
+ *
+ * @run main Test7024475
+ */
+
+public class Test7024475 {
+
+    static int i;
+    static int x1;
+    static int[] bucket_B;
+
+    static void test(Test7024475 test, int i, int c0, int j, int c1) {
+        for (;;) {
+            if (c1 > c0) {
+                if (c0 > 253) {
+                    throw new InternalError("c0 = " + c0);
+                }
+                int index = c0 * 256 + c1;
+                if (index == -1) return;
+                i = bucket_B[index];
+                if (1 < j - i && test != null)
+                    x1 = 0;
+                j = i;
+                c1--;
+            } else {
+                c0--;
+                if (j <= 0)
+                    break;
+                c1 = 255;
+            }
+        }
+    }
+
+    public static void main(String args[]) {
+        Test7024475 t = new Test7024475();
+        bucket_B = new int[256*256];
+        for (int i = 1; i < 256*256; i++) {
+            bucket_B[i] = 1;
+        }
+        for (int n = 0; n < 100000; n++) {
+            test(t, 2, 85, 1, 134);
+        }
+    }
+}