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

[Jamsheed C m]

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/~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 <mailto: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/cc32d8a1/attachment-0001.html>