8030976: Untaken paths should be more vigorously pruned at highest optimization level
Reviewed-by: roland, vlivanov
--- a/hotspot/src/share/vm/oops/methodData.hpp Wed Jun 04 10:01:28 2014 +0200
+++ b/hotspot/src/share/vm/oops/methodData.hpp Thu May 22 13:42:44 2014 +0200
@@ -2053,7 +2053,7 @@
// 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
--- a/hotspot/src/share/vm/opto/parse2.cpp Wed Jun 04 10:01:28 2014 +0200
+++ b/hotspot/src/share/vm/opto/parse2.cpp Thu May 22 13:42:44 2014 +0200
@@ -897,53 +897,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--------------------------------
@@ -1182,32 +1141,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/hotspot/src/share/vm/runtime/deoptimization.cpp Wed Jun 04 10:01:28 2014 +0200
+++ b/hotspot/src/share/vm/runtime/deoptimization.cpp Thu May 22 13:42:44 2014 +0200
@@ -1840,6 +1840,7 @@
"speculate_class_check",
"speculate_null_check",
"rtm_state_change",
+ "unstable_if",
"tenured"
};
const char* Deoptimization::_trap_action_name[] = {
--- a/hotspot/src/share/vm/runtime/deoptimization.hpp Wed Jun 04 10:01:28 2014 +0200
+++ b/hotspot/src/share/vm/runtime/deoptimization.hpp Thu May 22 13:42:44 2014 +0200
@@ -62,6 +62,10 @@
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 @@
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;
}
--- a/hotspot/src/share/vm/runtime/vmStructs.cpp Wed Jun 04 10:01:28 2014 +0200
+++ b/hotspot/src/share/vm/runtime/vmStructs.cpp Thu May 22 13:42:44 2014 +0200
@@ -2504,6 +2504,7 @@
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) \