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

[Tobias Hartmann]

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