On Mon, 24 May 2021 02:08:43 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> 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 I have filed JDK-8267585, but I realize now what you are saying. The existing code handles the barrier inconsistently. In most places it is only inside set_thread_state, for those platforms that need it. Elsewhere it is explicit in the code that calls set_thread_state - which leads to redundant barriers on the platforms that need them. I mistakenly flagged the implicit cases as a bug based on the existence of the explicit cases - but that is not the case. Sorry for the confusion. The additional storestore barriers you added before the call to set_thread_state can be removed, and I will tidy this up using the new RFE. Thanks, David ------------- PR: https://git.openjdk.java.net/jdk/pull/3875