RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v12]

Patricio Chilano Mateo pchilanomate at openjdk.org
Mon Oct 28 17:35:20 UTC 2024


On Sat, 26 Oct 2024 00:17:33 GMT, Dean Long <dlong at openjdk.org> wrote:

>It sounds like freeze/thaw isn't preserving FP, even though it is a callee-saved register according to the ABI. If the stubs tried to modify FP (or any other callee-saved register) and use that value after the native call, wouldn't that be a problem?
>
Yes, that would be a problem. We can't use callee saved registers in the stub after the call. I guess we could add some debug code that trashes all those registers right when we come back from the call. Or maybe just adding a comment there is enough.

> src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp line 191:
> 
>> 189:   // must restore the rfp value saved on enter though.
>> 190:   if (use_pop) {
>> 191:     ldp(rfp, lr, Address(post(sp, 2 * wordSize)));
> 
> leave() also calls authenticate_return_address(), which I assume we still want to call here.
> How about adding an optional parameter to leave() that will skip the problematic `mov(sp, rfp)`?

Right. I added it here for now to follow the same style in all platforms.

> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 135:
> 
>> 133:   assert(*f.addr_at(frame::interpreter_frame_last_sp_offset) == 0, "should be null for top frame");
>> 134:   intptr_t* lspp = f.addr_at(frame::interpreter_frame_last_sp_offset);
>> 135:   *lspp = f.unextended_sp() - f.fp();
> 
> Suggestion:
> 
>   f.interpreter_frame_set_last_sp(f.unextended_sp());

Changed, here and in the other platforms.

> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 159:
> 
>> 157: 
>> 158:   // The interpreter native wrapper code adds space in the stack equal to size_of_parameters()
>> 159:   // after the fixed part of the frame. For wait0 this is equal to 3 words (this + long parameter).
> 
> Suggestion:
> 
>   // after the fixed part of the frame. For wait0 this is equal to 2 words (this + long parameter).
> 
> Isn't that 2 words, not 3?

The timeout parameter is a long which we count as 2 words: https://github.com/openjdk/jdk/blob/0e3fc93dfb14378a848571a6b83282c0c73e690f/src/hotspot/share/runtime/signature.hpp#L347
I don't know why we do that for 64 bits.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819473410
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819465574
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819466532
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819472086


More information about the nio-dev mailing list