On Wed, 26 Apr 2023 03:09:53 GMT, Quan Anh Mai <qa...@openjdk.org> wrote:
>> Roman Kennke has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove unnecessary comments > > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 691: > >> 689: jccb(Assembler::notEqual, NO_COUNT); // If not recursive, ZF = 0 >> at this point (fail) >> 690: incq(Address(scrReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(recursions))); >> 691: xorq(rax, rax); // Set ZF = 1 (success) for recursive lock, denoting >> locking success > > `xorl` would save a byte here, and some similar places. Yes, but see above. > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 701: > >> 699: // ZFlag == 0 count in slow path >> 700: jccb(Assembler::notZero, NO_COUNT); // jump if ZFlag == 0 >> 701: > > `DONE_LABEL` is conditionally jumped into from a lot of places, the only path > it is reached without known `ZF` seems to be `LM_LEGAGY` fall-through. Maybe > refactor a little to eliminate this block. I intentionally have not changed the existing paths to make it absolutely clear that the old behaviour is not changed. I'd rather make any changes to the stack-locking in a separate follow-up. > src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 926: > >> 924: // Intentional fall-thru into DONE_LABEL >> 925: } >> 926: bind(DONE_LABEL); > > Similar to `fast_lock`, this `DONE_LABEL` can be removed. Yes but see above ;-) > src/hotspot/cpu/x86/macroAssembler_x86.cpp line 9704: > >> 9702: >> 9703: // If successful, push object to lock-stack. >> 9704: movl(tmp, Address(thread, JavaThread::lock_stack_top_offset())); > > This value seems to be loaded twice, can they be merged? That would be nice, but we cannot do this without allocating another tmp register. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177695188 PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177695040 PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177700194 PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1177696506
