RFR: 8256425: Obsolete Biased Locking in JDK 18 [v2]

[Patricio Chilano Mateo]

On Fri, 18 Jun 2021 15:34:07 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - remove test Test8062950.java + fix commments
>>  - fix comment in vm_version_ppc.cpp
>
> src/hotspot/cpu/arm/arm.ad line 5457:
> 
>> 5455:     __ b(loop, eq);
>> 5456:     __ teq($tmp$$Register, 0);
>> 5457:     __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::LoadStore | MacroAssembler::LoadLoad), noreg);
> 
> Does the comment you deleted mean that storeXConditional() is only used
> by biased locking or that only biased locking's use of storeXConditional() is
> the only caller that needed the membar?

If I grep for "new StoreIConditional" I don't find anything, same with StoreLConditional so it seems they are not used outside of biased locking (?). StoreIConditional was actually introduced for biased locking (6462850). I searched back in history and StoreLConditional appears first in opto/classes.hpp in 2002. Maybe @vnkozlov could confirm if we should keep them or remove them?

> src/hotspot/cpu/ppc/ppc.ad line 12140:
> 
>> 12138:                                  _rtm_counters, _stack_rtm_counters,
>> 12139:                                  ((Method*)(ra_->C->method()->constant_encoding()))->method_data(),
>> 12140:                                  /*TM*/ true, ra_->C->profile_rtm());
> 
> Not your bug, but that "TM" should be "RTM".

Fixed.

> src/hotspot/cpu/ppc/ppc.ad line 12174:
> 
>> 12172:     __ compiler_fast_unlock_object($crx$$CondRegister, $oop$$Register, $box$$Register,
>> 12173:                                    $tmp1$$Register, $tmp2$$Register, $tmp3$$Register,
>> 12174:                                    /*TM*/ true);
> 
> Not your bug, but that "TM" should be "RTM".

Fixed.

> src/hotspot/cpu/x86/templateTable_x86.cpp line 4026:
> 
>> 4024:     // initialize object header only.
>> 4025:     __ bind(initialize_header);
>> 4026:     __ movptr(Address(rax, oopDesc::mark_offset_in_bytes ()),
> 
> Not your bug, but can you delete the space before `()`?

Fixed.

> src/hotspot/share/runtime/deoptimization.cpp line 1440:
> 
>> 1438:           if (mark.has_locker() && fr.sp() > (intptr_t*)mark.locker()) {
>> 1439:             // With exec_mode == Unpack_none obj may be thread local and locked in
>> 1440:             // a callee frame. // Make the lock in the callee a recursive lock and restore the displaced header.
> 
> Please delete the embedded `//`.

Fixed.

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

PR: https://git.openjdk.java.net/jdk/pull/4522