On Fri, 21 May 2021 08:45:29 GMT, Robbin Ehn <r...@openjdk.org> wrote:
>> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 212: >> >>> 210: _thread->frame_anchor()->make_walkable(_thread); >>> 211: OrderAccess::storestore(); >>> 212: _thread->set_thread_state(_thread_in_native); >> >> Can't help but think the ppc/aarch64 folk have it right and that >> set_thread_state should always (?) have release semantics - and would then >> be renamed release_set_thread_state(). Also avoids a double barrier on >> ppc/aarch64. Followup RFE? >> But note that we need the storestore after all the make_walkable's that are >> followed by set_thread_state. > > Ah, you mean the pre-exiting missing of a storestore in TBIVM? Fixed. > But we have more of those. > > Note that on TSO it's not needed, since both are volatile and will not be > re-ordered by compiler. > Yes, the platform which actually do care about avoiding release for > performance have it (because it's needed). > So yes it should always be there, good RFE! Okay I will file a RFE to get the memory ordering semantics of set_thread_state cleaned up so we don't have any missing barriers where needed, nor redundant barriers. Note that the expectation is that we write the code for the loosest possible memory model with all barriers expressed in the code, and it is then up to the implementation of those barriers to reduce it to nothing if not needed on a given platform. Thanks, David ------------- PR: https://git.openjdk.java.net/jdk/pull/3875
