[9] RFR (XS): 8072422: Change a number of flags controlling loop optimizations to 'develop'

[Zoltán Majó]

P.S.: Typo in my previous mail: I meant webrev.02 (and not webrev.03). 
Sorry.

On 04/04/2016 12:49 PM, Zoltán Majó wrote:
> Thank you, Vladimir and Chris, for the reviews! For the record: I'll 
> push the latest webrev (webrev.03) today.
>
> Best regards,
>
>
> Zoltan
>
> On 04/01/2016 02:32 PM, Zoltán Majó wrote:
>> Hi Vladimir,
>>
>>
>> thank you for the feedback!
>>
>> On 03/31/2016 05:57 PM, Vladimir Kozlov wrote:
>>> It is nice to have not product flags which is easy to remove :)
>>
>> Yes, indeed. :-)
>>
>>>
>>> Clean up looks good.
>>
>> Thank you.
>>
>>>
>>> Can you leave test but remove "-XX:+UnlockDiagnosticVMOptions 
>>> -XX:-LoopLimitCheck" only? It has interesting code shape. Add 
>>> comment that it was ran with "-XX:+UnlockDiagnosticVMOptions 
>>> -XX:-LoopLimitCheck" to trigger problem.
>>
>> Yes, of course. Here is the updated webrev:
>> http://cr.openjdk.java.net/~zmajo/8072422/webrev.02/
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 3/31/16 1:15 AM, Zoltán Majó wrote:
>>>> Hi Vladimir,
>>>>
>>>>
>>>> thank you for your feedback!
>>>>
>>>> On 03/23/2016 11:58 PM, Vladimir Kozlov wrote:
>>>>> These flags were added when I fixed long standing C2 problem with 
>>>>> counted loops: 5091921.
>>>>> They were added to have ability to revert back to original code if 
>>>>> new code cause a problem.
>>>>> Looks like the old code which executed with these flags switched 
>>>>> off become rotten.
>>>>>
>>>>> Zoltan, did you find what cause the crash? Looks like product VM 
>>>>> was used in the bug report. What result gives
>>>>> fastdebug VM?
>>>>
>>>> I've tried starting different VM versions with the flag(s) off. The 
>>>> most frequent error I get is
>>>>
>>>> # Internal Error 
>>>> (/home/zmajo/Documents/repos/8072422/hotspot/src/share/vm/opto/loopnode.cpp:3615), 
>>>> pid=32727, tid=32746
>>>> # assert(false) failed: Bad graph detected in build_loop_late
>>>>
>>>> So it seems that the code executed with the flags off has indeed 
>>>> become rotten.
>>>>
>>>>> Converting flags to develop will not prevent problems happening 
>>>>> with fastdebug VM where these flags could be switched
>>>>> off even when they are develop.
>>>>>
>>>>> If the problem with original code (flags are off) is something 
>>>>> fundamental we may simple remove old code and remove
>>>>> these flags and have only new code. 5 years already passed since 
>>>>> 5091921 was fixed.
>>>>
>>>> Yes, I agree. I think it's reasonable to remove the old code.
>>>>
>>>> Here is the new webrev:
>>>> http://cr.openjdk.java.net/~zmajo/8072422/webrev.01/
>>>>
>>>> The changes pass JPRT.
>>>>
>>>> I've changed the title of the bug to "Cleanup: Remove some unused 
>>>> flags/code in loop optimizations" to better reflect
>>>> what the change is doing. I have kept the original title in the RFR.
>>>>
>>>> Thank you!
>>>>
>>>> Best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 3/23/16 6:26 AM, Zoltán Majó wrote:
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> please review the patch for 8072422.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8072422
>>>>>>
>>>>>> Problem: Some flags controlling loop optimizations are currently 
>>>>>> 'diagnostic'. Even though these flags are useful
>>>>>> mostly for compiler-related development, their value can be 
>>>>>> changed not only in
>>>>>> fastdebug, but also also in release builds,
>>>>>>
>>>>>> Solution: Change the flags to 'develop'.
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~zmajo/8072422/webrev.00/
>>>>>>
>>>>>> Testing:
>>>>>> - locally built/started VM;
>>>>>> - locally executed 
>>>>>> runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java.
>>>>>>
>>>>>> Thank you and best regards,
>>>>>>
>>>>>>
>>>>>> Zoltan
>>>>>>
>>>>
>>
>