RFR: 8364141: Remove LockingMode related code from x86 [v2]

Fredrik Bredberg fbredberg at openjdk.org
Mon Aug 4 12:52:40 UTC 2025


On Fri, 1 Aug 2025 06:12:28 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update one after review
>
> Nice cleanup! Some small initial comments.
> 
> All the "displaced header" comments looks out of place. Displacing the header word on the stack (in the box) was purely a LM_LEGACY thing. Now we only displace it in the ObjectMonitor which is only handled (inflation / deflation) in the C++ runtime.
> 
> There are some more `BasicLock::displaced_header_offset_in_bytes()` asserts inside the x86 code. For callers of these methods, could be removed now or when the `BasicLock` is cleaned up.
> 
> There are some unused variables because of "displaced header" code that is kept.
> 
> `fast_lock_lightweight` and `fast_unlock_lockweight` should probably be renamed `fast_lock` and `fast_unlock` to be in sync with all the comments. (Or all the comments should be updated) (Same with C2 AD instruction)

@xmas92 
> All the "displaced header" comments looks out of place. Displacing the header word on the stack (in the box) was purely a LM_LEGACY thing. Now we only displace it in the ObjectMonitor which is only handled (inflation / deflation) in the C++ runtime.
> 
> There are some more `BasicLock::displaced_header_offset_in_bytes()` asserts inside the x86 code. For callers of these methods, could be removed now or when the `BasicLock` is cleaned up.

I basically agree. It's a bit of a mess right now and arguments that are called `disp_hdr` should probably be changed to `basic_lock`. But I'd rather do that in a separate PR and have this (and all the similar other platform PRs) only handle removing of dead code due to removal of the `LockingMode` flag. It's far easier to review a PR that has just removed code, than to a one that has also refactored lots of code.

> There are some unused variables because of "displaced header" code that is kept.

Fixed those you saw, and some more that I found.

> `fast_lock_lightweight` and `fast_unlock_lockweight` should probably be renamed `fast_lock` and `fast_unlock` to be in sync with all the comments. (Or all the comments should be updated) (Same with C2 AD instruction)

Totally agree. But that also for a separate PR after the shared (non-platform specific) files has been fixed.

> src/hotspot/cpu/x86/interp_masm_x86.cpp line 1032:
> 
>> 1030:   const Register tmp_reg = rbx;
>> 1031:   const Register obj_reg = c_rarg3; // Will contain the oop
>> 1032:   const Register rklass_decode_tmp = rscratch1;
> 
> Unused variable.

Fixed

> src/hotspot/cpu/x86/interp_masm_x86.cpp line 1037:
> 
>> 1035:   const int lock_offset = in_bytes(BasicObjectLock::lock_offset());
>> 1036:   const int mark_offset = lock_offset +
>> 1037:                           BasicLock::displaced_header_offset_in_bytes();
> 
> Unused variable.

Fixed

> src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 2194:
> 
>> 2192: 
>> 2193:     // Load the oop from the handle
>> 2194:     __ movptr(obj_reg, Address(oop_handle_reg, 0));
> 
> `mark_word_offset` and `count_mon` unused variable above.

Fixed

-------------

PR Comment: https://git.openjdk.org/jdk/pull/26552#issuecomment-3150525525
PR Review Comment: https://git.openjdk.org/jdk/pull/26552#discussion_r2251387407
PR Review Comment: https://git.openjdk.org/jdk/pull/26552#discussion_r2251387954
PR Review Comment: https://git.openjdk.org/jdk/pull/26552#discussion_r2251388647


More information about the hotspot-compiler-dev mailing list