<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#ffffff">
Hi John,<br>
<br>
Here is the updated webrev with Address constant change in where
it's appropriate: <br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jiangli/8004076/webrev.01/">http://cr.openjdk.java.net/~jiangli/8004076/webrev.01/</a>. <br>
<br>
I'm also double checking with jprt and runthese tests.<br>
<br>
Thanks,<br>
Jiangli<br>
<br>
On 12/07/2012 02:52 PM, Jiangli Zhou wrote:
<blockquote cite="mid:50C2732B.8020801@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hi John,<br>
<br>
Thanks a lot for the review and suggestion. I'll add Address
constants where it's feasible.<br>
<br>
Jiangli<br>
<br>
On 12/07/2012 02:40 PM, John Rose wrote:
<blockquote
cite="mid:75AA2E86-8884-441C-B3E7-E4082807A1CD@oracle.com"
type="cite">
<div>
<div>On Dec 7, 2012, at 1:45 PM, Jiangli Zhou wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite"><span class="Apple-style-span"
style="border-collapse: separate; font-family: Helvetica;
font-style: normal; font-variant: normal; font-weight:
normal; letter-spacing: normal; line-height: normal;
orphans: 2; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
font-size: medium;">Could I have another reviewer for the
change? Either official or non-official reviewer is okay.<br>
</span></blockquote>
<br>
</div>
<div>I have a comment on this one. (And you can use me as a
reviewer.)</div>
<br>
<div>In the sparc code, I would prefer to see the Address
constants kept around, because they make the code easier to
read.</div>
<div><br>
</div>
<div>
<div>- const Address size_of_parameters(G5_method,
Method::size_of_parameters_offset());</div>
<div>- const Address size_of_locals (G5_method,
Method::size_of_locals_offset());</div>
<div>+ const Register G3_constMethod = Glocals_size;</div>
<div>+ const Address size_of_parameters(G3_ConstMethod,
ConstMethod::size_of_parameters_offset());</div>
<div>+ const Address size_of_locals
(G3_ConstMethod, ConstMethod::size_of_locals_offset());</div>
<div> const Address constMethod (G5_method,
Method::const_offset());</div>
<div> int rounded_vm_local_words = round_to(
frame::interpreter_frame_vm_local_words, WordsPerLong );</div>
<div> </div>
<div>@@ -511,7 +509,8 @@</div>
<div> // Lscratch can't be used as a temporary because the
call_stub uses</div>
<div> // it to assert that the stack frame was setup
correctly.</div>
<div> </div>
<div>+ __ ld_ptr( constMethod, G3_constMethod );</div>
<div>- __ lduh( size_of_parameters, Glocals_size);</div>
<div><br>
</div>
</div>
<div>Maybe the details make it difficult, but I think it's
possible in most places.</div>
<div><br>
</div>
<div>As a side effect, using a fixed register (like G3) for the
ConstMethod will make debugging easier.</div>
<div><br>
</div>
<div>Same comment for x86/64.</div>
<div><br>
</div>
<div>This may be more difficult to do on x86/32, since there are
fewer temp registers. But that's an oddball case.</div>
<div><br>
</div>
<div>Thanks,</div>
<div>— John</div>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>