[9] RFR(S): 8072398: assert fails in L1RGenerator::increment_event_counter_impl

[Igor Veresov]

Looks good.

igor

> On Feb 6, 2015, at 2:56 AM, Zoltán Majó <zoltan.majo at oracle.com> wrote:
> 
> Hi,
> 
> 
> the previous webrev excluded a one-line change that is needed to make this work.
> 
> Here is the new webrev: http://cr.openjdk.java.net/~zmajo/8072398/webrev.01/
> 
> The new change is in arguments.cpp at
> 
> *** 3919,3929 ****
> --- 3925,3936 ----
> 
> This change is analogous to the change at
> 
> *** 1202,1213 ****
> --- 1207,1219 ----
> 
> in the previous webrev.
> 
> Sorry for the extra effort.
> 
> Thank you and best regards,
> 
> 
> Zoltan
> 
> 
> 
> On 02/06/2015 10:57 AM, Zoltán Majó wrote:
>> Thank you, Vladimir and Igor, for the review! I'll push this change today.
>> 
>> Best regards,
>> 
>> 
>> Zoltan
>> 
>> On 02/05/2015 11:52 PM, Zoltán Majó wrote:
>>> Hi Igor,
>>> 
>>> 
>>> thank you for the feedback.
>>> 
>>> 
>>> On 02/05/2015 09:59 PM, Igor Veresov wrote:
>>>> The code looks good. But I think the comment about CompileThresholdScaling == 0 being equivalent to -Xint is bit misleading. The VM does not become interpreter-only, does it?
>>> 
>>> It does.
>>> 
>>> The reason is that CompileThreshold==0 has been historically equivalent to -Xint (see lines 3488-3493 in arguments.cpp). CompileThresholdScaling==0.0 scales down CompileThreshold to 0, so we decided to keep functionality consistent and make CompileThresholdScaling equivalent to -Xint as well when we introduced the CompileThresholdScaling flag in 8059604.
>>> 
>>> I hope this makes sense.
>>> 
>>> Thank you!
>>> 
>>> Best regards,
>>> 
>>> 
>>> Zoltan
>>> 
>>> 
>>> 
>>>> 
>>>> igor
>>>> 
>>>>> On Feb 5, 2015, at 8:18 AM, Zoltán Majó <zoltan.majo at oracle.com> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> 
>>>>> please review the following small patch.
>>>>> 
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8072398
>>>>> 
>>>>> Problem: The reported assert is triggered because the method LIRGenerator::increment_event_counter_impl is called with parameter 'frequency' set to -1.
>>>>> 
>>>>> The value originates from Arguments::scaled_freq_log(): If both variables 'freq_log' and 'scale' are small, 'scaled_freq' can become 0 and the function log2_intptr returns -1 for the value 0. A similar problem appears if freq_log == 0.
>>>>> 
>>>>> 
>>>>> Solution:
>>>>> 
>>>>> To treat the above mentioned cases graciously, this patch:
>>>>> 
>>>>> - Changes Arguments::scaled_freq_log() and Arguments::scaled_compile_threshold() to allow scaling the frequency/threshold by 0.0 and return 0.0 in these cases without calling log2_intptr().
>>>>> 
>>>>> - Adds a new condition to Arguments::set_tiered_flags(): Threshold values are *not scaled* if 'CompileThresholdScaling' is 0.0 (behavior equivalent to -Xint). Previously, this functionality was encoded in scaled_freq_log() and scaled_compile_threshold(). Handling this case in set_tiered_flags() results in clearer code.
>>>>> 
>>>>> - Improves comments.
>>>>> 
>>>>> 
>>>>> Webrev: http://cr.openjdk.java.net/~zmajo/8072398/webrev.00/
>>>>> 
>>>>> Testing:
>>>>> - manual testing with failing test cases
>>>>> - compiler/arguments/CheckCompileThresholdScaling.java
>>>>> - JPRT
>>>>> 
>>>>> Thank you and best regards,
>>>>> 
>>>>> 
>>>>> Zoltan
>>>>> 
>>> 
>> 
>