RFR: 8303951: Add asserts before record_method_not_compilable where possible

[Vladimir Kozlov]

On Wed, 15 Mar 2023 10:29:32 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> I went through all `C2` bailouts, and checked if they are justified to bail out of compilation silently. I added asserts everywhere. Those that were hit, I inspected by hand.
> 
> Some of them seem to be justified. There I added comments why they are justified. They are cases that we do not want to handle in `C2`, and that are rare enough so that it probably does not matter.
> 
> For the following bailouts I did not add an assert, because it may have revealed a bug:
> [JDK-8304328](https://bugs.openjdk.org/browse/JDK-8304328) C2 Bailout "failed spill-split-recycle sanity check" reveals hidden issue with RA
> 
> Note:
> [JDK-8303466](https://bugs.openjdk.org/browse/JDK-8303466) C2: COMPILE SKIPPED: malformed control flow - only one IfProj
> That bug bug was the reason for this RFE here. I added the assert for "malformed control flow". After this RFE here, that Bug will run into the assert on debug builds.
> 
> I ran `tier1-6` and stress testing. Now running `tier7-9`.
> 
> Should we file a follow-up RFE to do the same for `BAILOUT` in `C1`?

General suggestion is to print more info on bailouts to help debugging.

> Should we file a follow-up RFE to do the same for BAILOUT in C1?

Yes

src/hotspot/share/compiler/compileBroker.cpp line 2280:

> 2278: 
> 2279:     if (!ci_env.failing() && !task->is_success()) {
> 2280:       assert(false, "compiler should always document failure");

I suggest to add `ci_env.failure_reason()` to assert message.

src/hotspot/share/opto/buildOopMap.cpp line 254:

> 252:       // Check for a legal reg name in the oopMap and bailout if it is not.
> 253:       if (!omap->legal_vm_reg_name(r)) {
> 254:         assert(false, "illegal oopMap register name");

Added information about `r` to assert message.

src/hotspot/share/opto/buildOopMap.cpp line 322:

> 320:       // Check for a legal reg name in the oopMap and bailout if it is not.
> 321:       if (!omap->legal_vm_reg_name(r)) {
> 322:         assert(false, "illegal oopMap register name");

Same.

src/hotspot/share/opto/compile.cpp line 757:

> 755:     if (cg == nullptr) {
> 756:       const char* reason = InlineTree::check_can_parse(method());
> 757:       assert(reason != nullptr, "cannot parse method: why?");

Add `reason` to assert's message.

src/hotspot/share/opto/compile.cpp line 769:

> 767:     if ((jvms = cg->generate(jvms)) == nullptr) {
> 768:       if (!failure_reason_is(C2Compiler::retry_class_loading_during_parsing())) {
> 769:         assert(failure_reason() != nullptr, "method parse failed: why?");

same

src/hotspot/share/opto/compile.cpp line 4001:

> 3999:       // Recheck with a better notion of 'required_outcnt'
> 4000:       if (n->outcnt() != required_outcnt) {
> 4001:         assert(false, "malformed control flow");

Print more info about `n`

src/hotspot/share/opto/compile.cpp line 4020:

> 4018:     for (DUIterator_Fast jmax, j = n->fast_outs(jmax); j < jmax; j++)
> 4019:       if (!frc._visited.test(n->fast_out(j)->_idx)) {
> 4020:         assert(false, "infinite loop");

Print more info about n

src/hotspot/share/opto/output.cpp line 2847:

> 2845:     }
> 2846:     if (pinch->_idx >= _regalloc->node_regs_max_index()) {
> 2847:       assert(false, "too many D-U pinch points");

More info about `pinch` node.

-------------

PR: https://git.openjdk.org/jdk/pull/13038