RFR(XS): 8030976: untaken paths should be more vigorously pruned at highest optimization level

[Rickard Bäckman]

Thank you Roland.

/R

On 06/05, Roland Westrelin wrote:
> > Updated: http://cr.openjdk.java.net/~rbackman/8030976.4/
> 
> That looks good to me.
> 
> Roland.
> 
> > 
> > Thanks
> > /R
> > 
> > On 06/03, Igor Veresov wrote:
> >> I didn’t add an entry in _trap_hist for Reason_tenured because it’s tracked by a different counter. Perhaps Reason_unstable_if can be moved before Reason_tenured so that _trap_hist_limit can be 21.
> >> 
> >> igor
> >> 
> >> On Jun 3, 2014, at 3:42 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
> >> 
> >>> 2 questions:
> >>> - why do you set _trap_hist_limit = 22 and not 21?
> >>> 
> >>> - why do you map Reason_unstable_if to Reason_intrinsic in reason_recorded_per_bytecode_if_any?
> >>> +     else if (reason == Reason_unstable_if)
> >>> +       return Reason_intrinsic;
> >>> 
> >>> 
> >>> Best regards,
> >>> Vladimir Ivanov
> >>> 
> >>> On 6/3/14 5:07 PM, Rickard Bäckman wrote:
> >>>> Updated: http://cr.openjdk.java.net/~rbackman/8030976.2/
> >>>> 
> >>>> Thanks
> >>>> /R
> >>>> 
> >>>> On 06/03, Roland Westrelin wrote:
> >>>>> Hi Rickard,
> >>>>> 
> >>>>>> Updated webrev: http://cr.openjdk.java.net/~rbackman/8030976.1/
> >>>>> 
> >>>>> You need to pass Deoptimization::Reason_unstable_if to uncommon_trap() in Parse::adjust_map_after_if(). Also the code below in Parse::adjust_map_after_if() is no longer needed AFAICT.
> >>>>> 
> >>>>> Roland.
> >>>>> 
> >>>>> 1185     // If this might possibly turn into an implicit null check,
> >>>>> 1186     // and the null has never yet been seen, we need to generate
> >>>>> 1187     // an uncommon trap, so as to recompile instead of suffering
> >>>>> 1188     // with very slow branches.  (We'll get the slow branches if
> >>>>> 1189     // the program ever changes phase and starts seeing nulls here.)
> >>>>> 1190     //
> >>>>> 1191     // We do not inspect for a null constant, since a node may
> >>>>> 1192     // optimize to 'null' later on.
> >>>>> 1193     //
> >>>>> 1194     // Null checks, and other tests which expect inequality,
> >>>>> 1195     // show btest == BoolTest::eq along the non-taken branch.
> >>>>> 1196     // On the other hand, type tests, must-be-null tests,
> >>>>> 1197     // and other tests which expect pointer equality,
> >>>>> 1198     // show btest == BoolTest::ne along the non-taken branch.
> >>>>> 1199     // We prune both types of branches if they look unused.
> >>>>> 1200     repush_if_args();
> >>>>> 1201     // We need to mark this branch as taken so that if we recompile we will
> >>>>> 1202     // see that it is possible. In the tiered system the interpreter doesn't
> >>>>> 1203     // do profiling and by the time we get to the lower tier from the interpreter
> >>>>> 1204     // the path may be cold again. Make sure it doesn't look untaken
> >>>>> 1205     if (is_fallthrough) {
> >>>>> 1206       profile_not_taken_branch(!ProfileInterpreter);
> >>>>> 1207     } else {
> >>>>> 1208       profile_taken_branch(iter().get_dest(), !ProfileInterpreter);
> >>>>> 1209     }
> >>>>> 
> >>>>>> 
> >>>>>> Thanks
> >>>>>> /R
> >>>>>> 
> >>>>>> On 05/22, Rickard Bäckman wrote:
> >>>>>>> Roland pointed out a problem with the Reason used. New webrev coming
> >>>>>>> shortly.
> >>>>>>> 
> >>>>>>> Thanks Roland.
> >>>>>>> 
> >>>>>>> On 05/22, Rickard Bäckman wrote:
> >>>>>>>> Hi all,
> >>>>>>>> 
> >>>>>>>> can I please have reviews for this change.
> >>>>>>>> The patch makes C2 place uncommon traps on previously untaken branches
> >>>>>>>> much more aggressively (we are simply trusting the profiling more).
> >>>>>>>> This improves performance for a couple of different patterns.
> >>>>>>>> 
> >>>>>>>> Example:
> >>>>>>>> 
> >>>>>>>> class Test {
> >>>>>>>> public int[] array = new int[] = { 1, 1, 2, 3, 5, 8, 13 };
> >>>>>>>> 
> >>>>>>>> public void some_method() {
> >>>>>>>>   for (int i = 0; i < array.length; i++) {
> >>>>>>>>     if (array[i] < 255) {
> >>>>>>>>       some_call();
> >>>>>>>>     } else {
> >>>>>>>>       some_other_call();
> >>>>>>>>     }
> >>>>>>>>   }
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>> 
> >>>>>>>> Where we previously if the else branch had never been taken rarely would
> >>>>>>>> inline the some_other_call and when array escapes we can't make
> >>>>>>>> assumptions on non-changing lengths, call killing registers, etc.
> >>>>>>>> 
> >>>>>>>> On some of the Nashorn benchmark this patch increases score by 35%,
> >>>>>>>> others don't see any change at all. No difference on SpecJBB 2005.
> >>>>>>>> More performance numbers / microbenchmark in the comments of the bug.
> >>>>>>>> 
> >>>>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/8030976/
> >>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8030976
> >>>>>>>> 
> >>>>>>>> Thanks
> >>>>>>>> /R
> >>>>>>> 
> >>>>>>> 
> >>>>>>> /R
> >>>>> 
> >> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140605/fb82308d/signature.asc>