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

[Zoltán Majó]

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