On Wed, 3 May 2023 03:12:00 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add missing new file > > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 659: > >> 657: // Invariant: tmpReg == 0. tmpReg is EAX which is the implicit >> cmpxchg comparand. >> 658: lock(); >> 659: cmpxchgptr(thread, Address(boxReg, >> OM_OFFSET_NO_MONITOR_VALUE_TAG(owner))); > > Sorry I don't quite follow the changes here as this appears to changing the > logic for all locking modes - aren't we still supposed to be cas'ing in the > "box" (scrReg) in legacy mode rather than the "thread"? IIRC, I have done that in response to an earlier review by somebody. The previous logic transiently stored box into the owner, and later - if the CAS succeeded - fetches the current thread* and stores that into owner, a few lines down from here. However, I just noticed that I do not remove that other code. So, for the sake of cleanliness of the legacy path, I'm going to revert this (we can & should make that change in a follow-up). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1183273733
