6791572: assert("duplicating node that's already been matched")
authorkvn
Mon, 23 Feb 2009 16:03:19 -0800
changeset 2112 df46c83588fe
parent 2111 dab8a43dd738
child 2113 5c00968b7998
6791572: assert("duplicating node that's already been matched") Summary: Mark inputs for an address expression as shared if there are other uses besides address expressions. Reviewed-by: never
hotspot/src/share/vm/opto/matcher.cpp
--- a/hotspot/src/share/vm/opto/matcher.cpp	Mon Feb 23 12:02:30 2009 -0800
+++ b/hotspot/src/share/vm/opto/matcher.cpp	Mon Feb 23 16:03:19 2009 -0800
@@ -1707,11 +1707,18 @@
 void Matcher::find_shared( Node *n ) {
   // Allocate stack of size C->unique() * 2 to avoid frequent realloc
   MStack mstack(C->unique() * 2);
+  // Mark nodes as address_visited if they are inputs to an address expression
+  VectorSet address_visited(Thread::current()->resource_area());
   mstack.push(n, Visit);     // Don't need to pre-visit root node
   while (mstack.is_nonempty()) {
     n = mstack.node();       // Leave node on stack
     Node_State nstate = mstack.state();
+    uint nop = n->Opcode();
     if (nstate == Pre_Visit) {
+      if (address_visited.test(n->_idx)) { // Visited in address already?
+        // Flag as visited and shared now.
+        set_visited(n);
+      }
       if (is_visited(n)) {   // Visited already?
         // Node is shared and has no reason to clone.  Flag it as shared.
         // This causes it to match into a register for the sharing.
@@ -1726,7 +1733,7 @@
       set_visited(n);   // Flag as visited now
       bool mem_op = false;
 
-      switch( n->Opcode() ) {  // Handle some opcodes special
+      switch( nop ) {  // Handle some opcodes special
       case Op_Phi:             // Treat Phis as shared roots
       case Op_Parm:
       case Op_Proj:            // All handled specially during matching
@@ -1887,34 +1894,51 @@
             // to have a single use so force sharing here.
             set_shared(m->in(AddPNode::Base)->in(1));
           }
+
+          // Some inputs for address expression are not put on stack
+          // to avoid marking them as shared and forcing them into register
+          // if they are used only in address expressions.
+          // But they should be marked as shared if there are other uses
+          // besides address expressions.
+
           Node *off = m->in(AddPNode::Offset);
-          if( off->is_Con() ) {
-            set_visited(m);  // Flag as visited now
+          if( off->is_Con() &&
+              // When there are other uses besides address expressions
+              // put it on stack and mark as shared.
+              !is_visited(m) ) {
+            address_visited.test_set(m->_idx); // Flag as address_visited
             Node *adr = m->in(AddPNode::Address);
 
             // Intel, ARM and friends can handle 2 adds in addressing mode
             if( clone_shift_expressions && adr->is_AddP() &&
                 // AtomicAdd is not an addressing expression.
                 // Cheap to find it by looking for screwy base.
-                !adr->in(AddPNode::Base)->is_top() ) {
-              set_visited(adr);  // Flag as visited now
+                !adr->in(AddPNode::Base)->is_top() &&
+                // Are there other uses besides address expressions?
+                !is_visited(adr) ) {
+              address_visited.set(adr->_idx); // Flag as address_visited
               Node *shift = adr->in(AddPNode::Offset);
               // Check for shift by small constant as well
               if( shift->Opcode() == Op_LShiftX && shift->in(2)->is_Con() &&
-                  shift->in(2)->get_int() <= 3 ) {
-                set_visited(shift);  // Flag as visited now
+                  shift->in(2)->get_int() <= 3 &&
+                  // Are there other uses besides address expressions?
+                  !is_visited(shift) ) {
+                address_visited.set(shift->_idx); // Flag as address_visited
                 mstack.push(shift->in(2), Visit);
+                Node *conv = shift->in(1);
 #ifdef _LP64
                 // Allow Matcher to match the rule which bypass
                 // ConvI2L operation for an array index on LP64
                 // if the index value is positive.
-                if( shift->in(1)->Opcode() == Op_ConvI2L &&
-                    shift->in(1)->as_Type()->type()->is_long()->_lo >= 0 ) {
-                  set_visited(shift->in(1));  // Flag as visited now
-                  mstack.push(shift->in(1)->in(1), Pre_Visit);
+                if( conv->Opcode() == Op_ConvI2L &&
+                    conv->as_Type()->type()->is_long()->_lo >= 0 &&
+                    // Are there other uses besides address expressions?
+                    !is_visited(conv) ) {
+                  address_visited.set(conv->_idx); // Flag as address_visited
+                  mstack.push(conv->in(1), Pre_Visit);
                 } else
 #endif
-                mstack.push(shift->in(1), Pre_Visit);
+                mstack.push(conv, Pre_Visit);
               } else {
                 mstack.push(shift, Pre_Visit);
               }