RFR(XS): 8145333: -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:-EnableJVMCI makes JVM crash

Christian Thalinger christian.thalinger at oracle.com
Fri Feb 19 17:26:02 UTC 2016 

Looks good.  Thanks.

> On Feb 19, 2016, at 5:38 AM, Jamsheed C m <jamsheed.c.m at oracle.com> wrote:
> 
> Hi Chris,
> 
> Made all suggested changes.
> 
> Revised webrev: http://cr.openjdk.java.net/~jcm/8145333/webrev.05/ <http://cr.openjdk.java.net/~jcm/8145333/webrev.05/>
> 
> Thanks and Best Regards,
> Jamsheed
> 
> On 2/19/2016 10:53 AM, Jamsheed C m wrote:
>> 
>> 
>> On 2/18/2016 11:21 PM, Christian Thalinger wrote:
>>> +  if (EnableJVMCI)
>>> +    return true;
>>> 
>>> Please use { } for all if statements.
>> Ok.
>>> 
>>> Thanks for the comments; it’s easier to understand now.  May I suggest to do:
>>> 
>>>   int skip_fail_count;  
>>>   if (!FLAG_IS_DEFAULT(EnableJVMCI)) {
>>>     skip_fail_count = 1;
>>>   } else {
>>>     skip_fail_count = 0;  
>>>   }
>> Ok. 
>> 
>> Best Regards,
>> Jamsheed
>>> 
>>>> On Feb 18, 2016, at 6:26 AM, Jamsheed C m <jamsheed.c.m at oracle.com <mailto:jamsheed.c.m at oracle.com>> wrote:
>>>> 
>>>> Hi Chris,
>>>> 
>>>> Made all suggested changes.
>>>> 
>>>> Revised webrev: http://cr.openjdk.java.net/~thartmann/8145333/webrev.04/ <http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.04/>
>>>> 
>>>> Best Regards,
>>>> Jamsheed
>>>> 
>>>> On 2/18/2016 6:24 AM, Jamsheed C m wrote:
>>>>> 
>>>>> 
>>>>> On 2/18/2016 12:36 AM, Christian Thalinger wrote:
>>>>>> 
>>>>>>> On Feb 17, 2016, at 4:52 AM, Jamsheed C m <jamsheed.c.m at oracle.com <mailto:jamsheed.c.m at oracle.com>> wrote:
>>>>>>> 
>>>>>>> experimental and diagnostic flags are under condition checks now.
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~thartmann/8145333/webrev.03/ <http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.03/>
>>>>>> 
>>>>>> I’m not very happy about the new macros but I can see why it’s useful.
>>>>>> 
>>>>>> +#if INCLUDE_JVMCI
>>>>>> +
>>>>>> +// Check consistency of jvmci vm argument settings.
>>>>>> +bool Arguments::check_jvmci_args_consistency() {
>>>>>> + 
>>>>>> +
>>>>>> +  if (!EnableJVMCI && is_any_jvmci_arg_values_changed()) { 
>>>>>> +    print_jvmci_arg_inconsistency_error_message(); 
>>>>>> +    return false;
>>>>>> 
>>>>>> Please remove all these empty lines.  There are others as well.
>>>>>> 
>>>>>> +bool is_any_jvmci_arg_values_changed() {  
>>>>>> 
>>>>>> This method needs another name.
>>>>>> 
>>>>> Ok.
>>>>>> +  int check_count = 0, fail_count = 0;
>>>>>> 
>>>>>> I don’t like the check_count/fail_count logic because it’s fragile.
>>>>> As  EanbleJVMCI flag cannot be avoided from macro expansion logic, i had to use some technique to exclude this flag from failure decision making. 
>>>>> Check_count/fail_count logic was added as a solution for this. i.e When EnableJVMCI is changed, code requires at-least 2 check failures to decide a real failure. otherwise one failure is sufficient. 
>>>>>  
>>>>> if i remove this logic, i will have to use strcmp(#FLAG, "EnableJVMCI") which will add a little more instructions that will  execute at-least once for every flag consistent run.
>>>>> 
>>>>> let me know if i need to take second approach. or if existing code is ok.
>>>>>> 
>>>>>> +// Check if any jvmci global defaults changed.
>>>>>> +bool is_any_jvmci_arg_values_changed();
>>>>>> +// Print jvmci args inconsistency error message.
>>>>>> +void print_jvmci_arg_inconsistency_error_message();
>>>>>> 
>>>>>> These should be in some kind of namespace or class.  Maybe put them into: class JVMCIGlobals
>>>>> Ok.
>>>>> 
>>>>> Best Regards,
>>>>> Jamsheed
>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> Best Regards,
>>>>>>> Jamsheed
>>>>>>> 
>>>>>>> On 2/16/2016 6:50 PM, Jamsheed C m wrote:
>>>>>>>> Hi Chris,
>>>>>>>> 
>>>>>>>> Revised webrev:  <http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.02/>http://cr.openjdk.java.net/~thartmann/8145333/webrev.02/ <http://cr.openjdk.java.net/~thartmann/8145333/webrev.02/>
>>>>>>>> 
>>>>>>>> This code take care of every jvmci flags automatically.
>>>>>>>> 
>>>>>>>> Best Regards,
>>>>>>>> Jamsheed
>>>>>>>> 
>>>>>>>> On 2/13/2016 3:24 AM, Christian Thalinger wrote:
>>>>>>>>> 
>>>>>>>>>> On Feb 12, 2016, at 7:12 AM, Jamsheed C m < <mailto:jamsheed.c.m at oracle.com>jamsheed.c.m at oracle.com <mailto:jamsheed.c.m at oracle.com>> wrote:
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Hi Chris,
>>>>>>>>>> 
>>>>>>>>>> On 2/12/2016 10:12 PM, Christian Thalinger wrote:
>>>>>>>>>>> Well, now you have to manually check all flags that had a constraint directive.  That is the annoying part and what I complained about in:
>>>>>>>>>> check against original value, and if not fail ?
>>>>>>>>> 
>>>>>>>>> Same check for all flags; they all depend on +EnableJVMCI.
>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>  <https://bugs.openjdk.java.net/browse/JDK-8145357>https://bugs.openjdk.java.net/browse/JDK-8145357 <https://bugs.openjdk.java.net/browse/JDK-8145357>
>>>>>>>>>>> 
>>>>>>>>>>> Also, commandLineFlagConstraintsJVMCI.{cpp,hpp} files are now empty and I think we should remove them.
>>>>>>>>>> i kept the file as there can be future use. Ok, i will remove it.
>>>>>>>>>> 
>>>>>>>>>> Best Regards,
>>>>>>>>>> Jamsheed
>>>>>>>>>>> 
>>>>>>>>>>>> On Feb 11, 2016, at 9:47 PM, Jamsheed C m < <mailto:jamsheed.c.m at oracle.com>jamsheed.c.m at oracle.com <mailto:jamsheed.c.m at oracle.com>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>> 
>>>>>>>>>>>> revised webrev:  <http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.01/>http://cr.openjdk.java.net/~thartmann/8145333/webrev.01/ <http://cr.openjdk.java.net/~thartmann/8145333/webrev.01/>
>>>>>>>>>>>> 
>>>>>>>>>>>> Best Regards,
>>>>>>>>>>>> Jamsheed
>>>>>>>>>>>> 
>>>>>>>>>>>> On 2/11/2016 11:31 PM, Christian Thalinger wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Feb 11, 2016, at 3:47 AM, Jamsheed C m < <mailto:jamsheed.c.m at oracle.com>jamsheed.c.m at oracle.com <mailto:jamsheed.c.m at oracle.com>> wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Request for review
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> bug url:   <https://bugs.openjdk.java.net/browse/JDK-8145333>https://bugs.openjdk.java.net/browse/JDK-8145333 <https://bugs.openjdk.java.net/browse/JDK-8145333>
>>>>>>>>>>>>>> web url:  <http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.00/>http://cr.openjdk.java.net/~thartmann/8145333/webrev.00/ <http://cr.openjdk.java.net/~thartmann/8145333/webrev.00/>
>>>>>>>>>>>>> 
>>>>>>>>>>>>> That looks alright but we should remove the constraints from all the JVMCI command line flags because the way we use them is not supported.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Also, can you change the error message?  Right now it looks like this:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> $ ./build/macosx-x86_64-normal-server-release/jdk/bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseJVMCICompiler
>>>>>>>>>>>>> EnableJVMCI must be enabled
>>>>>>>>>>>>> Improperly specified VM option 'UseJVMCICompiler'
>>>>>>>>>>>>> Error: Could not create the Java Virtual Machine.
>>>>>>>>>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I think with your changes we will only see the first line without mentioning UseJVMCICompiler.  Is that correct?
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Fix Summary: added jvmci flag consistency check after arg parsing.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> tests: jprt
>>>>>>>>>>>>>> unit test: command line verification
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Best Regards,
>>>>>>>>>>>>>> Jamsheed
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160219/f53b268a/attachment-0001.html>

More information about the hotspot-compiler-dev mailing list