8059241: C2: Excessive RemoveUseless passes during incremental inlining
authorvlivanov
Thu, 31 Jan 2019 17:48:25 -0800
changeset 53594 47a8fdf84424
parent 53593 1ceebbe2c1da
child 53595 8462b295c08b
8059241: C2: Excessive RemoveUseless passes during incremental inlining Reviewed-by: roland, shade
src/hotspot/share/opto/callGenerator.cpp
src/hotspot/share/opto/castnode.cpp
src/hotspot/share/opto/compile.cpp
src/hotspot/share/opto/compile.hpp
--- a/src/hotspot/share/opto/callGenerator.cpp	Thu Jan 31 13:36:31 2019 -0800
+++ b/src/hotspot/share/opto/callGenerator.cpp	Thu Jan 31 17:48:25 2019 -0800
@@ -459,7 +459,7 @@
   C->set_has_loops(C->has_loops() || _inline_cg->method()->has_loops());
   C->env()->notice_inlined_method(_inline_cg->method());
   C->set_inlining_progress(true);
-
+  C->set_do_cleanup(kit.stopped()); // path is dead; needs cleanup
   kit.replace_call(call, result, true);
 }
 
--- a/src/hotspot/share/opto/castnode.cpp	Thu Jan 31 13:36:31 2019 -0800
+++ b/src/hotspot/share/opto/castnode.cpp	Thu Jan 31 17:48:25 2019 -0800
@@ -410,11 +410,11 @@
                                 Node* dispX,
                                 bool negate = false) {
   if (negate) {
-    dispX = new SubXNode(phase->MakeConX(0), phase->transform(dispX));
+    dispX = phase->transform(new SubXNode(phase->MakeConX(0), dispX));
   }
   return new AddPNode(phase->C->top(),
                       phase->transform(new CastX2PNode(base)),
-                      phase->transform(dispX));
+                      dispX);
 }
 
 Node *CastX2PNode::Ideal(PhaseGVN *phase, bool can_reshape) {
--- a/src/hotspot/share/opto/compile.cpp	Thu Jan 31 13:36:31 2019 -0800
+++ b/src/hotspot/share/opto/compile.cpp	Thu Jan 31 17:48:25 2019 -0800
@@ -648,6 +648,7 @@
                   _orig_pc_slot_offset_in_bytes(0),
                   _inlining_progress(false),
                   _inlining_incrementally(false),
+                  _do_cleanup(false),
                   _has_reserved_stack_access(target->has_reserved_stack_access()),
 #ifndef PRODUCT
                   _trace_opto_output(directive->TraceOptoOutputOption),
@@ -2051,52 +2052,49 @@
     }
     _boxing_late_inlines.trunc_to(0);
 
-    {
-      ResourceMark rm;
-      PhaseRemoveUseless pru(gvn, for_igvn());
-    }
-
-    igvn = PhaseIterGVN(gvn);
-    igvn.optimize();
-
-    set_inlining_progress(false);
+    inline_incrementally_cleanup(igvn);
+
     set_inlining_incrementally(false);
   }
 }
 
