Hi Jorn,

On 17/06/2021 9:28 pm, Jorn Vernee wrote:
On Thu, 17 Jun 2021 00:23:19 GMT, David Holmes <dhol...@openjdk.org> wrote:

Jorn Vernee has updated the pull request incrementally with one additional 
commit since the last revision:

   Add comment about optimized entry frames only being generated on x86_64

src/hotspot/share/runtime/frame.inline.hpp line 54:

52: inline bool frame::is_first_frame() const {
53:   return (is_entry_frame() && entry_frame_is_first())
54:       || (is_optimized_entry_frame() && optimized_entry_frame_is_first());

Given `optimized_entry_frame_is_first` is only defined on a couple of 
platforms, it is far from obvious that this call can never happen on the other 
platforms. A comment explaining this would be useful.

Thanks, I've added the following comment:

```C++
inline bool frame::is_first_frame() const {
   return (is_entry_frame() && entry_frame_is_first())
       // optimized_entry_frame_is_first is currently only implemented on 
x86_64.
       // This is okay since optimized entry frames are only generated on x86_64
       // as well (see ProgrammableUpcallHandler::generate_optimized_upcall_stub
       // in universalUpcallHandler_x86_64.cpp), so is_optimized_entry_frame 
will
       // always return false on platforms where optimized_entry_frame_is_first
       // is not implemented.
       || (is_optimized_entry_frame() && optimized_entry_frame_is_first());
}

Now that you have explained it I think a much simpler comment will suffice :)

    return (is_entry_frame() && entry_frame_is_first()) ||
       // Optimized entry frames are only present on certain platforms
       (is_optimized_entry_frame() && optimized_entry_frame_is_first());

Cheers,
David


-------------

PR: https://git.openjdk.java.net/jdk17/pull/76

Reply via email to