8294540: Remove Opaque2Node: it is broken and triggers assert

Reviewed-by: chagedorn, kvn

Author: eme64

src/hotspot/share/opto/classes.hpp

@@ -268,7 +268,6 @@ macro(OnSpinWait)
 macro(Opaque1)
 macro(OpaqueLoopInit)
 macro(OpaqueLoopStride)
-macro(Opaque2)
 macro(Opaque3)
 macro(Opaque4)
 macro(ProfileBoolean)

src/hotspot/share/opto/compile.cpp

@@ -3161,7 +3161,6 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
     frc.inc_double_count();
     break;
   case Op_Opaque1:              // Remove Opaque Nodes before matching
-  case Op_Opaque2:              // Remove Opaque Nodes before matching
   case Op_Opaque3:
     n->subsume_by(n->in(1), this);
     break;

src/hotspot/share/opto/loopTransform.cpp

@@ -2287,18 +2287,7 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
       set_ctrl(new_limit, C->root());
     } else {
       // Limit is not constant.
-      if (loop_head->unrolled_count() == 1) { // only for first unroll
-        // Separate limit by Opaque node in case it is an incremented
-        // variable from previous loop to avoid using pre-incremented
-        // value which could increase register pressure.
-        // Otherwise reorg_offsets() optimization will create a separate
-        // Opaque node for each use of trip-counter and as result
-        // zero trip guard limit will be different from loop limit.
-        assert(has_ctrl(opaq), "should have it");
-        Node* opaq_ctrl = get_ctrl(opaq);
-        limit = new Opaque2Node(C, limit);
-        register_new_node(limit, opaq_ctrl);
-      }
+      assert(loop_head->unrolled_count() != 1 || has_ctrl(opaq), "should have opaque for first unroll");
       if ((stride_con > 0 && (java_subtract(limit_type->_lo, stride_con) < limit_type->_lo)) ||
           (stride_con < 0 && (java_subtract(limit_type->_hi, stride_con) > limit_type->_hi))) {
         // No underflow.
@@ -2346,20 +2335,6 @@ void PhaseIdealLoop::do_unroll(IdealLoopTree *loop, Node_List &old_new, bool adj
         new_limit = new CMoveINode(adj_bool, adj_limit, adj_max, TypeInt::INT);
       }
       register_new_node(new_limit, ctrl);
-      if (loop_head->unrolled_count() == 1) {
-        // The Opaque2 node created above (in the case of the first unrolling) hides the type of the loop limit.
-        // As a result, if the iv Phi constant folds (because it captured the iteration range), the exit test won't
-        // constant fold and the graph contains a broken counted loop.
-        const Type* new_limit_t;
-        if (stride_con > 0) {
-          new_limit_t = TypeInt::make(min_jint, limit_type->_hi, limit_type->_widen);
-        } else {
-          assert(stride_con < 0, "stride can't be 0");
-          new_limit_t = TypeInt::make(limit_type->_lo, max_jint, limit_type->_widen);
-        }
-        new_limit = new CastIINode(new_limit, new_limit_t);
-        register_new_node(new_limit, ctrl);
-      }
     }
 
     assert(new_limit != NULL, "");
@@ -3940,10 +3915,6 @@ bool IdealLoopTree::iteration_split(PhaseIdealLoop* phase, Node_List &old_new) {
     }
   }
 
-  // Minor offset re-organization to remove loop-fallout uses of
-  // trip counter when there was no major reshaping.
-  phase->reorg_offsets(this);
-
   if (_next && !_next->iteration_split(phase, old_new)) {
     return false;
   }

src/hotspot/share/opto/loopnode.hpp