-void Compile::inline_incrementally_one(PhaseIterGVN& igvn) {
+bool Compile::inline_incrementally_one() {
   assert(IncrementalInline, "incremental inlining should be on");
-  PhaseGVN* gvn = initial_gvn();
+
+  TracePhase tp("incrementalInline_inline", &timers[_t_incrInline_inline]);
+  set_inlining_progress(false);
+  set_do_cleanup(false);
+  int i = 0;
+  for (; i <_late_inlines.length() && !inlining_progress(); i++) {
+    CallGenerator* cg = _late_inlines.at(i);
+    _late_inlines_pos = i+1;
+    cg->do_late_inline();
+    if (failing())  return false;
+  }
+  int j = 0;
+  for (; i < _late_inlines.length(); i++, j++) {
+    _late_inlines.at_put(j, _late_inlines.at(i));
+  }
+  _late_inlines.trunc_to(j);
+  assert(inlining_progress() || _late_inlines.length() == 0, "");
+
+  bool needs_cleanup = do_cleanup() || over_inlining_cutoff();
 
   set_inlining_progress(false);
-  for_igvn()->clear();
-  gvn->replace_with(&igvn);
-
-  {
-    TracePhase tp("incrementalInline_inline", &timers[_t_incrInline_inline]);
-    int i = 0;
-    for (; i <_late_inlines.length() && !inlining_progress(); i++) {
-      CallGenerator* cg = _late_inlines.at(i);
-      _late_inlines_pos = i+1;
-      cg->do_late_inline();
-      if (failing())  return;
-    }
-    int j = 0;
-    for (; i < _late_inlines.length(); i++, j++) {
-      _late_inlines.at_put(j, _late_inlines.at(i));
-    }
-    _late_inlines.trunc_to(j);
-  }
-
+  set_do_cleanup(false);
+  return (_late_inlines.length() > 0) && !needs_cleanup;
+}
+
+void Compile::inline_incrementally_cleanup(PhaseIterGVN& igvn) {
   {
     TracePhase tp("incrementalInline_pru", &timers[_t_incrInline_pru]);
     ResourceMark rm;
-    PhaseRemoveUseless pru(gvn, for_igvn());
+    PhaseRemoveUseless pru(initial_gvn(), for_igvn());
   }
-
   {
     TracePhase tp("incrementalInline_igvn", &timers[_t_incrInline_igvn]);
-    igvn = PhaseIterGVN(gvn);
+    igvn = PhaseIterGVN(initial_gvn());
+    igvn.optimize();
   }
 }
 
@@ -2104,14 +2102,10 @@
 void Compile::inline_incrementally(PhaseIterGVN& igvn) {
   TracePhase tp("incrementalInline", &timers[_t_incrInline]);
 
-  PhaseGVN* gvn = initial_gvn();
-
   set_inlining_incrementally(true);
-  set_inlining_progress(true);
   uint low_live_nodes = 0;
 
-  while(inlining_progress() && _late_inlines.length() > 0) {
-
+  while (_late_inlines.length() > 0) {
     if (live_nodes() > (uint)LiveNodeCountInliningCutoff) {
       if (low_live_nodes < (uint)LiveNodeCountInliningCutoff * 8 / 10) {
         TracePhase tp("incrementalInline_ideal", &timers[_t_incrInline_ideal]);
@@ -2125,22 +2119,23 @@
       }
 
       if (live_nodes() > (uint)LiveNodeCountInliningCutoff) {
-        break;
+        break; // finish
       }
     }
 
-    inline_incrementally_one(igvn);
-
-    if (failing())  return;
-
-    {
-      TracePhase tp("incrementalInline_igvn", &timers[_t_incrInline_igvn]);
-      igvn.optimize();
+    for_igvn()->clear();
+    initial_gvn()->replace_with(&igvn);
+
+    while (inline_incrementally_one()) {
+      assert(!failing(), "inconsistent");
     }
 
     if (failing())  return;
+
+    inline_incrementally_cleanup(igvn);
+
+    if (failing())  return;
   }
-
   assert( igvn._worklist.size() == 0, "should be done with igvn" );
 
   if (_string_late_inlines.length() > 0) {
@@ -2152,17 +2147,7 @@
 
     if (failing())  return;
 
-    {
-      TracePhase tp("incrementalInline_pru", &timers[_t_incrInline_pru]);
-      ResourceMark rm;
-      PhaseRemoveUseless pru(initial_gvn(), for_igvn());
-    }
-
-    {
-      TracePhase tp("incrementalInline_igvn", &timers[_t_incrInline_igvn]);
-      igvn = PhaseIterGVN(gvn);
-      igvn.optimize();
-    }
+    inline_incrementally_cleanup(igvn);
   }
 
   set_inlining_incrementally(false);
--- a/src/hotspot/share/opto/compile.hpp	Thu Jan 31 13:36:31 2019 -0800
+++ b/src/hotspot/share/opto/compile.hpp	Thu Jan 31 17:48:25 2019 -0800
@@ -383,6 +383,7 @@
   int                   _major_progress;        // Count of something big happening
   bool                  _inlining_progress;     // progress doing incremental inlining?
   bool                  _inlining_incrementally;// Are we doing incremental inlining (post parse)
+  bool                  _do_cleanup;            // Cleanup is needed before proceeding with incremental inlining
   bool                  _has_loops;             // True if the method _may_ have some loops
   bool                  _has_split_ifs;         // True if the method _may_ have some split-if
   bool                  _has_unsafe_access;     // True if the method _may_ produce faults in unsafe loads or stores.
@@ -653,6 +654,8 @@
   int               inlining_progress() const   { return _inlining_progress; }
   void          set_inlining_incrementally(bool z) { _inlining_incrementally = z; }
   int               inlining_incrementally() const { return _inlining_incrementally; }
+  void          set_do_cleanup(bool z)          { _do_cleanup = z; }
+  int               do_cleanup() const          { return _do_cleanup; }
   void          set_major_progress()            { _major_progress++; }
   void        clear_major_progress()            { _major_progress = 0; }
   int               max_inline_size() const     { return _max_inline_size; }
@@ -1075,7 +1078,11 @@
     if (!inlining_incrementally()) {
       return unique() > (uint)NodeCountInliningCutoff;
     } else {
-      return live_nodes() > (uint)LiveNodeCountInliningCutoff;
+      // Give some room for incremental inlining algorithm to "breathe"
+      // and avoid thrashing when live node count is close to the limit.
+      // Keep in mind that live_nodes() isn't accurate during inlining until
+      // dead node elimination step happens (see Compile::inline_incrementally).
+      return live_nodes() > (uint)LiveNodeCountInliningCutoff * 11 / 10;
     }
   }
 
@@ -1083,7 +1090,8 @@
   void dec_number_of_mh_late_inlines() { assert(_number_of_mh_late_inlines > 0, "_number_of_mh_late_inlines < 0 !"); _number_of_mh_late_inlines--; }
   bool has_mh_late_inlines() const     { return _number_of_mh_late_inlines > 0; }
 
-  void inline_incrementally_one(PhaseIterGVN& igvn);
+  bool inline_incrementally_one();
+  void inline_incrementally_cleanup(PhaseIterGVN& igvn);
   void inline_incrementally(PhaseIterGVN& igvn);
   void inline_string_calls(bool parse_time);
   void inline_boxing_calls(PhaseIterGVN& igvn);