RFR(XS): 8211451: ~2.5% regression on compression benchmark starting with 12-b11

[Vladimir Kozlov]

On 11/7/18 4:30 AM, Roland Westrelin wrote:
> 
> Thanks for looking at the new webrev.
> 
>> I think code in insert_loop_limit_check() is not matching previous code:
>>
>> now:
>>
>> +  _igvn.replace_input_of(iff, 1, bol);
>>
>> before:
>>
>> -    _igvn.replace_input_of(opq, 1, bol);
>> -    // Update ctrl.
>> -    set_ctrl(opq, check_iff->in(0));
>> -    set_ctrl(check_iff->in(1), check_iff->in(0));
> 
> It's not matching the previous code indeed.
> 
> My understanding is that the previous code preserved the Conv2B/Opaque1
> pattern so the predicate is matched by ProjNode::is_uncommon_trap_if_pattern():
> 
>    // we need "If(Conv2B(Opaque1(...)))" pattern for reason_predicate
>    if (reason != Deoptimization::Reason_none) {
>      if (iff->in(1)->Opcode() != Op_Conv2B ||
>         iff->in(1)->in(1)->Opcode() != Op_Opaque1) {
>        return NULL;
>      }
>    }
> 
> The code that adds the loop limit check now uses
> create_new_if_for_predicate(). That code creates a new if and leaves the
> one with the Conv2B/Opaque1 added at parse time untouched. So I think
> it's fine to not keep the Conv2B/Opaque1 for the actual limit checks and
> follow what's done for other predicates.

Got it. Agree. I was concern about predicates copying code when loop is cloned. But we don't copy 
limit check predicate when loop is already converted to counted loop. I forgot why we don't clone it 
in such case :( (I read my comment in 5091921 but...).

Small note. Can you use ProjNode limit_check_proj for insert_loop_limit_check() calls instead of 
init_control.

Thanks,
Vladimir

> 
> Roland.
>