Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8286104: use aggressive liveness for unstable_if traps #8545

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
37de8fe
8286104: use aggressive liveness for unstable_if traps
May 5, 2022
2c38b87
8286104: use aggressive liveness for unstable_if traps
May 5, 2022
c482800
retain bci for unstable_if
May 8, 2022
2f650bf
disable comparison_folding temporarily.
May 8, 2022
8d9ba30
adjust unstable_if after fold_compares
May 9, 2022
b4ceec5
update unstable_if after igvn.
May 10, 2022
77ee4db
fix release build
May 10, 2022
89fa3ce
add sanity check for operands if bc is if_acmp_eq/ne and ifnull/nonnull
May 11, 2022
5e689d4
change the flag to diagnostic.
May 11, 2022
4f75dda
rule out if a If nodes has 2 branches of unstable_if trap.
May 12, 2022
1b5cb89
Merge branch 'JDK-8276998' into JDK-8286104
May 12, 2022
2f04745
revert code change from 1st revision.
May 12, 2022
c3f0cbc
bail out a corner case that ifnode postpones fold-compares after loop
May 20, 2022
1a2d048
Merge branch 'master' into JDK-8286104
May 20, 2022
80b9e44
remember UnstableIfTrap in parser.
May 24, 2022
3eae8c5
reimplement process_unstable_ifs
May 25, 2022
849cdec
Merge branch 'master' into JDK-8286104
May 25, 2022
7c305a4
update comments.
May 26, 2022
a30e0ef
Merge branch 'master' into JDK-8286104
May 26, 2022
4fdd1c8
support option AggressiveLivessForUnstableIf
May 26, 2022
6e9f267
Remove useless flag. if jdwp is on, liveness_at_bci() marks all local
Jun 1, 2022
d7e5f06
Refactor per reviewer's feedback.
Jun 2, 2022
f6771d6
move preprocess() after remove Useless.
Jun 2, 2022
428aeaf
Remame all methods to _unstable_if_trap(s) and group them.
Jun 2, 2022
4130cd1
Merge branch 'master' into JDK-8286104
Jun 2, 2022
9c91737
Bail out if fold-compares sees that a unstable_if trap has modified.
Jun 5, 2022
81a8ccf
monior change for code style.
Jun 6, 2022
481b66c
Merge branch 'master' into JDK-8286104
Jun 20, 2022
e5c8e55
update per reviewer's feedback.
Jun 21, 2022
49bcc41
remove _path from UnstableIfTrap. remember _next_bci(int) is enough.
Jun 23, 2022
ed20a82
restore the option OptimizeUnstableIf to diagnostic.
Jun 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/compiler/methodLiveness.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class MethodLivenessResult : public ResourceBitMap {
{}

void set_is_valid() { _is_valid = true; }
bool is_valid() { return _is_valid; }
bool is_valid() const { return _is_valid; }
};

