[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Incorrect comments in fast-path lightweight unlocking code in the interpreter
From:       rednaxelafx () gmail ! com (Krystal Mok)
Date:       2012-03-30 13:02:43
Message-ID: CA+cQ+tRx_myLH6hk=j=HKvbU5YSwyF+Eyn8PsHRKykPEKo9hJQ () mail ! gmail ! com
[Download RAW message or body]

Hi all,

I've updated the patch, posted here:
https://gist.github.com/2250418#file_comment_ver2.patch
Could anyone please review the updated version?

There are a few more edits, but none of them really change the generated
code; these are purely cosmetic changes that fix comment inconsistencies.

Some explanations:

src/cpu/x86/vm/frame_x86.hpp:

The asm interpreter stack frame layout diagram contained a field "monitor
block size", which doesn't match the actual layout.
> From the logic at the end
of TemplateInterpreterGenerator::generate_fixed_frame(), we can see that
the field is "pointer to monitor block top / expression stack bottom".

src/cpu/x86/vm/interp_masm_x86_32.cpp
src/cpu/x86/vm/interp_masm_x86_64.cpp

Updated the comments as David suggested.

src/cpu/x86/vm/templateInterpreter_x86_32.cpp
src/cpu/x86/vm/templateInterpreter_x86_64.cpp

The names "montop" and "monbot" are reversed, so that they become
consistent with the code in frame::interpreter_frame_monitor_begin()

src/cpu/x86/vm/templateTable_x86_32.cpp
src/cpu/x86/vm/templateTable_x86_64.cpp

Added comments to the local variables "monitor_block_top" and
"monitor_block_bot".
Also fixed a place where "monitor_block_bot" was used, but should have been
"monitor_block_top".
These two variables actually contained the same value due to the way the
frame is laid out on x86. So even though the semantics weren't correct, the
generated code was correct.

- Kris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20120330/bcd1234d/attachment.html \



[prev in list] [next in list] [prev in thread] [next in thread]