review(M): 7057120: Tiered: Allow C1 to inline methods with loops

[Igor Veresov]

Thanks for the review, Vladimir! Webrev updated. Please find the replies 
inline.

On 6/22/11 9:51 AM, Vladimir Kozlov wrote:
> Can you explain why in counter_overflow_helper() (c1_Runtime1.cpp) you
> expect the caller (enclosing_method) to be nmethod?

It's not the caller. The enclosing_method is the actual method to which 
the nmethod belongs. In other words: enclosing_method is a method who's 
frame is current on stack and "method" is possibly the inlinee inside 
the enclosing method that generated the event.

>
> Could you rename should_not_inline() to should_not_inline_for_profile()
> or something since it is only for C1 compilation with profiling?
>

I'd rather leave it generalized if you're not totally against it. We can 
use it later as a wrapper for oracle to control inlining in both compilers.

> Add {} for else:
>
> + } else
> + if (next_level != CompLevel_full_optimization) {
> + // next_level is not full opt, so we need to recompile the
> + // enclosing method without the inlinee
> + cur_level = CompLevel_none;
> + make_not_entrant = true;
> + }
> + if (make_not_entrant) {
>

Ok.

> I don't understand why you need to recompile caller. It comes from a
> different frame so it did not inline callee before :
>
> + if (max_osr_level == CompLevel_full_optimization) {
> + // The inlinee OSRed to full opt, we need to modify the enclosing
> method to avoid deopts
>

If we're are in this clause (that is we passed the mh() != imh() check) 
that means that the event was generated by the imh() that was inlined in 
mh(). So, we need to recompile mh() because otherwise we would be still 
executing the inline version of imh() and would be deoptimizing and 
switching to the full opt version.

> Also next check should be at the beginning of this logic since "Fix up"
> could trigger unneeded recompilation:
>
> + if (!CompileBroker::compilation_is_in_queue(mh, InvocationEntryBci) &&
> cur_level != next_level) {

But that would mean that we won't be able to make mh() non-entrant in 
case mh() is in the queue already. I move the fix up down since the code 
that makes the enclosing method non-entrant doesn't depend on it.


igor

>
> Vladimir
>
> Igor Veresov wrote:
>> This allows inlining of methods with loops by C1 on levels with
>> profiling (2 and 3). The solution is to recompile the enclosing
>> methods without inlining of the method that has OSRed to level 4 or
>> recompile the enclosing method at level 4.
>>
>> There's no significant impact on performance because of these changes.
>>
>> Webrev: http://cr.openjdk.java.net/~iveresov/7057120/webrev.00/
>>
>> The change is for 7u2.
>>
>> Thanks,
>> igor