changeset 6021:33878f8ae419

8030976: Untaken paths should be more vigorously pruned at highest optimization level Reviewed-by: kvn, rbackman, roland, vlivanov
author sgabdura
date Thu, 23 Oct 2014 10:53:13 +0200
parents 21a476d3a51c
children 82ee04e1f525
files src/share/vm/oops/methodDataOop.hpp src/share/vm/opto/parse2.cpp src/share/vm/runtime/deoptimization.cpp src/share/vm/runtime/deoptimization.hpp src/share/vm/runtime/vmStructs.cpp
diffstat 5 files changed, 12 insertions(+), 72 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/oops/methodDataOop.hpp	Thu Oct 02 11:36:44 2014 -0700
+++ b/src/share/vm/oops/methodDataOop.hpp	Thu Oct 23 10:53:13 2014 +0200
@@ -1203,7 +1203,7 @@
   // Whole-method sticky bits and flags
 public:
   enum {
-    _trap_hist_limit    = 17,   // decoupled from Deoptimization::Reason_LIMIT
+    _trap_hist_limit    = 18,   // decoupled from Deoptimization::Reason_LIMIT
     _trap_hist_mask     = max_jubyte,
     _extra_data_count   = 4     // extra DataLayout headers, for trap history
   }; // Public flag values
--- a/src/share/vm/opto/parse2.cpp	Thu Oct 02 11:36:44 2014 -0700
+++ b/src/share/vm/opto/parse2.cpp	Thu Oct 23 10:53:13 2014 +0200
@@ -895,53 +895,12 @@
 // if a path is never taken, its controlling comparison is
 // already acting in a stable fashion.  If the comparison
 // seems stable, we will put an expensive uncommon trap