class MethodLiveness : public ResourceObj {
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/opto/c2_globals.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,9 @@
"Set level of loop optimization for tier 1 compiles") \
range(5, 43) \
\
develop(bool, OptimizeUnstableIf, true, \
"Optimize UnstableIf traps") \
navyxliu marked this conversation as resolved.
Show resolved Hide resolved
\
/* controls for heat-based inlining */ \
\
develop(intx, NodeCountInliningCutoff, 18000, \
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/opto/callnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,12 @@ Node *SafePointNode::peek_monitor_obj() const {
return monitor_obj(jvms(), mon);
}

Node* SafePointNode::peek_operand(uint off) const {
assert(jvms()->sp() > 0, "must have an operand");
assert(off < jvms()->sp(), "off is out-of-range");
return stack(jvms(), jvms()->sp() - off - 1);
}

// Do we Match on this edge index or not? Match no edges
uint SafePointNode::match_edge(uint idx) const {
return (TypeFunc::Parms == idx);
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/callnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,8 @@ class SafePointNode : public MultiNode {
void pop_monitor ();
Node *peek_monitor_box() const;
Node *peek_monitor_obj() const;
// Peek Operand Stacks, JVMS 2.6.2
Node* peek_operand(uint off = 0) const;

// Access functions for the JVM
Node *control () const { return in(TypeFunc::Control ); }
Expand Down
109 changes: 109 additions & 0 deletions src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ void Compile::remove_useless_node(Node* dead) {
remove_useless_late_inlines( &_string_late_inlines, dead);
remove_useless_late_inlines( &_boxing_late_inlines, dead);
remove_useless_late_inlines(&_vector_reboxing_late_inlines, dead);

if (dead->is_CallStaticJava()) {
remove_unstable_if_trap(dead->as_CallStaticJava(), false);
}
}
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
bs->unregister_potential_barrier_node(dead);
Expand Down Expand Up @@ -434,6 +438,7 @@ void Compile::remove_useless_nodes(Unique_Node_List &useful) {
remove_useless_nodes(_skeleton_predicate_opaqs, useful);
remove_useless_nodes(_expensive_nodes, useful); // remove useless expensive nodes
remove_useless_nodes(_for_post_loop_igvn, useful); // remove useless node recorded for post loop opts IGVN pass
remove_useless_unstable_if_traps(useful); // remove useless unstable_if traps
remove_useless_coarsened_locks(useful); // remove useless coarsened locks nodes

BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
Expand Down Expand Up @@ -607,6 +612,7 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci,
_skeleton_predicate_opaqs (comp_arena(), 8, 0, NULL),
_expensive_nodes (comp_arena(), 8, 0, NULL),
_for_post_loop_igvn(comp_arena(), 8, 0, NULL),
_unstable_if_traps (comp_arena(), 8, 0, NULL),
_coarsened_locks (comp_arena(), 8, 0, NULL),
_congraph(NULL),
NOT_PRODUCT(_igv_printer(NULL) COMMA)
Expand Down Expand Up @@ -1854,6 +1860,107 @@ void Compile::process_for_post_loop_opts_igvn(PhaseIterGVN& igvn) {
}
}

void Compile::record_unstable_if_trap(UnstableIfTrap* trap) {
if (OptimizeUnstableIf) {
_unstable_if_traps.append(trap);
}
}

void Compile::remove_useless_unstable_if_traps(Unique_Node_List& useful) {
for (int i = _unstable_if_traps.length() - 1; i >= 0; i--) {
UnstableIfTrap* trap = _unstable_if_traps.at(i);
Node* n = trap->uncommon_trap();
if (!useful.member(n)) {
_unstable_if_traps.delete_at(i); // replaces i-th with last element which is known to be useful (already processed)
}
}
}
//
// Remove unstable_if trap of unc from candicates. It is either dead or fold-compares case.
// Return true if succeed or not found.
//
// In rare cases, the found traps have been processed. It is too late to delete. return false
// and ask fold-compares to yield.
//
// 'fold-compares' may use the uncommon_trap of the dominating IfNode to cover the fused
// IfNode. This breaks the unstable_if trap invariant: control takes the unstable path
// when deoptimization does happen.
//
bool Compile::remove_unstable_if_trap(CallStaticJavaNode* unc, bool yield) {
for (int i = 0; i < _unstable_if_traps.length(); ++i) {
UnstableIfTrap* trap = _unstable_if_traps.at(i);
if (trap->uncommon_trap() == unc) {
if (yield && trap->modified()) {
return false;
}
_unstable_if_traps.delete_at(i);
break;
}
}
return true;
}

// Re-calculate unstable_if traps with the liveness of next_bci, which points to the unlikely path.
// It needs to be done after igvn because fold-compares may fuse uncommon_traps and before renumbering.
void Compile::process_for_unstable_if_traps(PhaseIterGVN& igvn) {
for (int i = _unstable_if_traps.length() - 1; i >= 0; --i) {
UnstableIfTrap* trap = _unstable_if_traps.at(i);
CallStaticJavaNode* unc = trap->uncommon_trap();
int next_bci = trap->next_bci();
bool modified = trap->modified();

if (next_bci != -1 && !modified) {
assert(!_dead_node_list.test(unc->_idx), "changing a dead node!");
JVMState* jvms = unc->jvms();
ciMethod* method = jvms->method();
ciBytecodeStream iter(method);

iter.force_bci(jvms->bci());
assert(next_bci == iter.next_bci() || next_bci == iter.get_dest(), "wrong next_bci at unstable_if");
Bytecodes::Code c = iter.cur_bc();
Node* lhs = nullptr;
Node* rhs = nullptr;
if (c == Bytecodes::_if_acmpeq || c == Bytecodes::_if_acmpne) {
lhs = unc->peek_operand(0);
rhs = unc->peek_operand(1);
} else if (c == Bytecodes::_ifnull || c == Bytecodes::_ifnonnull) {
lhs = unc->peek_operand(0);
}

ResourceMark rm;
const MethodLivenessResult& live_locals = method->liveness_at_bci(next_bci);
assert(live_locals.is_valid(), "broken liveness info");
int len = (int)live_locals.size();

for (int i = 0; i < len; i++) {
Node* local = unc->local(jvms, i);
// kill local using the liveness of next_bci.
// give up when the local looks like an operand to secure reexecution.
if (!live_locals.at(i) && !local->is_top() && local != lhs && local!= rhs) {
uint idx = jvms->locoff() + i;
#ifdef ASSERT
if (Verbose) {
tty->print("[unstable_if] kill local#%d: ", idx);
local->dump();
tty->cr();
}
#endif
igvn.replace_input_of(unc, idx, top());
modified = true;
}
}
}

// keep the mondified trap for late query
if (modified) {
trap->set_modified();
} else {
_unstable_if_traps.delete_at(i);
}
}
igvn.optimize();
}

// StringOpts and late inlining of string methods
void Compile::inline_string_calls(bool parse_time) {
{
Expand Down Expand Up @@ -2138,6 +2245,8 @@ void Compile::Optimize() {

print_method(PHASE_ITER_GVN1, 2);

process_for_unstable_if_traps(igvn);

inline_incrementally(igvn);

print_method(PHASE_INCREMENTAL_INLINE, 2);
Expand Down
8 changes: 8 additions & 0 deletions src/hotspot/share/opto/compile.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class AddPNode;
class Block;
class Bundle;
class CallGenerator;
class CallStaticJavaNode;
class CloneMap;
class ConnectionGraph;
class IdealGraphPrinter;
Expand Down Expand Up @@ -90,6 +91,7 @@ class TypeOopPtr;
class TypeFunc;
class TypeVect;
class Unique_Node_List;
class UnstableIfTrap;
class nmethod;
class Node_Stack;
struct Final_Reshape_Counts;
Expand Down Expand Up @@ -357,6 +359,7 @@ class Compile : public Phase {
GrowableArray<Node*> _skeleton_predicate_opaqs; // List of Opaque4 nodes for the loop skeleton predicates.
GrowableArray<Node*> _expensive_nodes; // List of nodes that are expensive to compute and that we'd better not let the GVN freely common
GrowableArray<Node*> _for_post_loop_igvn; // List of nodes for IGVN after loop opts are over
GrowableArray<UnstableIfTrap*> _unstable_if_traps; // List of ifnodes after IGVN
GrowableArray<Node_List*> _coarsened_locks; // List of coarsened Lock and Unlock nodes
ConnectionGraph* _congraph;
#ifndef PRODUCT
Expand Down Expand Up @@ -732,6 +735,11 @@ class Compile : public Phase {
void remove_from_post_loop_opts_igvn(Node* n);
void process_for_post_loop_opts_igvn(PhaseIterGVN& igvn);

void record_unstable_if_trap(UnstableIfTrap* trap);
bool remove_unstable_if_trap(CallStaticJavaNode* unc, bool yield);
void remove_useless_unstable_if_traps(Unique_Node_List &useful);
void process_for_unstable_if_traps(PhaseIterGVN& igvn);

void sort_macro_nodes();

// remove the opaque nodes that protect the predicates so that the unused checks and
Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2018,12 +2018,12 @@ void GraphKit::increment_counter(Node* counter_addr) {
// Bail out to the interpreter in mid-method. Implemented by calling the
// uncommon_trap blob. This helper function inserts a runtime call with the
// right debug info.
void GraphKit::uncommon_trap(int trap_request,
Node* GraphKit::uncommon_trap(int trap_request,
ciKlass* klass, const char* comment,
bool must_throw,
bool keep_exact_action) {
if (failing()) stop();
if (stopped()) return; // trap reachable?
if (stopped()) return NULL; // trap reachable?

// Note: If ProfileTraps is true, and if a deopt. actually
// occurs here, the runtime will make sure an MDO exists. There is
Expand Down Expand Up @@ -2139,6 +2139,7 @@ void GraphKit::uncommon_trap(int trap_request,
root()->add_req(halt);

stop_and_kill_map();
return call;
}


Expand Down
10 changes: 5 additions & 5 deletions src/hotspot/share/opto/graphKit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -728,25 +728,25 @@ class GraphKit : public Phase {
// The optional klass is the one causing the trap.
// The optional reason is debug information written to the compile log.
// Optional must_throw is the same as with add_safepoint_edges.
void uncommon_trap(int trap_request,
Node* uncommon_trap(int trap_request,
ciKlass* klass = NULL, const char* reason_string = NULL,
bool must_throw = false, bool keep_exact_action = false);

// Shorthand, to avoid saying "Deoptimization::" so many times.
void uncommon_trap(Deoptimization::DeoptReason reason,
Node* uncommon_trap(Deoptimization::DeoptReason reason,
Deoptimization::DeoptAction action,
ciKlass* klass = NULL, const char* reason_string = NULL,
bool must_throw = false, bool keep_exact_action = false) {
uncommon_trap(Deoptimization::make_trap_request(reason, action),
return uncommon_trap(Deoptimization::make_trap_request(reason, action),
klass, reason_string, must_throw, keep_exact_action);
}

// Bail out to the interpreter and keep exact action (avoid switching to Action_none).
void uncommon_trap_exact(Deoptimization::DeoptReason reason,
Node* uncommon_trap_exact(Deoptimization::DeoptReason reason,
Deoptimization::DeoptAction action,
ciKlass* klass = NULL, const char* reason_string = NULL,
bool must_throw = false) {
uncommon_trap(Deoptimization::make_trap_request(reason, action),
return uncommon_trap(Deoptimization::make_trap_request(reason, action),
klass, reason_string, must_throw, /*keep_exact_action=*/true);
}

Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/opto/ifnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,8 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod
ciMethod* dom_method = dom_unc->jvms()->method();
int dom_bci = dom_unc->jvms()->bci();
if (!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_unstable_fused_if) &&
!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check)) {
!igvn->C->too_many_traps(dom_method, dom_bci, Deoptimization::Reason_range_check) &&
igvn->C->remove_unstable_if_trap(dom_unc, true)) {
navyxliu marked this conversation as resolved.
Show resolved Hide resolved
success = unc_proj;
fail = unc_proj->other_if_proj();
return true;
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/opto/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,10 @@ void Node::destruct(PhaseValues* phase) {

if (is_SafePoint()) {
as_SafePoint()->delete_replaced_nodes();

if (is_CallStaticJava()) {
compile->remove_unstable_if_trap(as_CallStaticJava(), false);
}
}
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
bs->unregister_potential_barrier_node(this);
Expand Down
39 changes: 39 additions & 0 deletions src/hotspot/share/opto/parse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,4 +603,43 @@ class Parse : public GraphKit {
#endif
};

// Specialized uncommon_trap of unstable_if, we have 2 optimizations for them:
// 1. suppress trivial Unstable_If traps
navyxliu marked this conversation as resolved.
Show resolved Hide resolved
// 2. use next_bci of path to update live locals.
class UnstableIfTrap {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving this information into CallStaticJavaNode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CallStaticJavaNode is popular. uncommon_trap/unstable_if is just a special case. that's why factor out and use a dedicated class for it.

CallStaticJavaNode* const _unc;
bool _modified; // modified locals based on next_bci()
int _next_bci;

public:
UnstableIfTrap(CallStaticJavaNode* call, Parse::Block* path): _unc(call), _modified(false) {
assert(_unc != NULL && Deoptimization::trap_request_reason(_unc->uncommon_trap_request()) == Deoptimization::Reason_unstable_if,
"invalid uncommon_trap call!");
_next_bci = path != nullptr ? path->start() : -1;
}

// The starting point of the pruned block, where control goes when
// deoptimization does happen.
int next_bci() const {
return _next_bci;
}

bool modified() const {
return _modified;
}

void set_modified() {
_modified = true;
}

CallStaticJavaNode* uncommon_trap() const {
return _unc;
}

inline void* operator new(size_t x) throw() {
Compile* C = Compile::current();
return C->comp_arena()->AmallocWords(x);
}
};

#endif // SHARE_OPTO_PARSE_HPP
6 changes: 5 additions & 1 deletion src/hotspot/share/opto/parse2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1586,10 +1586,14 @@ void Parse::adjust_map_after_if(BoolTest::mask btest, Node* c, float prob,

if (path_is_suitable_for_uncommon_trap(prob)) {
repush_if_args();
uncommon_trap(Deoptimization::Reason_unstable_if,
Node* call = uncommon_trap(Deoptimization::Reason_unstable_if,
Deoptimization::Action_reinterpret,
NULL,
(is_fallthrough ? "taken always" : "taken never"));

if (call != nullptr) {
C->record_unstable_if_trap(new UnstableIfTrap(call->as_CallStaticJava(), path));
}
return;
}

Expand Down
Loading