[15] RFR(S): 8244724: CTW: C2 compilation fails with "Live Node limit exceeded limit"

[Christian Hagedorn]

Thank you Nils and Vladimir for your reviews!

On 27.06.20 00:35, Vladimir Kozlov wrote:
> You don't need to use 'C->' in Compile::Optimize() method:
> 
> DEBUG_ONLY(C->set_phase_optimize_finished();)

Oh, right!

> Yes, remove all nodes limit checks after optimization phase.

I created a new webrev with all those Compile::check_node_count() calls 
removed and the change above:
http://cr.openjdk.java.net/~chagedorn/8244724/webrev.02/

However, since these calls also affect bailout decisions in the product 
version and this bug is targeted for 15, I suggest to remove these 
calls/bailouts for 16 only in a separate RFE to minimize the risk.

Best regards,
Christian

> Thanks,
> Vladimir
> 
> On 6/26/20 7:38 AM, Christian Hagedorn wrote:
>> Hi
>>
>> Please review the following patch:
>> https://bugs.openjdk.java.net/browse/JDK-8244724
>> http://cr.openjdk.java.net/~chagedorn/8244724/webrev.01/
>>
>> The testcase contains many string concatinations. These are compiled 
>> by javac with -XDstringConcat=inline which creates a lot of 
>> StringBuilder objects and calls. As a result, we get a huge graph and 
>> eventually hit the live node limit assert during code generation when 
>> trying to create new nodes - either during PhaseCFG::build_cfg() or 
>> later in PhaseCFG::global_code_motion().
>>
>> We could try to introduce estimates for them to bailout but that 
>> appears to be difficult to get right without being too pessimistic 
>> about it. But we need to be in order to avoid hitting the assert again 
>> by just modifying the testcase.
>>
>> Therefore, my suggestion is to completely skip the assert once the 
>> optimization phase is finished as we should not strictly care about 
>> the node limit anymore at this point in time and it does not really 
>> provide much help for finding bugs.
>>
>> A question remains, though, if we should also get rid of the remaining 
>> live node limit bailout checks in Compile::Code_Gen() like [1] as it 
>> appears to be a waste to go through all the optimization in the 
>> optimization phase to then bailout while generating code based only on 
>> the live node limit itself. What do you think about that?
>>
>> I also updated "<" into "<=" in the live node limit assert because we 
>> should be allowed to reach the limit but not go beyond.
>>
>> Thank you!
>>
>> Best regards,
>> Christian
>>
>>
>> [1] 
>> http://hg.openjdk.java.net/jdk/jdk/file/8fd3e34e8379/src/hotspot/share/opto/coalesce.cpp#l236 
>>