@@ -1522,12 +1522,6 @@ class PhaseIdealLoop : public PhaseTransform {
   // Attempt to use a conditional move instead of a phi/branch
   Node *conditional_move( Node *n );
 
-  // Reorganize offset computations to lower register pressure.
-  // Mostly prevent loop-fallout uses of the pre-incremented trip counter
-  // (which are then alive with the post-incremented trip counter
-  // forcing an extra register move)
-  void reorg_offsets( IdealLoopTree *loop );
-
   // Check for aggressive application of 'split-if' optimization,
   // using basic block level info.
   void  split_if_with_blocks     ( VectorSet &visited, Node_Stack &nstack);

src/hotspot/share/opto/loopopts.cpp

@@ -1023,8 +1023,7 @@ Node *PhaseIdealLoop::split_if_with_blocks_pre( Node *n ) {
   if (n->is_CFG() || n->is_LoadStore()) {
     return n;
   }
-  if (n->is_Opaque1() ||     // Opaque nodes cannot be mod'd
-      n_op == Op_Opaque2) {
+  if (n->is_Opaque1()) { // Opaque nodes cannot be mod'd
     if (!C->major_progress()) {   // If chance of no more loop opts...
       _igvn._worklist.push(n);  // maybe we'll remove them
     }
@@ -1426,14 +1425,6 @@ void PhaseIdealLoop::split_if_with_blocks_post(Node *n) {
   try_sink_out_of_loop(n);
 
   try_move_store_after_loop(n);
-
-  // Check for Opaque2's who's loop has disappeared - who's input is in the
-  // same loop nest as their output.  Remove 'em, they are no longer useful.
-  if( n_op == Op_Opaque2 &&
-      n->in(1) != NULL &&
-      get_loop(get_ctrl(n)) == get_loop(get_ctrl(n->in(1))) ) {
-    _igvn.replace_node( n, n->in(1) );
-  }
 }
 
 // Transform:
@@ -4106,89 +4097,3 @@ bool PhaseIdealLoop::duplicate_loop_backedge(IdealLoopTree *loop, Node_List &old
 
   return true;
 }
-
-//------------------------------reorg_offsets----------------------------------
-// Reorganize offset computations to lower register pressure.  Mostly
-// prevent loop-fallout uses of the pre-incremented trip counter (which are
-// then alive with the post-incremented trip counter forcing an extra
-// register move):
-//
-//     iv Phi            iv Phi
-//       |                 |
-//       |                AddI (+stride)
-//       |                 |
-//       |              Opaque2  # Blocks IGVN from folding these nodes until loop opts are over.
-//       |     ====>       |
-//       |                AddI (-stride)
-//       |                 |
-//       |               CastII  # Preserve type of iv Phi
-//       |                 |
-//   Outside Use       Outside Use
-//
-void PhaseIdealLoop::reorg_offsets(IdealLoopTree *loop) {
-  // Perform it only for canonical counted loops.
-  // Loop's shape could be messed up by iteration_split_impl.
-  if (!loop->_head->is_CountedLoop())
-    return;
-  if (!loop->_head->as_Loop()->is_valid_counted_loop(T_INT))
-    return;
-
-  CountedLoopNode *cl = loop->_head->as_CountedLoop();
-  CountedLoopEndNode *cle = cl->loopexit();
-  Node *exit = cle->proj_out(false);
-  Node *phi = cl->phi();
-
-  // Check for the special case when using the pre-incremented trip-counter on
-  // the fall-out  path (forces the pre-incremented  and post-incremented trip
-  // counter to be live  at the same time).  Fix this by  adjusting to use the
-  // post-increment trip counter.
-
-  bool progress = true;
-  while (progress) {
-    progress = false;
-    for (DUIterator_Fast imax, i = phi->fast_outs(imax); i < imax; i++) {
-      Node* use = phi->fast_out(i);   // User of trip-counter
-      if (!has_ctrl(use))  continue;
-      Node *u_ctrl = get_ctrl(use);
-      if (use->is_Phi()) {
-        u_ctrl = NULL;
-        for (uint j = 1; j < use->req(); j++)
-          if (use->in(j) == phi)
-            u_ctrl = dom_lca(u_ctrl, use->in(0)->in(j));
-      }
-      IdealLoopTree *u_loop = get_loop(u_ctrl);
-      // Look for loop-invariant use
-      if (u_loop == loop) continue;
-      if (loop->is_member(u_loop)) continue;
-      // Check that use is live out the bottom.  Assuming the trip-counter
-      // update is right at the bottom, uses of of the loop middle are ok.
-      if (dom_lca(exit, u_ctrl) != exit) continue;
-      // Hit!  Refactor use to use the post-incremented tripcounter.
-      // Compute a post-increment tripcounter.
-      Node* c = exit;
-      if (cl->is_strip_mined()) {
-        IdealLoopTree* outer_loop = get_loop(cl->outer_loop());
-        if (!outer_loop->is_member(u_loop)) {
-          c = cl->outer_loop_exit();
-        }
-      }
-      Node *opaq = new Opaque2Node(C, cle->incr());
-      register_new_node(opaq, c);
-      Node *neg_stride = _igvn.intcon(-cle->stride_con());
-      set_ctrl(neg_stride, C->root());
-      Node *post = new AddINode(opaq, neg_stride);
-      register_new_node(post, c);
-      post = new CastIINode(post, phi->bottom_type()); // preserve the iv phi's type
-      register_new_node(post, c);
-      _igvn.rehash_node_delayed(use);
-      for (uint j = 1; j < use->req(); j++) {
-        if (use->in(j) == phi)
-          use->set_req(j, post);
-      }
-      // Since DU info changed, rerun loop
-      progress = true;
-      break;
-    }
-  }
-
-}

src/hotspot/share/opto/macro.cpp

@@ -2371,7 +2371,6 @@ void PhaseMacroExpand::eliminate_macro_nodes() {
         break;
       default:
         assert(n->Opcode() == Op_LoopLimit ||
-               n->Opcode() == Op_Opaque2   ||
                n->Opcode() == Op_Opaque3   ||
                n->Opcode() == Op_Opaque4   ||
                BarrierSet::barrier_set()->barrier_set_c2()->is_gc_barrier_node(n),
@@ -2414,7 +2413,7 @@ bool PhaseMacroExpand::expand_macro_nodes() {
         C->remove_macro_node(n);
         _igvn._worklist.push(n);
         success = true;
-      } else if (n->is_Opaque1() || n->Opcode() == Op_Opaque2) {
+      } else if (n->is_Opaque1()) {
         _igvn.replace_node(n, n->in(1));
         success = true;
 #if INCLUDE_RTM_OPT

src/hotspot/share/opto/opaquenode.cpp

@@ -44,20 +44,8 @@ Node* Opaque1Node::Identity(PhaseGVN* phase) {
   return this;
 }
 
-//=============================================================================
-// A node to prevent unwanted optimizations.  Allows constant folding.  Stops
-// value-numbering, most Ideal calls or Identity functions.  This Node is
-// specifically designed to prevent the pre-increment value of a loop trip
-// counter from being live out of the bottom of the loop (hence causing the
-// pre- and post-increment values both being live and thus requiring an extra
-// temp register and an extra move).  If we "accidentally" optimize through
-// this kind of a Node, we'll get slightly pessimal, but correct, code.  Thus
-// it's OK to be slightly sloppy on optimizations here.
-
-// Do NOT remove the opaque node until no more loop opts can happen. Opaque1
-// and Opaque2 nodes are removed together in order to optimize loops away
-// before macro expansion.
-Node* Opaque2Node::Identity(PhaseGVN* phase) {
+// Do NOT remove the opaque node until no more loop opts can happen.
+Node* Opaque3Node::Identity(PhaseGVN* phase) {
   if (phase->C->post_loop_opts_phase()) {
     return in(1);
   } else {
@@ -67,8 +55,8 @@ Node* Opaque2Node::Identity(PhaseGVN* phase) {
 }
 
 // Do not allow value-numbering
-uint Opaque2Node::hash() const { return NO_HASH; }
-bool Opaque2Node::cmp( const Node &n ) const {
+uint Opaque3Node::hash() const { return NO_HASH; }
+bool Opaque3Node::cmp(const Node &n) const {
   return (&n == this);          // Always fail except on self
 }
 

src/hotspot/share/opto/opaquenode.hpp

@@ -70,38 +70,23 @@ class OpaqueLoopStrideNode : public Opaque1Node {
   virtual int Opcode() const;
 };
 
-//------------------------------Opaque2Node------------------------------------
-// A node to prevent unwanted optimizations.  Allows constant folding.  Stops
-// value-numbering, most Ideal calls or Identity functions.  This Node is
-// specifically designed to prevent the pre-increment value of a loop trip
-// counter from being live out of the bottom of the loop (hence causing the
-// pre- and post-increment values both being live and thus requiring an extra
-// temp register and an extra move).  If we "accidentally" optimize through
-// this kind of a Node, we'll get slightly pessimal, but correct, code.  Thus
-// it's OK to be slightly sloppy on optimizations here.
-class Opaque2Node : public Node {
-  virtual uint hash() const ;                  // { return NO_HASH; }
-  virtual bool cmp( const Node &n ) const;
+//------------------------------Opaque3Node------------------------------------
+// A node to prevent unwanted optimizations. Will be optimized only during
+// macro nodes expansion.
+class Opaque3Node : public Node {
+  int _opt; // what optimization it was used for
+  virtual uint hash() const;
+  virtual bool cmp(const Node &n) const;
   public:
-  Opaque2Node( Compile* C, Node *n ) : Node(0,n) {
+  enum { RTM_OPT };
+  Opaque3Node(Compile* C, Node* n, int opt) : Node(0, n), _opt(opt) {
     // Put it on the Macro nodes list to removed during macro nodes expansion.
     init_flags(Flag_is_macro);
     C->add_macro_node(this);
   }
   virtual int Opcode() const;
   virtual const Type* bottom_type() const { return TypeInt::INT; }
   virtual Node* Identity(PhaseGVN* phase);
-};
-
-//------------------------------Opaque3Node------------------------------------
-// A node to prevent unwanted optimizations. Will be optimized only during
-// macro nodes expansion.
-class Opaque3Node : public Opaque2Node {
-  int _opt; // what optimization it was used for
-  public:
-  enum { RTM_OPT };
-  Opaque3Node(Compile* C, Node *n, int opt) : Opaque2Node(C, n), _opt(opt) {}
-  virtual int Opcode() const;
   bool rtm_opt() const { return (_opt == RTM_OPT); }
 };
 

src/hotspot/share/opto/phaseX.cpp

@@ -2116,7 +2116,6 @@ Node *PhaseCCP::transform_once( Node *n ) {
   case Op_CountedLoop:
   case Op_Conv2B:
   case Op_Opaque1:
-  case Op_Opaque2:
     _worklist.push(n);
     break;
   default:

src/hotspot/share/opto/subnode.cpp

@@ -69,15 +69,6 @@ Node* SubNode::Identity(PhaseGVN* phase) {
     if (in(1)->in(1) == in(2)) {
       return in(1)->in(2);
     }
-
-    // Also catch: "(X + Opaque2(Y)) - Y".  In this case, 'Y' is a loop-varying
-    // trip counter and X is likely to be loop-invariant (that's how O2 Nodes
-    // are originally used, although the optimizer sometimes jiggers things).
-    // This folding through an O2 removes a loop-exit use of a loop-varying
-    // value and generally lowers register pressure in and around the loop.
-    if (in(1)->in(2)->Opcode() == Op_Opaque2 && in(1)->in(2)->in(1) == in(2)) {
-      return in(1)->in(1);
-    }
   }
 
   return ( phase->type( in(2) )->higher_equal( zero ) ) ? in(1) : this;