<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 20, 2016, at 8:12 AM, Jamsheed C m <<a href="mailto:jamsheed.c.m@oracle.com" class="">jamsheed.c.m@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
Hi Chris,<br class="">
<br class="">
I missed handling a valid case where
UnlockExperimentalVMOptions/UnlockDiagnosticVMOptions are made false
after experimental/diagnostic jvmci flags are modified.<br class="">
<br class="">
i.e : bin/java -XX:+UnlockExperimentalVMOptions -XX:-EnableJVMCI
-XX:+UseJVMCICompiler -XX:-UnlockExperimentalVMOptions -version<br class="">
<br class="">
made changes to handle this case.<br class="">
<br class="">
code change<br class="">
<pre style="font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: 1; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span class="new" style="color: blue; font-weight: bold;">+ // Check consistency of diagnostic flags if UnlockDiagnosticVMOptions is true</span>
<span class="new" style="color: blue; font-weight: bold;">+ // or not default. UnlockDiagnosticVMOptions is default true in debug builds.</span>
<span class="new" style="color: blue; font-weight: bold;">+ if (UnlockDiagnosticVMOptions ||</span>
<span class="new" style="color: blue; font-weight: bold;">+ !FLAG_IS_DEFAULT(UnlockDiagnosticVMOptions)) {
</span><span class="new" style="color: blue; font-weight: bold;">+ // Check consistency of experimental flags if UnlockExperimentalVMOptions is </span>
<span class="new" style="color: blue; font-weight: bold;">+ // true or not default.</span>
<span class="new" style="color: blue; font-weight: bold;">+ if (UnlockExperimentalVMOptions ||</span>
<span class="new" style="color: blue; font-weight: bold;">+ !FLAG_IS_DEFAULT(UnlockExperimentalVMOptions)) {</span>
</pre></div></div></blockquote><div><br class=""></div>Oh, good. But please don’t break the line. No new webrev needed.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div bgcolor="#FFFFFF" text="#000000" class="">
<br class="">
<br class="">
revised webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jcm/8145333/webrev.06/">http://cr.openjdk.java.net/~jcm/8145333/webrev.06/</a><br class="">
<br class="">
Thanks and Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On 2/20/2016 5:10 AM, Jamsheed C m
wrote:<br class="">
</div>
<blockquote cite="mid:56C7A7F1.3090500@oracle.com" type="cite" class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
Thanks, Chris.<br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On 2/19/2016 10:56 PM, Christian
Thalinger wrote:<br class="">
</div>
<blockquote cite="mid:FB70533A-C9ED-450D-9ECE-A0A71CBAE8E8@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
Looks good. Thanks.
<div class=""><br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On Feb 19, 2016, at 5:38 AM, Jamsheed C m
<<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> Hi
Chris,<br class="">
<br class="">
Made all suggested changes.<br class="">
<br class="">
Revised webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ejcm/8145333/webrev.05/">http://cr.openjdk.java.net/~jcm/8145333/webrev.05/</a><br class="">
<br class="">
Thanks and Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On 2/19/2016 10:53 AM,
Jamsheed C m wrote:<br class="">
</div>
<blockquote cite="mid:56C6A6DE.9060806@oracle.com" type="cite" class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<br class="">
<br class="">
<div class="moz-cite-prefix">On 2/18/2016 11:21 PM,
Christian Thalinger wrote:<br class="">
</div>
<blockquote cite="mid:F9AE0A5E-6DA3-45A7-83C2-70E74F479A24@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html; charset=windows-1252" class="">
+ if (EnableJVMCI)<br class="">
+ return true;<br class="">
<br class="">
Please use { } for all if statements.</blockquote>
Ok.<br class="">
<blockquote cite="mid:F9AE0A5E-6DA3-45A7-83C2-70E74F479A24@oracle.com" type="cite" class="">
<div class=""><br class="">
</div>
<div class="">Thanks for the comments; it’s easier
to understand now. May I suggest to do:</div>
<div class=""><br class="">
</div>
int skip_fail_count; <br class="">
<div class=""> if (!FLAG_IS_DEFAULT(EnableJVMCI))
{<br class="">
skip_fail_count = 1;<br class="">
} else {<br class="">
</div>
<div class=""> skip_fail_count = 0; <br class="">
}</div>
</blockquote>
Ok. <br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<blockquote cite="mid:F9AE0A5E-6DA3-45A7-83C2-70E74F479A24@oracle.com" type="cite" class="">
<div class=""><br class="">
</div>
<div class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On Feb 18, 2016, at 6:26 AM,
Jamsheed C m <<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> Hi Chris,<br class="">
<br class="">
Made all suggested changes.<br class="">
<br class="">
Revised webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.04/"></a><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~thartmann/8145333/webrev.04/">http://cr.openjdk.java.net/~thartmann/8145333/webrev.04/</a><br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/18/2016 6:24 AM, Jamsheed C m wrote:<br class="">
</div>
<blockquote cite="mid:56C5163D.7010208@oracle.com" type="cite" class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/18/2016 12:36 AM, Christian
Thalinger wrote:<br class="">
</div>
<blockquote cite="mid:E9876AA5-3C62-449B-9F92-60D1480EA2EC@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On Feb 17, 2016,
at 4:52 AM, Jamsheed C m <<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com"></a><a class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
experimental and diagnostic
flags are under condition
checks now.<br class="">
<br class="">
<a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.03/" class="">http://cr.openjdk.java.net/~thartmann/8145333/webrev.03/</a><br class="">
</div>
</div>
</blockquote>
<div class=""><br class="">
</div>
</div>
<div class="">I’m not very happy
about the new macros but I can see
why it’s useful.</div>
<div class=""><br class="">
</div>
<div class="">+#if INCLUDE_JVMCI<br class="">
+<br class="">
+// Check consistency of jvmci vm
argument settings.<br class="">
+bool
Arguments::check_jvmci_args_consistency()
{<br class="">
+ <br class="">
+<br class="">
+ if (!EnableJVMCI &&
is_any_jvmci_arg_values_changed())
{ <br class="">
+
print_jvmci_arg_inconsistency_error_message(); <br class="">
+ return false;<br class="">
<br class="">
<div class="">Please remove all
these empty lines. There are
others as well.</div>
</div>
<div class=""><br class="">
</div>
<div class="">+bool
is_any_jvmci_arg_values_changed()
{ </div>
<div class=""><br class="">
This method needs another name.</div>
<div class=""><br class="">
</div>
</blockquote>
Ok.<br class="">
<blockquote cite="mid:E9876AA5-3C62-449B-9F92-60D1480EA2EC@oracle.com" type="cite" class="">
<div class="">+ int check_count =
0, fail_count = 0;<br class="">
<br class="">
I don’t like the
check_count/fail_count logic
because it’s fragile.</div>
</blockquote>
As EanbleJVMCI flag cannot be avoided
from macro expansion logic, i had to
use some technique to exclude this
flag from failure decision making. <br class="">
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. <br class="">
<br class="">
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.<br class="">
<br class="">
let me know if i need to take second
approach. or if existing code is ok.<br class="">
<blockquote cite="mid:E9876AA5-3C62-449B-9F92-60D1480EA2EC@oracle.com" type="cite" class="">
<div class=""><br class="">
</div>
<div class="">+// Check if any jvmci
global defaults changed.<br class="">
+bool
is_any_jvmci_arg_values_changed();<br class="">
+// Print jvmci args inconsistency
error message.<br class="">
+void
print_jvmci_arg_inconsistency_error_message();<br class="">
<br class="">
These should be in some kind of
namespace or class. Maybe put
them into: class JVMCIGlobals</div>
</blockquote>
Ok.<br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<blockquote cite="mid:E9876AA5-3C62-449B-9F92-60D1480EA2EC@oracle.com" type="cite" class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> <br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/16/2016 6:50 PM,
Jamsheed C m wrote:<br class="">
</div>
<blockquote cite="mid:56C3221D.2040507@oracle.com" type="cite" class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
Hi Chris,<br class="">
<br class="">
Revised webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.02/"></a><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~thartmann/8145333/webrev.02/">http://cr.openjdk.java.net/~thartmann/8145333/webrev.02/</a><br class="">
<br class="">
This code take care of
every jvmci flags
automatically.<br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/13/2016 3:24 AM,
Christian Thalinger
wrote:<br class="">
</div>
<blockquote cite="mid:41CB30DB-33EE-4478-9EF8-57BF70CF6379@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On Feb
12, 2016, at 7:12
AM, Jamsheed C m
<<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com"></a><a class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> <br class="">
Hi Chris,<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/12/2016
10:12 PM,
Christian
Thalinger
wrote:<br class="">
</div>
<blockquote cite="mid:B788A82B-FE30-452A-BCCC-090DA6945584@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
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:</blockquote>
check against
original value,
and if not fail
?<br class="">
</div>
</div>
</blockquote>
<div class=""><br class="">
</div>
Same check for all
flags; they all depend
on +EnableJVMCI.</div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">
<div bgcolor="#FFFFFF" text="#000000" class="">
<blockquote cite="mid:B788A82B-FE30-452A-BCCC-090DA6945584@oracle.com" type="cite" class="">
<div class=""><br class="">
</div>
<div class=""><a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8145357"></a><a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8145357">https://bugs.openjdk.java.net/browse/JDK-8145357</a></div>
<div class=""><br class="">
</div>
<div class="">Also,
commandLineFlagConstraintsJVMCI.{cpp,hpp}
files are now
empty and I
think we
should remove
them.<br class="">
</div>
</blockquote>
i kept the file
as there can be
future use. Ok,
i will remove
it.<br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<blockquote cite="mid:B788A82B-FE30-452A-BCCC-090DA6945584@oracle.com" type="cite" class="">
<div class="">
<div class=""><br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On
Feb 11, 2016,
at 9:47 PM,
Jamsheed C m
<<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com"></a><a class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html;
charset=windows-1252" http-equiv="Content-Type" class="">
<div bgcolor="#FFFFFF" text="#000000" class=""> Hi
Chris,<br class="">
<br class="">
revised
webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.01/"></a><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~thartmann/8145333/webrev.01/">http://cr.openjdk.java.net/~thartmann/8145333/webrev.01/</a><br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
<br class="">
<div class="moz-cite-prefix">On
2/11/2016
11:31 PM,
Christian
Thalinger
wrote:<br class="">
</div>
<blockquote cite="mid:BE18392E-1A8A-4A72-B75B-3A24E11B0BCB@oracle.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On
Feb 11, 2016,
at 3:47 AM,
Jamsheed C m
<<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com"></a><a class="moz-txt-link-abbreviated" href="mailto:jamsheed.c.m@oracle.com">jamsheed.c.m@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">Hi,<br class="">
<br class="">
Request for
review<br class="">
<br class="">
bug url: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8145333"></a><a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8145333">https://bugs.openjdk.java.net/browse/JDK-8145333</a><br class="">
web url: <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Ethartmann/8145333/webrev.00/"></a><a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~thartmann/8145333/webrev.00/">http://cr.openjdk.java.net/~thartmann/8145333/webrev.00/</a><br class="">
</div>
</div>
</blockquote>
<div class=""><br class="">
</div>
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.</div>
<div class=""><br class="">
</div>
<div class="">Also,
can you change
the error
message?
Right now it
looks like
this:</div>
<div class=""><br class="">
</div>
<div class="">
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class="">$
./build/macosx-x86_64-normal-server-release/jdk/bin/java
-XX:+UnlockExperimentalVMOptions
-XX:+UseJVMCICompiler</div>
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class="">EnableJVMCI
must be
enabled</div>
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class="">Improperly
specified VM
option
'UseJVMCICompiler'</div>
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class="">Error:
Could not
create the
Java Virtual
Machine.</div>
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class="">Error:
A fatal
exception has
occurred.
Program will
exit.</div>
<div style="margin:
0px;
font-size:
10px;
line-height:
normal;
font-family:
Monaco;" class=""><br class="">
</div>
I think with
your changes
we will only
see the first
line without
mentioning
UseJVMCICompiler.
Is that
correct?</div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
Fix Summary:
added jvmci
flag
consistency
check after
arg parsing.<br class="">
<br class="">
tests: jprt<br class="">
unit test:
command line
verification<br class="">
<br class="">
Best Regards,<br class="">
Jamsheed<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></body></html>