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

[Vladimir Ivanov]

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
>>