-// on the untaken path.  To be conservative, and to allow
-// partially executed counted loops to be compiled fully,
-// we will plant uncommon traps only after pointer comparisons.
+// on the untaken path.
 bool Parse::seems_stable_comparison(BoolTest::mask btest, Node* cmp) {
-  for (int depth = 4; depth > 0; depth--) {
-    // The following switch can find CmpP here over half the time for
-    // dynamic language code rich with type tests.
-    // Code using counted loops or array manipulations (typical
-    // of benchmarks) will have many (>80%) CmpI instructions.
-    switch (cmp->Opcode()) {
-    case Op_CmpP:
-      // A never-taken null check looks like CmpP/BoolTest::eq.
-      // These certainly should be closed off as uncommon traps.
-      if (btest == BoolTest::eq)
-        return true;
-      // A never-failed type check looks like CmpP/BoolTest::ne.
-      // Let's put traps on those, too, so that we don't have to compile
-      // unused paths with indeterminate dynamic type information.
-      if (ProfileDynamicTypes)
-        return true;
-      return false;
-
-    case Op_CmpI:
-      // A small minority (< 10%) of CmpP are masked as CmpI,
-      // as if by boolean conversion ((p == q? 1: 0) != 0).
-      // Detect that here, even if it hasn't optimized away yet.
-      // Specifically, this covers the 'instanceof' operator.
-      if (btest == BoolTest::ne || btest == BoolTest::eq) {
-        if (_gvn.type(cmp->in(2))->singleton() &&
-            cmp->in(1)->is_Phi()) {
-          PhiNode* phi = cmp->in(1)->as_Phi();
-          int true_path = phi->is_diamond_phi();
-          if (true_path > 0 &&
-              _gvn.type(phi->in(1))->singleton() &&
-              _gvn.type(phi->in(2))->singleton()) {
-            // phi->region->if_proj->ifnode->bool->cmp
-            BoolNode* bol = phi->in(0)->in(1)->in(0)->in(1)->as_Bool();
-            btest = bol->_test._test;
-            cmp = bol->in(1);
-            continue;
-          }
-        }
-      }
-      return false;
-    }
+  if (C->too_many_traps(method(), bci(), Deoptimization::Reason_unstable_if)) {
+    return false;
   }
-  return false;
+  return true;
 }
 
 //-------------------------------repush_if_args--------------------------------
@@ -1180,32 +1139,8 @@
   bool is_fallthrough = (path == successor_for_bci(iter().next_bci()));
 
   if (seems_never_taken(prob) && seems_stable_comparison(btest, c)) {
-    // If this might possibly turn into an implicit null check,
-    // and the null has never yet been seen, we need to generate
-    // an uncommon trap, so as to recompile instead of suffering
-    // with very slow branches.  (We'll get the slow branches if
-    // the program ever changes phase and starts seeing nulls here.)
-    //
-    // We do not inspect for a null constant, since a node may
-    // optimize to 'null' later on.
-    //
-    // Null checks, and other tests which expect inequality,
-    // show btest == BoolTest::eq along the non-taken branch.
-    // On the other hand, type tests, must-be-null tests,
-    // and other tests which expect pointer equality,
-    // show btest == BoolTest::ne along the non-taken branch.
-    // We prune both types of branches if they look unused.
     repush_if_args();
-    // We need to mark this branch as taken so that if we recompile we will
-    // see that it is possible. In the tiered system the interpreter doesn't
-    // do profiling and by the time we get to the lower tier from the interpreter
-    // the path may be cold again. Make sure it doesn't look untaken
-    if (is_fallthrough) {
-      profile_not_taken_branch(!ProfileInterpreter);
-    } else {
-      profile_taken_branch(iter().get_dest(), !ProfileInterpreter);
-    }
-    uncommon_trap(Deoptimization::Reason_unreached,
+    uncommon_trap(Deoptimization::Reason_unstable_if,
                   Deoptimization::Action_reinterpret,
                   NULL,
                   (is_fallthrough ? "taken always" : "taken never"));
--- a/src/share/vm/runtime/deoptimization.cpp	Thu Oct 02 11:36:44 2014 -0700
+++ b/src/share/vm/runtime/deoptimization.cpp	Thu Oct 23 10:53:13 2014 +0200
@@ -1796,7 +1796,8 @@
   "div0_check",
   "age",
   "predicate",
-  "loop_limit_check"
+  "loop_limit_check",
+  "unstable_if"
 };
 const char* Deoptimization::_trap_action_name[Action_LIMIT] = {
   // Note:  Keep this in sync. with enum DeoptAction.
--- a/src/share/vm/runtime/deoptimization.hpp	Thu Oct 02 11:36:44 2014 -0700
+++ b/src/share/vm/runtime/deoptimization.hpp	Thu Oct 23 10:53:13 2014 +0200
@@ -59,6 +59,7 @@
     Reason_age,                   // nmethod too old; tier threshold reached
     Reason_predicate,             // compiler generated predicate failed
     Reason_loop_limit_check,      // compiler generated loop limits check failed
+    Reason_unstable_if,           // a branch predicted always false was taken
     Reason_LIMIT,
     // Note:  Keep this enum in sync. with _trap_reason_name.
     Reason_RECORDED_LIMIT = Reason_bimorphic  // some are not recorded per bc
@@ -311,6 +312,8 @@
       return reason;
     else if (reason == Reason_div0_check) // null check due to divide-by-zero?
       return Reason_null_check;           // recorded per BCI as a null check
+    else if (reason == Reason_unstable_if)
+      return Reason_intrinsic;
     else
       return Reason_none;
   }
--- a/src/share/vm/runtime/vmStructs.cpp	Thu Oct 02 11:36:44 2014 -0700
+++ b/src/share/vm/runtime/vmStructs.cpp	Thu Oct 23 10:53:13 2014 +0200
@@ -2432,6 +2432,7 @@
   declare_constant(Deoptimization::Reason_age)                            \
   declare_constant(Deoptimization::Reason_predicate)                      \
   declare_constant(Deoptimization::Reason_loop_limit_check)               \
+  declare_constant(Deoptimization::Reason_unstable_if)                    \
   declare_constant(Deoptimization::Reason_LIMIT)                          \
   declare_constant(Deoptimization::Reason_RECORDED_LIMIT)                 \
                                                                           \