<!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>
    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>
  </body>
</html>