8030976: Untaken paths should be more vigorously pruned at highest optimization level

Reviewed-by: roland, vlivanov

hotspot/src/share/vm/oops/methodData.hpp

@@ -2053,7 +2053,7 @@ public:
 
   // Whole-method sticky bits and flags
   enum {
-    _trap_hist_limit    = 20,   // decoupled from Deoptimization::Reason_LIMIT
+    _trap_hist_limit    = 21,   // decoupled from Deoptimization::Reason_LIMIT
     _trap_hist_mask     = max_jubyte,
     _extra_data_count   = 4     // extra DataLayout headers, for trap history
   }; // Public flag values

hotspot/src/share/vm/opto/parse2.cpp

@@ -897,53 +897,12 @@ bool Parse::seems_never_taken(float prob) {
 // 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
+// on the untaken path.
-// partially executed counted loops to be compiled fully,
-// we will plant uncommon traps only after pointer comparisons.
 bool Parse::seems_stable_comparison(BoolTest::mask btest, Node* cmp) {
-  for (int depth = 4; depth > 0; depth--) {
+  if (C->too_many_traps(method(), bci(), Deoptimization::Reason_unstable_if)) {
-    // The following switch can find CmpP here over half the time for
+    return false;
-    // 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;
-    }
   }
-  return false;
+  return true;
 }
 
 //-------------------------------repush_if_args--------------------------------
@@ -1182,32 +1141,8 @@ void Parse::adjust_map_after_if(BoolTest::mask btest, Node* c, float prob,
   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
+    uncommon_trap(Deoptimization::Reason_unstable_if,
-    // 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,
                   Deoptimization::Action_reinterpret,
                   NULL,
                   (is_fallthrough ? "taken always" : "taken never"));

hotspot/src/share/vm/runtime/deoptimization.cpp

@@ -1840,6 +1840,7 @@ const char* Deoptimization::_trap_reason_name[] = {
   "speculate_class_check",
   "speculate_null_check",
   "rtm_state_change",
+  "unstable_if",
   "tenured"
 };
 const char* Deoptimization::_trap_action_name[] = {

hotspot/src/share/vm/runtime/deoptimization.hpp

@@ -62,6 +62,10 @@ class Deoptimization : AllStatic {
     Reason_speculate_class_check, // saw unexpected object class from type speculation
     Reason_speculate_null_check,  // saw unexpected null from type speculation
     Reason_rtm_state_change,      // rtm state change detected
+    Reason_unstable_if,           // a branch predicted always false was taken
+
+    // Reason_tenured is counted separately, add normal counted Reasons above.
+    // Related to MethodData::_trap_hist_limit where Reason_tenured isn't included
     Reason_tenured,               // age of the code has reached the limit
     Reason_LIMIT,
     // Note:  Keep this enum in sync. with _trap_reason_name.
@@ -319,6 +323,8 @@ class Deoptimization : AllStatic {
       return Reason_class_check;
     else if (reason == Reason_speculate_null_check)
       return Reason_null_check;
+    else if (reason == Reason_unstable_if)
+      return Reason_intrinsic;
     else
       return Reason_none;
   }

hotspot/src/share/vm/runtime/vmStructs.cpp

@@ -2504,6 +2504,7 @@ typedef TwoOopHashtable<Symbol*, mtClass>     SymbolTwoOopHashtable;
   declare_constant(Deoptimization::Reason_speculate_class_check)          \
   declare_constant(Deoptimization::Reason_speculate_null_check)           \
   declare_constant(Deoptimization::Reason_rtm_state_change)               \
+  declare_constant(Deoptimization::Reason_unstable_if)                    \
   declare_constant(Deoptimization::Reason_tenured)                        \
   declare_constant(Deoptimization::Reason_LIMIT)                          \
   declare_constant(Deoptimization::Reason_RECORDED_LIMIT)                 \