On Thu, 11 Jul 2024 09:25:52 GMT, Yudi Zheng <yzh...@openjdk.org> wrote:
>> Axel Boldt-Christmas has updated the pull request incrementally with four >> additional commits since the last revision: >> >> - Add extra comments in LightweightSynchronizer::inflate_fast_locked_object >> - Fix typos >> - Remove unused variable >> - Add missing inline qualifiers > > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 843: > >> 841: movptr(monitor, Address(box, >> BasicLock::object_monitor_cache_offset_in_bytes())); >> 842: // null check with ZF == 0, no valid pointer below >> alignof(ObjectMonitor*) >> 843: cmpptr(monitor, alignof(ObjectMonitor*)); > > Is this only for keeping `ZF == 0` and can be replaced by `test; je` if we > are not using `jne` to jump to the slow path? Or is there any performance > concern? Btw, I think `ZF` is always rewritten before entering into the slow > path > https://github.com/openjdk/jdk/blob/b32e4a68bca588d908bd81a398eb3171a6876dc5/src/hotspot/cpu/x86/c2_CodeStubs_x86.cpp#L98-L102 You are correct the condition flag is not important here. At some point we had more than just `nullptr` and and `ObjectMonitor*` values, but also some small signal values which allowed us to move some slow path code into the runtime. When this was removed I just made the checks do the same on both aarch64 and x86. (Where aarch64 does not have a stub and jumps directly to the continuation requiring the correct condition flags after the branch.) _Side note: This might be something that will be explored further in the future. And allow to move a lot of the LM_LIGHTWEIGHT slow path code away from the lock node and code stub into the runtime._ ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1673800250
