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

Fri Feb 19 15:38:02 UTC 2016

Hi Chris,

Made all suggested changes.

Revised webrev: 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> 
>>> wrote:
>>>
>>> Hi Chris,
>>>
>>> Made all suggested changes.
>>>
>>> Revised webrev: http://cr.openjdk.java.net/~thartmann/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> 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/~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 
>>>>>>>>> <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
>>>>>>>>>>
>>>>>>>>>> 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 
>>>>>>>>>>> <jamsheed.c.m at oracle.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>
>>>>>>>>>>> revised webrev: 
>>>>>>>>>>> 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 
>>>>>>>>>>>>> <jamsheed.c.m at oracle.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Request for review
>>>>>>>>>>>>>
>>>>>>>>>>>>> bug url: https://bugs.openjdk.java.net/browse/JDK-8145333
>>>>>>>>>>>>> web url: 
>>>>>>>>>>>>> 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/bc02b666/attachment-0001.html>