From af4d5600e37ec6d331e62c5d37491ee97cad5311 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Thu, 23 Mar 2023 07:44:18 +0000 Subject: [PATCH] 8303951: Add asserts before record_method_not_compilable where possible Reviewed-by: kvn, thartmann --- src/hotspot/share/compiler/compileBroker.cpp | 3 ++- src/hotspot/share/opto/buildOopMap.cpp | 12 ++++++++++-- src/hotspot/share/opto/compile.cpp | 19 +++++++++++++++++-- src/hotspot/share/opto/domgraph.cpp | 1 + src/hotspot/share/opto/gcm.cpp | 2 ++ src/hotspot/share/opto/loopnode.cpp | 2 ++ src/hotspot/share/opto/matcher.cpp | 16 +++++++++++++++- src/hotspot/share/opto/output.cpp | 2 ++ src/hotspot/share/opto/parse1.cpp | 6 ++++++ src/hotspot/share/opto/reg_split.cpp | 1 + 10 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/compiler/compileBroker.cpp b/src/hotspot/share/compiler/compileBroker.cpp index cf0cd111448..7829b196cee 100644 --- a/src/hotspot/share/compiler/compileBroker.cpp +++ b/src/hotspot/share/compiler/compileBroker.cpp @@ -2277,7 +2277,8 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) { DirectivesStack::release(directive); if (!ci_env.failing() && !task->is_success()) { - //assert(false, "compiler should always document failure"); + assert(ci_env.failure_reason() != nullptr, "expect failure reason"); + assert(false, "compiler should always document failure: %s", ci_env.failure_reason()); // The compiler elected, without comment, not to register a result. // Do not attempt further compilations of this method. ci_env.record_method_not_compilable("compile failed"); diff --git a/src/hotspot/share/opto/buildOopMap.cpp b/src/hotspot/share/opto/buildOopMap.cpp index e51fa586c08..5b93e21b597 100644 --- a/src/hotspot/share/opto/buildOopMap.cpp +++ b/src/hotspot/share/opto/buildOopMap.cpp @@ -251,7 +251,11 @@ OopMap *OopFlow::build_oop_map( Node *n, int max_reg, PhaseRegAlloc *regalloc, i // Check for a legal reg name in the oopMap and bailout if it is not. if (!omap->legal_vm_reg_name(r)) { - regalloc->C->record_method_not_compilable("illegal oopMap register name"); + stringStream ss; + ss.print("illegal oopMap register name: "); + r->print_on(&ss); + assert(false, "%s", ss.as_string()); + regalloc->C->record_method_not_compilable(ss.as_string()); continue; } if( t->is_ptr()->_offset == 0 ) { // Not derived? @@ -318,7 +322,11 @@ OopMap *OopFlow::build_oop_map( Node *n, int max_reg, PhaseRegAlloc *regalloc, i assert( !OptoReg::is_valid(_callees[reg]), "oop can't be callee save" ); // Check for a legal reg name in the oopMap and bailout if it is not. if (!omap->legal_vm_reg_name(r)) { - regalloc->C->record_method_not_compilable("illegal oopMap register name"); + stringStream ss; + ss.print("illegal oopMap register name: "); + r->print_on(&ss); + assert(false, "%s", ss.as_string()); + regalloc->C->record_method_not_compilable(ss.as_string()); continue; } if( mcall ) { diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index bce89e4b007..ec3bd19eca9 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -753,7 +753,11 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, } if (failing()) return; if (cg == nullptr) { - record_method_not_compilable("cannot parse method"); + const char* reason = InlineTree::check_can_parse(method()); + assert(reason != nullptr, "expect reason for parse failure"); + stringStream ss; + ss.print("cannot parse method: %s", reason); + record_method_not_compilable(ss.as_string()); return; } @@ -762,7 +766,10 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, JVMState* jvms = build_start_state(start(), tf()); if ((jvms = cg->generate(jvms)) == nullptr) { if (!failure_reason_is(C2Compiler::retry_class_loading_during_parsing())) { - record_method_not_compilable("method parse failed"); + assert(failure_reason() != nullptr, "expect reason for parse failure"); + stringStream ss; + ss.print("method parse failed: %s", failure_reason()); + record_method_not_compilable(ss.as_string()); } return; } @@ -3922,6 +3929,8 @@ bool Compile::final_graph_reshaping() { // an infinite loop may have been eliminated by the optimizer, // in which case the graph will be empty. if (root()->req() == 1) { + // Do not compile method that is only a trivial infinite loop, + // since the content of the loop may have been eliminated. record_method_not_compilable("trivial infinite loop"); return true; } @@ -3987,8 +3996,11 @@ bool Compile::final_graph_reshaping() { } } } + // Recheck with a better notion of 'required_outcnt' if (n->outcnt() != required_outcnt) { + DEBUG_ONLY( n->dump_bfs(1, 0, "-"); ); + assert(false, "malformed control flow"); record_method_not_compilable("malformed control flow"); return true; // Not all targets reachable! } @@ -4007,6 +4019,9 @@ bool Compile::final_graph_reshaping() { // must be infinite loops. for (DUIterator_Fast jmax, j = n->fast_outs(jmax); j < jmax; j++) if (!frc._visited.test(n->fast_out(j)->_idx)) { + DEBUG_ONLY( n->fast_out(j)->dump(); ); + DEBUG_ONLY( n->dump_bfs(1, 0, "-"); ); + assert(false, "infinite loop"); record_method_not_compilable("infinite loop"); return true; // Found unvisited kid; must be unreach } diff --git a/src/hotspot/share/opto/domgraph.cpp b/src/hotspot/share/opto/domgraph.cpp index bd7b69fdf5b..db7181697e8 100644 --- a/src/hotspot/share/opto/domgraph.cpp +++ b/src/hotspot/share/opto/domgraph.cpp @@ -86,6 +86,7 @@ void PhaseCFG::build_dominator_tree() { // such dead loops (as was done for the NTarjan code farther below). // Since this situation is so unlikely, instead I've decided to bail out. // CNC 7/24/2001 + assert(false, "unreachable loop"); C->record_method_not_compilable("unreachable loop"); return; } diff --git a/src/hotspot/share/opto/gcm.cpp b/src/hotspot/share/opto/gcm.cpp index 95bb4f11472..23ae8cbcb6b 100644 --- a/src/hotspot/share/opto/gcm.cpp +++ b/src/hotspot/share/opto/gcm.cpp @@ -1500,6 +1500,7 @@ void PhaseCFG::global_code_motion() { Node_Stack stack((C->live_nodes() >> 2) + 16); // pre-grow if (!schedule_early(visited, stack)) { // Bailout without retry + assert(false, "early schedule failed"); C->record_method_not_compilable("early schedule failed"); return; } @@ -1600,6 +1601,7 @@ void PhaseCFG::global_code_motion() { Block* block = get_block(i); if (!schedule_local(block, ready_cnt, visited, recalc_pressure_nodes)) { if (!C->failure_reason_is(C2Compiler::retry_no_subsuming_loads())) { + assert(false, "local schedule failed"); C->record_method_not_compilable("local schedule failed"); } _regalloc = nullptr; diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index e2c009331df..afe6dd9499e 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -4310,6 +4310,7 @@ void PhaseIdealLoop::build_and_optimize() { if (!has_node(C->root())) { if (!_verify_only) { C->clear_major_progress(); + assert(false, "empty program detected during loop optimization"); C->record_method_not_compilable("empty program detected during loop optimization"); } return; @@ -5171,6 +5172,7 @@ int PhaseIdealLoop::build_loop_tree_impl( Node *n, int pre_order ) { m->dump(); } #endif + // This is a rare case that we do not want to handle in C2. C->record_method_not_compilable("unhandled CFG detected during loop optimization"); return pre_order; } diff --git a/src/hotspot/share/opto/matcher.cpp b/src/hotspot/share/opto/matcher.cpp index 039144f700b..c9ea54bde1f 100644 --- a/src/hotspot/share/opto/matcher.cpp +++ b/src/hotspot/share/opto/matcher.cpp @@ -142,6 +142,7 @@ OptoReg::Name Matcher::warp_incoming_stk_arg( VMReg reg ) { _in_arg_limit = OptoReg::add(warped, 1); // Bump max stack slot seen if (!RegMask::can_represent_arg(warped)) { // the compiler cannot represent this method's calling sequence + // Bailout. We do not have space to represent all arguments. C->record_method_not_compilable("unsupported incoming calling sequence"); return OptoReg::Bad; } @@ -311,6 +312,7 @@ void Matcher::match( ) { if (!RegMask::can_represent_arg(OptoReg::add(_out_arg_limit,-1))) { // the compiler cannot represent this method's calling sequence + // Bailout. We do not have space to represent all arguments. C->record_method_not_compilable("must be able to represent all call arguments in reg mask"); } @@ -358,6 +360,7 @@ void Matcher::match( ) { Node* xroot = xform( C->root(), 1 ); if (xroot == nullptr) { Matcher::soft_match_failure(); // recursive matching process failed + assert(false, "instruction match failed"); C->record_method_not_compilable("instruction match failed"); } else { // During matching shared constants were attached to C->root() @@ -389,7 +392,15 @@ void Matcher::match( ) { } } if (C->top() == nullptr || C->root() == nullptr) { - C->record_method_not_compilable("graph lost"); // %%% cannot happen? + // New graph lost. This is due to a compilation failure we encountered earlier. + stringStream ss; + if (C->failure_reason() != nullptr) { + ss.print("graph lost: %s", C->failure_reason()); + } else { + assert(C->failure_reason() != nullptr, "graph lost: reason unknown"); + ss.print("graph lost: reason unknown"); + } + C->record_method_not_compilable(ss.as_string()); } if (C->failing()) { // delete old; @@ -1242,6 +1253,7 @@ OptoReg::Name Matcher::warp_outgoing_stk_arg( VMReg reg, OptoReg::Name begin_out if( warped >= out_arg_limit_per_call ) out_arg_limit_per_call = OptoReg::add(warped,1); if (!RegMask::can_represent_arg(warped)) { + // Bailout. For example not enough space on stack for all arguments. Happens for methods with too many arguments. C->record_method_not_compilable("unsupported calling sequence"); return OptoReg::Bad; } @@ -1434,6 +1446,7 @@ MachNode *Matcher::match_sfpt( SafePointNode *sfpt ) { uint r_cnt = mcall->tf()->range()->cnt(); MachProjNode *proj = new MachProjNode( mcall, r_cnt+10000, RegMask::Empty, MachProjNode::fat_proj ); if (!RegMask::can_represent_arg(OptoReg::Name(out_arg_limit_per_call-1))) { + // Bailout. We do not have space to represent all arguments. C->record_method_not_compilable("unsupported outgoing calling sequence"); } else { for (int i = begin_out_arg_area; i < out_arg_limit_per_call; i++) @@ -1621,6 +1634,7 @@ Node* Matcher::Label_Root(const Node* n, State* svec, Node* control, Node*& mem) // out of stack space. See bugs 6272980 & 6227033 for more info. LabelRootDepth++; if (LabelRootDepth > MaxLabelRootDepth) { + // Bailout. Can for example be hit with a deep chain of operations. C->record_method_not_compilable("Out of stack space, increase MaxLabelRootDepth"); return nullptr; } diff --git a/src/hotspot/share/opto/output.cpp b/src/hotspot/share/opto/output.cpp index 230c6d889b8..5539eb5724c 100644 --- a/src/hotspot/share/opto/output.cpp +++ b/src/hotspot/share/opto/output.cpp @@ -2849,6 +2849,8 @@ void Scheduling::anti_do_def( Block *b, Node *def, OptoReg::Name def_reg, int is pinch = new Node(1); // Pinch point to-be } if (pinch->_idx >= _regalloc->node_regs_max_index()) { + DEBUG_ONLY( pinch->dump(); ); + assert(false, "too many D-U pinch points: %d >= %d", pinch->_idx, _regalloc->node_regs_max_index()); _cfg->C->record_method_not_compilable("too many D-U pinch points"); return; } diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp index a17fe66ee02..414a1f3c00e 100644 --- a/src/hotspot/share/opto/parse1.cpp +++ b/src/hotspot/share/opto/parse1.cpp @@ -213,6 +213,7 @@ void Parse::load_interpreter_state(Node* osr_buf) { } // Do not OSR inside finally clauses: if (osr_block->has_trap_at(osr_block->start())) { + assert(false, "OSR starts with an immediate trap"); C->record_method_not_compilable("OSR starts with an immediate trap"); return; } @@ -253,6 +254,7 @@ void Parse::load_interpreter_state(Node* osr_buf) { MethodLivenessResult live_locals = method()->liveness_at_bci(osr_bci()); if (!live_locals.is_valid()) { // Degenerate or breakpointed method. + assert(false, "OSR in empty or breakpointed method"); C->record_method_not_compilable("OSR in empty or breakpointed method"); return; } @@ -429,6 +431,7 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses) _iter.reset_to_method(method()); _flow = method()->get_flow_analysis(); if (_flow->failing()) { + assert(false, "type flow failed during parsing"); C->record_method_not_compilable(_flow->failure_reason()); } @@ -507,6 +510,7 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses) _entry_bci = C->entry_bci(); _flow = method()->get_osr_flow_analysis(osr_bci()); if (_flow->failing()) { + assert(false, "type flow analysis failed for OSR compilation"); C->record_method_not_compilable(_flow->failure_reason()); #ifndef PRODUCT if (PrintOpto && (Verbose || WizardMode)) { @@ -1044,6 +1048,7 @@ void Parse::do_exits() { // loading. It could also be due to an error, so mark this method as not compilable because // otherwise this could lead to an infinite compile loop. // In any case, this code path is rarely (and never in my testing) reached. + assert(false, "Can't determine return type."); C->record_method_not_compilable("Can't determine return type."); return; } @@ -1119,6 +1124,7 @@ SafePointNode* Parse::create_entry_map() { // Check for really stupid bail-out cases. uint len = TypeFunc::Parms + method()->max_locals() + method()->max_stack(); if (len >= 32760) { + // Bailout expected, this is a very rare edge case. C->record_method_not_compilable("too many local variables"); return nullptr; } diff --git a/src/hotspot/share/opto/reg_split.cpp b/src/hotspot/share/opto/reg_split.cpp index 27ba9838a89..f1394788ebe 100644 --- a/src/hotspot/share/opto/reg_split.cpp +++ b/src/hotspot/share/opto/reg_split.cpp @@ -306,6 +306,7 @@ Node* clone_node(Node* def, Block *b, Compile* C) { C->record_failure(C2Compiler::retry_no_subsuming_loads()); } else { // Bailout without retry + assert(false, "RA Split failed: attempt to clone node with anti_dependence"); C->record_method_not_compilable("RA Split failed: attempt to clone node with anti_dependence"); } return 0;