RFR(XS): 8145333: -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:-EnableJVMCI makes JVM crash
Jamsheed C m
jamsheed.c.m at oracle.com
Sat Feb 20 18:12:50 UTC 2016
Hi Chris,
I missed handling a valid case where
UnlockExperimentalVMOptions/UnlockDiagnosticVMOptions are made false
after experimental/diagnostic jvmci flags are modified.
i.e : bin/java -XX:+UnlockExperimentalVMOptions -XX:-EnableJVMCI
-XX:+UseJVMCICompiler -XX:-UnlockExperimentalVMOptions -version
made changes to handle this case.
code change
+ // Check consistency of diagnostic flags if UnlockDiagnosticVMOptions
is true
+ // or not default. UnlockDiagnosticVMOptions is default true in debug
builds.
+ if (UnlockDiagnosticVMOptions ||
+ !FLAG_IS_DEFAULT(UnlockDiagnosticVMOptions)) { + // Check consistency
of experimental flags if UnlockExperimentalVMOptions is
+ // true or not default.
+ if (UnlockExperimentalVMOptions ||
+ !FLAG_IS_DEFAULT(UnlockExperimentalVMOptions)) {
revised webrev: http://cr.openjdk.java.net/~jcm/8145333/webrev.06/
Thanks and Best Regards,
Jamsheed
On 2/20/2016 5:10 AM, Jamsheed C m wrote:
> Thanks, Chris.
>
> Best Regards,
> Jamsheed
>
> On 2/19/2016 10:56 PM, Christian Thalinger wrote:
>> 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/
>>>
>>> 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/20160220/384b3902/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list