RFR: 8240220: IdealLoopTree::dump_head predicate printing is broken

[Nils Eliasson]

+1

// Nils

On 2020-02-28 14:10, Tobias Hartmann wrote:
> Hi Stefan,
>
> good catch! Looks good to me.
>
> Best regards,
> Tobias
>
> On 28.02.20 11:49, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to fix the loop predicate printing in IdealLoopTree::dump_head.
>>
>> https://cr.openjdk.java.net/~stefank/8240220/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8240220
>>
>> In GraphKit::add_predicate you can see the order in which the predicates are added:
>>
>>      add_predicate_impl(Deoptimization::Reason_predicate, nargs);
>>      add_predicate_impl(Deoptimization::Reason_profile_predicate, nargs);
>>      add_predicate_impl(Deoptimization::Reason_loop_limit_check, nargs);
>>
>> When IdealLoopTree::dump_head looks for these predicates, the code has incorrectly swapped the
>> 'predicate' and 'profile_predicate':
>>
>> 2541   Node* predicate = PhaseIdealLoop::find_predicate_insertion_point(entry,
>> Deoptimization::Reason_loop_limit_check);
>> 2547     entry = PhaseIdealLoop::find_predicate_insertion_point(entry,
>> Deoptimization::Reason_predicate);
>> 2554     entry = PhaseIdealLoop::find_predicate_insertion_point(entry,
>> Deoptimization::Reason_profile_predicate);
>>
>> (Note that the predicates are added top-down, while dump_head walks them bottom-up)
>>
>> The code also uses wrong node when calling skip_loop_predicates. Compare the skipping of 'limit_check':
>>
>> 2541   Node* predicate = PhaseIdealLoop::find_predicate_insertion_point(entry,
>> Deoptimization::Reason_loop_limit_check);
>> 2542   if (predicate != NULL ) {
>> 2543     tty->print(" limit_check");
>> 2544     entry = PhaseIdealLoop::skip_loop_predicates(entry);
>>
>> and see how the skipping of 'predicate' sets entry on line 2547;
>> 2547     entry = PhaseIdealLoop::find_predicate_insertion_point(entry,
>> Deoptimization::Reason_predicate);
>> 2548     if (entry != NULL) {
>> 2549       tty->print(" predicated");
>> 2550       entry = PhaseIdealLoop::skip_loop_predicates(entry);
>> 2551     }
>>
>> Before the patch, the 'profile_predicated' and 'predicated' items were missing:
>>    Loop: N344/N304  limit_check counted [0,int),+1 (-1 iters)  has_sfpt
>>
>> After the patch, the lines look like this:
>>    Loop: N344/N304  limit_check profile_predicated predicated counted [0,int),+1 (-1 iters)  has_sfpt
>>
>> I've only tested this by visually inspecting the output. I don't know if there are any existing
>> tests for this kind of code in the Compiler.
>>
>> Thanks,
>> StefanK