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

[Roland Westrelin]

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