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

[Christian Thalinger]

> 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/~thartmann/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.

+  int check_count = 0, fail_count = 0;

I don’t like the check_count/fail_count logic because it’s fragile.

+// 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

> 
> 
> 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/ <http://cr.openjdk.java.net/%7Ethartmann/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>
>>>>> 
>>>>> 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/ <http://cr.openjdk.java.net/%7Ethartmann/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/20160217/6edaf705/attachment-0001.html>