<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
I agree with Vitaly. You can find existing examples in the code:<br>
<br>
interpreter/rewriter.cpp: assert(cache_index ==
(u1)cache_index, "index overflow");<br>
<br>
You could also assert that reference_type == t after the assignment.<br>
<br>
dl<span class="changed"><br>
<br>
</span>On 1/24/2012 7:19 PM, Jiangli Zhou wrote:
<blockquote cite="mid:4F1F74D1.1040708@oracle.com" type="cite">
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
Hi Vitaly,<br>
<br>
Thanks for catching it! A range check seems to be more explicit in
this case:<br>
<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ejiangli/7132690/webrev.02/">http://cr.openjdk.java.net/~jiangli/7132690/webrev.02/</a><br>
<br>
Thanks,<br>
<br>
Jiangli<br>
<br>
On 01/24/2012 04:17 PM, Vitaly Davidovich wrote:
<blockquote
cite="mid:CAHjP37HpMrhA495m3xtWGdf-JfbsdxQQ8iUXvtDWn+3jRZ1jWg@mail.gmail.com"
type="cite">
<p>I think you'll be fully covered (including negative values)
by doing the following instead of hard coding max value in the
assert:</p>
<p>assert(t == (u1)t, "doesn't fit")</p>
<p>Sent from my phone</p>
<div class="gmail_quote">On Jan 24, 2012 6:33 PM, "Jiangli Zhou"
<<a moz-do-not-send="true"
href="mailto:jiangli.zhou@oracle.com" target="_blank">jiangli.zhou@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt
0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left: 1ex;">
<div text="#000000" bgcolor="#ffffff"> The updated webrev is
: <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ejiangli/7132690/webrev.01/"
target="_blank">http://cr.openjdk.java.net/~jiangli/7132690/webrev.01/</a><br>
<br>
Thanks,<br>
Jiangli<br>
<br>
On 01/24/2012 02:54 PM, Jiangli Zhou wrote:
<blockquote type="cite"> Hi Vitaly,<br>
<br>
An assert in the setter probably is a good idea. I
should have added it when making the change. Thanks for
the comments!<br>
<br>
Thanks,<br>
Jiangli<br>
<br>
On 01/24/2012 02:13 PM, Vitaly Davidovich wrote:
<blockquote type="cite">
<p>Hi Jiangli,</p>
<p>This is probably overly paranoid so feel free to
ignore, but should the setter in InstanceKlass
assert that the passed in ReferenceType fits into a
u1 instead of silently narrowing it? Or change the
setter to take a u1 and make caller do the cast?
This would prevent someone defining another member
of the enum with an explicit value that doesn't fit
into u1. Like I said, paranoia ... :)</p>
<p>Thanks</p>
<p>Sent from my phone</p>
<div class="gmail_quote">On Jan 24, 2012 3:06 PM,
"Jiangli Zhou" <<a moz-do-not-send="true"
href="mailto:jiangli.zhou@oracle.com"
target="_blank">jiangli.zhou@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="gmail_quote" style="margin: 0pt
0pt 0pt 0.8ex; border-left: 1px solid rgb(204,
204, 204); padding-left: 1ex;"> Hi,<br>
<br>
Please review the change for 7132690:<br>
<br>
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ejiangli/7132690/webrev.00/"
target="_blank">http://cr.openjdk.java.net/~jiangli/7132690/webrev.00/</a><br>
<br>
It changes InstanceKlass::_reference_type from
ReferenceType to u1. The ReferenceType is defined
as an enum with 6 values
(src/share/vm/utilities/globalDefinitions.hpp).
The change saves 4-byte for each class.<br>
<br>
Tested with runThese and ute vm.quick.testlist.
Ran jprt.<br>
<br>
Thanks,<br>
<br>
Jiangli<br>
</blockquote>
</div>
</blockquote>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</blockquote>
</body>
</html>