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

[Jamsheed C m]

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/20160218/40ae3aef/attachment-0001.html>