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

[Zoltán Majó]

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