On Tue, 29 Oct 2024 22:15:16 GMT, Patricio Chilano Mateo 
<pchilanom...@openjdk.org> wrote:

>> src/hotspot/share/code/nmethod.cpp line 1302:
>> 
>>> 1300:     _compiler_type           = type;
>>> 1301:     _orig_pc_offset          = 0;
>>> 1302:     _num_stack_arg_slots     = 0;
>> 
>> Was the old value wrong, unneeded, or is this set somewhere else?  If this 
>> field is not used, then we might want to set it to an illegal value in debug 
>> builds.
>
> We read this value from the freeze/thaw code in several places. Since the 
> only compiled native frame we allow to freeze is Object.wait0 the old value 
> would be zero too. But I think the correct thing is to just set it to zero 
> always since a value > 0 is only meaningful for Java methods.

Isn't it possible that we might allow more compiled native frames in the 
future, and then we would have to undo this change?  I think this change should 
be reverted.  If continuations code wants to assert that this is 0, then that 
should be in continuations code, the nmethod code doesn't need to know how this 
field is used.  However, it looks like continuations code is the only client of 
this field, so I can see how it would be tempting to just set it to 0 here, but 
it doesn't feel right.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1823572138

Reply via email to